fix(oci): read digest and subject headers across a redirect hop
What
headerAcrossRedirects reads a content-describing response header from
the final response and falls back to the nearest redirect hop that
carried one. Two headers go through it: Docker-Content-Digest on blob
GET/HEAD/Range and manifest GET/HEAD/PUT, and OCI-Subject
on manifest PUT.
The fallback feeds the failure message, not the verdict. Catalog rows
oci.blob.download, oci.blob.head and oci.http.docker-content-digest
require the header on the response they address, so a hop-only value
still fails them; what changes is that the failure names the value the
registry did advertise and says which hop carried it.
Four exported fields are added, and four change semantics — see §Exported API change below.
Why
The client read the header off the response it got after following a
blob-GET redirect. A registry that redirects blob reads to object storage
sets the header on the 3xx; the storage backend does not repeat it. So
the client saw an empty value and reported a conformant registry as
omitting the header.
Seven hosted rows failed against Artifact Registry staging for this
reason alone: oci.blob.upload-monolithic, upload-single,
upload-chunked, download, cross-repo-mount,
oci.http.docker-content-digest, oci.concurrency.blob-uploads.
Those seven rows stay red after this MR. An earlier revision let the
hop's value satisfy the verdict, which would have turned them green; the
rows bind the header to the 200, so that was a loosening the catalog
does not admit, and §Resolutions now records why no amendment route is
available for it. What this MR fixes is the other half of #57 (closed): the
message. Instead of reporting that the registry advertised nothing, the
row now names the digest the registry sent and says it was on a redirect
hop, which is the difference between an operator filing a registry bug
and filing one against this suite.
The read policy also made oci.remote.head-parity order-dependent: green
on a fresh run ID, red on a repeat once the blob had moved to the CDN.
Scope of the fallback
Two headers get it, both because they describe the content the request
addressed, which no hop can change. OCI-Subject needs it for a second
reason: an absent header is read as "this registry does not implement
v1.1", which skips the referrers rows rather than failing them, so a
redirected PUT would silently stop testing.
Content-Type and Content-Length get no fallback — they describe the
bytes the final hop actually sent. Location gets none either: it names
the next hop, so a walk would return the redirect target in place of the
resource URL.
A value on the final response wins, so a registry that serves blobs inline, and one that sets the header on both hops, behave exactly as they did before. When no hop carries it, there is no value to name and AC #26 (closed) reports the omission rather than the placement.
The chain is walked through http.Request.Response, which net/http
populates with the redirect response that caused a request to be created.
It is nil outside a client redirect, so the loop is a single iteration on
the common path.
Exported API change
No !: the change is the fix, and the previous semantics are what this
MR argues were never the contract. But the Artifact Registry's own
integration tests import this package, so the change is disclosed here.
Four fields can now hold a value that was never on the response the
caller holds, with no change in type or signature:
BlobResponse.DockerContentDigest,
ManifestResponse.DockerContentDigest,
PutManifestResponse.DockerContentDigest, and
PutManifestResponse.OCISubject. A consumer reading any of them to
assert "the final response carried header X" now gets a different answer
for identical registry behavior.
DigestVerdict.Advertised is not among them, and that is the one
field where an earlier revision of this MR differed: it holds the value
the final response carried itself and is empty when only a hop did, which
is what it meant before this branch. Read it to learn what the 200
sent; read DockerContentDigest beside its flag to learn what any hop
sent.
Four new fields answer that question:
DockerContentDigestFromRedirect on all three response structs, and
OCISubjectFromRedirect on PutManifestResponse. Each field's doc
comment points at its flag.
The provenance lives on the response structs rather than on
DigestVerdict, which is where an earlier revision put it. DigestVerdict's
contract is that a zero value means no verdict, and a flag there created
two holes: a Range GET populates the header from the redirect hop but
computes no verdict, so the flag read false; and OCISubject has no
verdict to hang a flag on, so it got no provenance at all. Beside the
header, both close. The cost is four fields instead of one.
newDigestVerdict is unexported. It takes three arguments now, the two
headerAcrossRedirects returns plus the expected digest, so the
bind-to-the-final-response rule lives in one place and every call site is
compiler-checked against it.
Message redaction
A registry answering Docker-Content-Digest: token: <secret> landed that
value verbatim in stdout and in the JUnit <failure message> attribute,
at thirteen sites. internal/report's redactMessage applies only
RedactURLUserinfo and deliberately not ScanContent, so
scrubForMessage at the point the bytes enter the message is what closes
it.
The durable cause was the guard, not the sites:
internal/messagescan matches on the trailing selector name, so
resp.Digest.Advertised presented as Advertised and classified clean
while resp.DockerContentDigest did not. Advertised and OCISubject
join its OCI selector list, and the thirteen reads it then flags are
wrapped. Behaviour-neutral for conformant values: a hex digest passes
ScanContent byte-for-byte.
The OCI table was also the only one of the three format modules that did
not name Content-Type, so a registry answering
Content-Type: application/json; token=<secret> landed that parameter
verbatim in the JUnit <failure message>. ContentType,
FiltersApplied and MediaType join the selectors, and contentType
and mediaType the identifiers. The identifiers are what reach
checkCleanMediaType, whose renders read a parameter rather than a
field; npm's table already names mediaType for that shape. Every site
the widened guard then reports is wrapped, including one the review did
not list (index.MediaType in testReferrersTagSchemaFallback).
Effect on Artifact Registry staging
No row's verdict changes. The seven hosted rows listed under §Why fail
before and after, because Artifact Registry advertises the digest on the
3xx only and the rows require it on the 200.
What changes is what those failures say. A row that reported an empty
Docker-Content-Digest now reports
Docker-Content-Digest "sha256:…" was advertised on a redirect hop, not on the final response, which is where this row requires it.
An earlier revision of this MR did flip those rows green (oci.blob.*
11/11, oci.http.* 3/3, oci.concurrency.* 3/3 against staging). That
result is withdrawn: it depended on the hop-only value satisfying the
verdict, which §Resolutions now records as a loosening the catalog does
not admit.
oci.remote.head-parity was order-dependent for the same underlying
read: green on a fresh run ID, red on a repeat once the blob had moved to
the CDN, failing with the blob GET carried an empty Docker-Content-Digest, so its HEAD has no value to agree with. Its
message improves the same way; its verdict does not change either. That
row is not reproducible from this MR: remote-plan Step 33 has not
landed, so the slug has no implementation and no catalog row here. It was
observed on the unmerged local branch local-throwaway-oci-remote-run
(pkg/conformance/oci/remote_head.go).
Tests
pkg/client/oci/digest_redirect_test.go:
- carry-forward on blob
GET/HEAD/Rangeand manifestGET/HEAD/PUT, each asserting the header field and its provenance flag are populated whileDigest.Advertisedstays empty andDigest.Matchfalse, which is the split the bind-to-the-200rule creates - the final response wins when both hops advertise, staged with a different value on each hop so a pass cannot be explained by the two agreeing
- empty stays empty when no hop advertises, and both provenance flags stay false rather than pointing at a hop that carried nothing
- the Range
GETcase pins the zeroDigestbeside a trueDockerContentDigestFromRedirect, which is the pair the old placement could not express OCI-Subjectcarried forward on manifestPUT, and staying empty when no hop sets it- the redirected-
PUTstage captures the replayed body and every PUT test asserts it equals what was pushed: nothing else those tests assert observes the push, so all of it holds against an empty one Locationstaying empty on a redirectedPUT, pinning that it is read off the final response onlyheaderAcrossRedirectsexercised directly on six chain shapes, includingRequest == nilandRequest.Response == nil, which no httptest redirect can produce and where a nil-deref would panic rather than fail, plus a chain where the nearest and outermost hops carry different values
pkg/conformance/oci, at the layer that reported the red:
TestTestBlobDownload_NamesTheHopWhenTheDigestComesOffARedirectHopstages a registry answering the blobGETwith a307carrying the header and a storage hop carrying none, and asserts three things aboutoci.blob.download: it fails, its message names the digest the307advertised, and its message does not say "does not match" (the two digests are equal in that shape, so a mismatch sentence would print the same string twice and read as a bug in the suite).TestTestReferrersSubjectField_NamesTheRedirectHopInTheMismatchpins the provenance clause on theOCI-Subjectmismatch.TestDockerContentDigest_KeywordBearingDigestDoesNotReachJUnitdrives the Fn against a registry advertising a credential-bearing digest and asserts it reaches neither the failure message nor the rendered JUnit file.
Every server-backed test asserts the far hop was reached exactly once, so a passing value cannot come from a redirect that was never followed.
Verified the suite is not vacuous: reducing the walk to the final
response fails both multi-hop chain cases and the two conformance-layer
redirect tests, and every carry-forward test's header-field and
provenance assertions; removing the Request == nil guard fails the case
named for it; dropping either scrub fails the render test; letting the
verdict take the hop's value fails the five carry-forward
Digest.Advertised assertions and the blob-download row test.
Spec coverage
| Spec row | Covered by |
|---|---|
AC #5 (closed) (blob GET) |
TestGetBlob_dockerContentDigestCarriedForwardFromRedirect, TestTestBlobDownload_NamesTheHopWhenTheDigestComesOffARedirectHop |
AC #6 (closed) (blob HEAD) |
TestHeadBlob_dockerContentDigestCarriedForwardFromRedirect |
AC #10 (closed) (manifest GET) |
TestGetManifest_dockerContentDigestCarriedForwardFromRedirect |
AC #11 (closed) (manifest HEAD) |
TestHeadManifest_dockerContentDigestCarriedForwardFromRedirect |
AC #19 (closed), #20 (closed) (OCI-Subject) |
TestPutManifest_ociSubjectCarriedForwardFromRedirect, TestPutManifest_ociSubjectEmptyWhenNoHopSetsIt, TestTestReferrersSubjectField_NamesTheRedirectHopInTheMismatch |
AC #26 (closed) (oci.http.docker-content-digest, including the manifest PUT response header) |
all of the above, plus TestPutManifest_dockerContentDigestCarriedForwardFromRedirect, and TestDigestSourceSuffix / TestDcdCheckMatch_reportsTheHopThatCarriedTheValue for the two provenance messages |
S07 §Docker-Content-Digest across the redirect |
TestHeaderAcrossRedirects_chainShapes |
The Range GET carry-forward is covered by
TestGetBlobRange_dockerContentDigestCarriedForwardFromRedirect but has
no AC row: AC #29 (closed) is about 206, Content-Range, and the first ten
bytes, and testBlobRangeRequest reads no digest field.
Spec
Seven passages in S07 change, all keeping the fallback a client read policy rather than a loosened requirement, and all now agreeing with the code that the verdict binds to the final response.
Catalog rows oci.blob.download, oci.blob.head, and
oci.http.docker-content-digest bind the header to the 200. The spec
used to route a loosening to docs/catalog/README.md §Amending a catalog
row; it no longer does, because that section's gate does not admit the
case. §Resolutions now records which of the six admissible justifications
were considered and why the only arguable one, "the row makes a
demonstrably wrong claim about OCI reference behavior", fails on this
spec's own sourced finding that distribution and Container Registry set
the header on neither hop.
- §AR-style blob redirect delivery gains
Docker-Content-Digestacross the redirect: where the client looks, why only these two headers get the fallback, and that an omission by every hop is still reported. - §
Docker-Content-Digestresponse header, §subjectfield (v1.1), the blobGET3xxheader row, the blobHEADstatus and header rows, and AC #26 (closed) each state the read policy and point at the catalog for what a registry MUST send. - The blob
HEADheader row scopesContent-Lengthout of the fallback:testBlobHeadchecks it before the digest verdict, so a redirect-servedHEADwhose storage hop omits it stays red on the length check. - Three §Resolutions entries: where registries actually put the header
(with
distributionand Container Registryblobserver.gopermalinks), the four things this MR decides, and the gate check recording that the rows stay bound to the final response.
Closes #57 (closed)