fix(secrets): restore external delete for non-sync keys on writable Vault#3815
Closed
cursor[bot] wants to merge 0 commit intodevelopfrom
Closed
fix(secrets): restore external delete for non-sync keys on writable Vault#3815cursor[bot] wants to merge 0 commit intodevelopfrom
cursor[bot] wants to merge 0 commit intodevelopfrom
Conversation
ab25dec to
aa231cf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug and impact
Deleting an access key backed by a writable (non–read-only) Vault (or other external) storage did not call the backend delete when
synchronizedwas false. The Semaphore row was removed but the secret could remain in the external store—orphaned secrets and misleading UX (“deleted” in UI but still present in Vault).Root cause
In
AccessKeyServiceImpl.Delete, commit757e9e9c(refactor(secrets): vault sync mechanism) merged the read-only and synchronized cases intoif storage.ReadOnly || key.Synchronized, which skippedencryptionService.DeleteSecretfor any synchronized flag on writable storage. The intended behavior from the prior fix (52268c02) was: skip external delete for read-only storage, or for synchronized keys (local mirror only)—not skip external delete for writable non-sync keys.Fix
DeleteSecretonly whenstorage.ReadOnlyor when the key is synchronized on a writable backend (local reference removal only).DeleteSecretcalled; writable + synchronized →DeleteSecretskipped.Validation
go test ./services/server/... -shortgo vet ./services/server/...