Support verbatim subject descriptors when writing bundle referrers#562
Merged
Conversation
cosign's WriteAttestationNewBundleFormat resolves the referrer's subject descriptor with a mandatory HEAD, which fails when the subject manifest doesn't exist in the target repository. The OCI statusmanager attests synthetic digests (layer blobs, pseudo-digests of nothing at all) into an override repository, so the subject is never there. Fork the referrer write path so callers can supply the subject descriptor verbatim via Statement.SubjectDescriptor. The registry and verify paths only ever key on the subject digest, so a digest-only descriptor is sufficient. A nil descriptor keeps cosign's strict HEAD semantics: a silent 404 fallback would mask auth failures on the Terraform resource paths, where the subject must exist. The fork mirrors cosign's manifest layout byte-for-byte (pinned by a parity test) and is temporary until upstream cosign grows a verbatim subject-descriptor option. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Cody Soyland <cody.soyland@chainguard.dev>
Comment on lines
+22
to
+25
| // referrerManifest augments v1.Manifest with the OCI 1.1 top-level artifactType | ||
| // field (which go-containerregistry's v1.Manifest does not model) and implements | ||
| // remote.Taggable so it can be PUT directly. It mirrors the unexported type of the | ||
| // same name in cosign's pkg/oci/remote so the serialized manifest is byte-identical. |
There was a problem hiding this comment.
these are extremely helpful and valuable for reviewers and future us, thanks for taking the time and documenting code so well
| return err | ||
| } | ||
|
|
||
| manifest := referrerManifest{ |
There was a problem hiding this comment.
nit: we could have a func that creates the manifest so that we can test it as it is rather large with many params
Contributor
Author
There was a problem hiding this comment.
Good idea. I extracted and separately tested it.
konradzapalowicz
approved these changes
Jun 11, 2026
konradzapalowicz
left a comment
konradzapalowicz
left a comment
There was a problem hiding this comment.
I do not see anything bad in it, ack. Left one suggestion but that is more like an observation so does not hold it
Review feedback on #562: the manifest literal inside writeBundleReferrer was large enough to deserve its own function. Extract it and pin the construction with a cmp.Diff against the expected layout. No functional change; the parity test pins the bytes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Cody Soyland <cody.soyland@chainguard.dev>
Replace reflect.DeepEqual and piecemeal field checks with cmp.Diff against complete expected manifests. The verbatim-subject test now pins every field of the written manifest, not just the subject, and failures print a real diff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Cody Soyland <cody.soyland@chainguard.dev>
The parity test could pass vacuously: it compared whatever referrers it found under the subject, so a fork referrer indexed elsewhere (or deduped into cosign's) was never actually diffed. Write each implementation into its own repository and require exactly one referrer on each side. Also verify the bundle blob round-trips through the registry, build the verbatim test's expected manifest with newReferrerManifest (the literal is pinned once, in TestNewReferrerManifest), and assert "size":0 on the subject descriptor specifically rather than substring-matching the whole manifest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Cody Soyland <cody.soyland@chainguard.dev>
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
2 participants
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.
Writing a bundle referrer currently requires the subject manifest to exist in the target repository: cosign's
WriteAttestationNewBundleFormatbuilds the referrer's subject descriptor by unconditionally HEADing the subject reference, so the write fails when the subject is absent — even though the OCI distribution spec explicitly allows referrer manifests whose subject does not exist.Downstream status tooling hits exactly that case: it attests synthetic digests (layer blobs, pseudo-digests of non-registry entities) into an override repository where the subject manifest never exists.
This adds
Statement.SubjectDescriptor, used verbatim as the referrer's subject when supplied, while nil keeps the strict HEAD behavior. The forked write path mirrors cosign's referrer manifest byte-for-byte (pinned by a parity test) and is temporary until sigstore/cosign#4940, which adds anociremote.WithSubjectDescriptoroption upstream, merges.Assisted by Claude Fable 5