Skip to content

improvement(secret-rotation): validate conflicts before rotating external credentials#5461

Merged
victorvhs017 merged 1 commit intomainfrom
improvement/validate-secret-before-rotation
Feb 18, 2026
Merged

improvement(secret-rotation): validate conflicts before rotating external credentials#5461
victorvhs017 merged 1 commit intomainfrom
improvement/validate-secret-before-rotation

Conversation

@victorvhs017
Copy link
Contributor

Context

When creating a new secret rotation, if validation checks fail (e.g., rotation with the same name already exists, or secrets with mapped names already exist), the error is returned, but the resource credentials have already been changed.

This happens because the validation for conflicting secrets was only performed inside the transaction callback, which executes after the external credential rotation has already occurred. This leaves users in a confusing state where their password was changed on the external system, but no rotation record was created in Infisical.

To fix this we added a check before the rotation to fail fast.

Steps to verify the change

  1. Create a secret with key DB_PASSWORD
  2. Try to create a secret rotation that maps to DB_PASSWORD at the same path
  3. Before: The external password would be rotated, then the creation would fail with "secret already exists"
  4. After: The creation fails immediately with "secret already exists" without rotating the external password

Type

  • Fix
  • Feature
  • Improvement
  • Breaking
  • Docs
  • Chore

Checklist

  • Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
  • Tested locally
  • Updated docs (if needed)
  • Read the contributing guide

…hance validation checks

- Updated the transaction parameter to be optional in the secret rotation service factory.
- Improved validation logic to ensure no conflicting secrets exist before credential rotation, including checks for existing rotations and mapped names.
- Added comments for clarity on validation steps.
@maidul98
Copy link
Collaborator

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 12, 2026

Greptile Overview

Greptile Summary

This PR adds an important validation check before rotating external credentials to prevent a confusing scenario where credentials are changed on external systems but the rotation record creation fails.

Key Changes:

  • Added pre-rotation validation check (line 539-543) that verifies no conflicting secrets exist before calling rotationFactory.issueCredentials()
  • Made the tx parameter optional in $throwOnConflictingSecrets() helper function (line 219) to support both transactional and non-transactional calls
  • Kept the duplicate check inside the transaction (line 560-565) with advisory lock to handle race conditions
  • Updated code comments to clarify the validation strategy

Impact:
This prevents the problematic flow where external passwords would be rotated, then creation would fail with "secret already exists" error, leaving users with changed credentials but no rotation record in Infisical.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The change implements proper fail-fast validation with defense-in-depth strategy (checking both before and inside transaction). The optional parameter change is backward compatible and properly handled. Uses advisory locks to prevent race conditions. No security vulnerabilities introduced.
  • No files require special attention

Important Files Changed

Filename Overview
backend/src/ee/services/secret-rotation-v2/secret-rotation-v2-service.ts Adds pre-rotation validation check to prevent external credential changes before validation failures. Makes tx parameter optional in $throwOnConflictingSecrets helper function.

@victorvhs017 victorvhs017 merged commit a79f4eb into main Feb 18, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants