fix: honor --attachment-tag-prefix in cosign clean - #5060
Open
hamodywe wants to merge 1 commit into
Open
Conversation
CleanCmd built its own ociremote options with only WithRemoteOptions, so the tag prefix was never applied. SignatureTag, AttestationTag, SBOMTag and ResolveDigest then resolved the default sha256-<digest>.sig/.att/.sbom tags, and a clean run that passed --attachment-tag-prefix deleted the unprefixed attachments instead of the ones it named. sign and attest apply the prefix when they push these tags, so clean has to apply it when it resolves them. Deleting an attachment the caller did not name is not recoverable, which is why this is worth fixing even though the flag is deprecated in favour of OCI referrers. Fixes sigstore#4344 Signed-off-by: hamodywe <iosapk.org@gmail.com>
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
1 participant
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.
Fixes #4344.
Summary
cosign clean --attachment-tag-prefix=<p>deleted the unprefixed attachments instead of the prefixed ones it was asked to remove.CleanCmdbuilds its own ociremote options:That carries the registry/transport options but never
RefOpts.TagPrefix, soSignatureTag,AttestationTag,SBOMTagandResolveDigestall resolve the defaultsha256-<digest>.sig|.att|.sbomtags.sign/attestdo apply the prefix when they push those tags, socleanwas looking at a different set of tags than the ones it created.This applies the prefix when it is set, leaving the no-prefix path byte-identical.
Why it is worth fixing a deprecated flag
The flag is deprecated in favour of OCI referrers, but the failure mode is deleting an attachment the caller never named, and a registry delete is not recoverable. That seemed worth a small fix rather than leaving it until removal.
Scope
Deliberately narrow: only the prefix is threaded through. I did not switch
CleanCmdtoregOpts.ClientOpts()even though that would be the tidier call, because it would additionally start honouringCOSIGN_REPOSITORYhere — a behaviour change that overlaps #5037/#5033, where the target-registry semantics are still being defined. Happy to fold that in if you would rather it moved in one step.Tests
cmd/cosign/cli/clean_test.gocovers both directions: with--attachment-tag-prefix=cve-the resolved tags arecve-sha256-….sig|.att|.sbom, and with no prefix they stay exactly as before. Both fail against the pre-fix behaviour for the prefixed case and pass after.go build ./cmd/...,go vet ./cmd/cosign/cli/and the new tests are clean locally.