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/Range and manifest GET/HEAD/PUT, each asserting the header field and its provenance flag are populated while Digest.Advertised stays empty and Digest.Match false, which is the split the bind-to-the-200 rule 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 GET case pins the zero Digest beside a true DockerContentDigestFromRedirect, which is the pair the old placement could not express
  • OCI-Subject carried forward on manifest PUT, and staying empty when no hop sets it
  • the redirected-PUT stage 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
  • Location staying empty on a redirected PUT, pinning that it is read off the final response only
  • headerAcrossRedirects exercised directly on six chain shapes, including Request == nil and Request.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_NamesTheHopWhenTheDigestComesOffARedirectHop stages a registry answering the blob GET with a 307 carrying the header and a storage hop carrying none, and asserts three things about oci.blob.download: it fails, its message names the digest the 307 advertised, 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_NamesTheRedirectHopInTheMismatch pins the provenance clause on the OCI-Subject mismatch.
  • TestDockerContentDigest_KeywordBearingDigestDoesNotReachJUnit drives 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-Digest across 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-Digest response header, §subject field (v1.1), the blob GET 3xx header row, the blob HEAD status 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 HEAD header row scopes Content-Length out of the fallback: testBlobHead checks it before the digest verdict, so a redirect-served HEAD whose storage hop omits it stays red on the length check.
  • Three §Resolutions entries: where registries actually put the header (with distribution and Container Registry blobserver.go permalinks), the four things this MR decides, and the gate check recording that the rows stay bound to the final response.

Closes #57 (closed)

Edited by Sylvia Shen

Merge request reports

Loading
Loading