fix(secret-v2-bridge): move/delete personal overrides when moving shared secrets#6078
fix(secret-v2-bridge): move/delete personal overrides when moving shared secrets#6078PuneetKumar1790 wants to merge 2 commits intoInfisical:mainfrom
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad7148eb62
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }) | ||
| .whereIn("key", secretKeys) | ||
| .update({ folderId: destinationFolderId }) |
There was a problem hiding this comment.
When moveSecrets overwrites into a destination that already has personal overrides for the same key/user, this bulk update({ folderId: destinationFolderId }) blindly relocates source overrides and can create duplicate personal rows in the destination folder. That violates the one-override-per-user/key assumption used by secret resolution (the priority map only keys by key-folder and will pick one personal row based on query order), so users can see stale or nondeterministic override values after a move. Please resolve destination conflicts (e.g., upsert or delete/replace per key + userId) before updating folder IDs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Handle override key/user conflicts when moving folders
When
moveSecretsoverwrites into a destination that already has personal overrides for the same key/user, this bulkupdate({ folderId: destinationFolderId })blindly relocates source overrides and can create duplicate personal rows in the destination folder. That violates the one-override-per-user/key assumption used by secret resolution (the priority map only keys bykey-folderand will pick one personal row based on query order), so users can see stale or nondeterministic override values after a move. Please resolve destination conflicts (e.g., upsert or delete/replace perkey + userId) before updating folder IDs.Useful? React with 👍 / 👎.
Thanks for catching this! I've addressed the conflict scenario in the latest commits.
What I added:
- When moving source personal overrides to destination, destination overrides for the same keys are now deleted first
- This prevents creating duplicate override rows and maintains the one-override-per-user/key invariant
- Added a test case verifying this conflict resolution behavior
Changes:
movePersonalOverridesnow deletes conflicting destination overrides before moving source ones- New test: "should delete conflicting destination overrides before moving source overrides"
All tests pass and lint is clean. The fix ensures source overrides safely replace any pre-existing destination overrides without creating duplicates.
…onal overrides When moving source personal overrides to destination, delete any existing destination overrides for the same keys first. This prevents creating duplicate personal override rows and maintains the one-override-per-user/key invariant used by secret resolution logic.
2 participants
Context
Fixes data integrity issue where personal overrides could be left orphaned when moving shared secrets (
#4627).Additionally addresses conflict handling when destination already has personal overrides for the same keys (raised by code review).
What changed
Personal override move/delete behavior:
moveSecretsto:Conflict resolution:
Tests:
Steps to verify the change
node --max-old-space-size=8192 ./node_modules/.bin/eslint --ext ts src/services/secret-v2-bridge/secret-v2-bridge-dal.ts src/services/secret-v2-bridge/secret-v2-bridge-service.ts src/services/secret-v2-bridge/secret-v2-bridge-move-overrides.test.ts --max-warnings 0npm run -s test:unit -- src/services/secret-v2-bridge/secret-v2-bridge-move-overrides.test.tsType
Checklist