feat(remote): cross-check the upstream's digest before the fill commits

🎯 What

Cross-checks the upstream's own Docker-Content-Digest against the bytes the fill staged, before the commit runs — on the fills that ask for it, and only those.

Third and last of three MRs extracted from !1325, which is kept as the combined reference rather than merged. This one is internal/remote and internal/format/oci plus two specs, and it carries a set of comment and message corrections to the seam the first two left behind — see the Corrections that ride along section.

🤔 Why

A tag resolution names no digest, so FetchOptions.ExpectedDigest has no reference value to work with there. The upstream's own Docker-Content-Digest is the only cross-check available, and today an upstream whose bytes disagree with its own stated identity fills the cache unnoticed.

The check's reach is as much of the design as its mechanics, and every blocking review finding across both rounds was about reach rather than mechanics. Two fills must not pay it, for the same underlying reason in each case: they already hold a reference value, or the header is not theirs to be judged by. The second round found a third — the by-digest manifest pull, which the manifest fetch helper was opting in on behalf of, and which is the first of the two.

🔨 How, and why the ordering matters

The comparison runs before storage.Session.Commit, not after.

Commit is terminal: it finalizes the staging object and leaves the session refusing Cancel with storage.ErrSessionTerminated. A comparison on its far side can report a mismatch but cannot discard the fill, so the contradicted blob survives unreferenced. S16 requires the staged fill be discarded exactly as a by-digest mismatch is, and the storage.Session.Commit contract states that verifying before finalization is what keeps a mismatch from producing an orphaned staging object.

The header is read only when the fetch asks, through FetchOptions.CrossCheckUpstreamDigest. It is off by default and nothing gets it implicitly. Docker-Content-Digest is defined by one format, and the other two have settled the question for themselves: S14 § Security Considerations makes a fill-time comparison against the upstream's declared sidecar "a possible follow-up, never a serving gate", and S15 puts the same comparison behind an observe-only log and states outright that a tarball is never refused on content grounds. Nothing about running the comparison needs format knowledge, which is why the gate is the caller's declaration rather than a sniff of the response.

No production fetch asks for it yet, and that is deliberate. oci.RemoteOperations.FetchManifest takes the opt-in as a parameter and decides nothing: the helper cannot tell a tag read from a by-digest read that passed no digest, so the decision sits with the caller that knows which it is making. Its one caller today is the by-digest fill, commitUpstreamManifest, reached only from fillByDigest — settleUncachedRead keeps the interim answer for a tag — and it asks for nothing. The fill that settles a tag against the upstream is the one that will ask, and S16's Follow-ups entry for the container fetch wiring is where that obligation now lives, next to passing the expected digest.

An earlier revision of this branch had the helper set the flag unconditionally. Against main as it stands, that would run the cross-check on the by-digest manifest pull and on nothing else — the class this MR argues must never pay it — turning the cache store's 503 UNAVAILABLE into a 500, and a correct body under a stale or index-level header into a permanent 500 where main answers 200. The parameter is what closes that.

FetchBlob asks for nothing either, because a blob is always addressed by digest. No Maven or npm fetch asks, and npm's tarball route says in place why it stays false.

The field crosses both composition seams, exactly as ExpectedDigest does: FlowOptions and StandaloneOptions carry it and both projections onto FetchOptions pass it through, so whichever of Fetch, StandaloneFlow.Deliver and Standalone.Serve the container fill path ends up composing can opt in. Each projection is held down by its own end-to-end test rather than by the field count alone.

Three things drop the claim even on an opted-in fetch, leaving the computed sha256 authoritative.

• A request that named a digest. ExpectedDigest is already the reference and Commit verifies the staged bytes against it, so the upstream's claim can establish nothing further — agreeing or not. Consulting it there could only ever fail a fill whose bytes are provably the ones asked for, and permanently, since no retry changes a header. This is what S16 § Digest verification, § API Contracts and § Cache integrity and body caps all say in their own words, and what S12 does on the hosted side, where only the request-path or ?digest= value is ever the reference.
• A Transform. The staged bytes are then ones the upstream never sent, so its claim describes a different document.
• A claim the fill cannot read. An absent header, one digest.Parse rejects, or one naming an algorithm other than sha256. A fill is failed by a claim that disagrees and never by one it could not make sense of. The algorithm arm matters because digest.Parse accepts sha384 and sha512, and a well-formed sha512 claim — truthful about the very bytes being staged, even — can never equal the sha256 it would be compared against, so comparing it would fail the path on every retry and blame the upstream for an algorithm ADR-008 says this service does not support. The parse arm covers the upper-case sha256 hex oci.ParseDigest accepts and canonicalizes on the request side; mustBeCanonicalDigest records the same grammar asymmetry for that side.

A by-digest read that passes no ExpectedDigest cannot be told apart from a tag read at this seam, which is why the opt-in is the fetching slice's declaration rather than something the fill infers. Passing the digest is what moves the verdict onto the commit, and S16's Follow-ups entry is where that obligation lives.

The claim never reaches a caller as a header or a result field. A format slice needs the verdict rather than the value, and gets it as the fill's failure; the forwardable-header allowlist is unchanged. The failure message does name the claimed digest alongside the staged one, so a slice mapping the error can quote it in a detail. S16 agrees on the surface: the upstream's value is "a cross-check only, never the stored value", and the response header is "service-generated, not relayed".

The upstream's claim is never Commit's expected-digest input. The two stay separate checks sharing one sentinel, because a composing flow answers both the same way.

🧹 Corrections that ride along

None of these changes what the seam does.

The expected-digest seam this builds on left six things wrong or missing.

• SingleFlight.Fetch's form check gave two reasons that are both wrong. The wrapped Fetcher does not make the same call for a follower, which returns from joinFollower and never reaches s.inner.Fetch — the one re-entry, refetchVanishedRow, fetches under the leader's options. So that line is the only seam that ever inspects a follower's own digest, which makes it critical rather than a courtesy repeat. And no flight is left behind either way: runLeader's deferred recovery releases the fill slot and deregisters before re-panicking. What the check does buy, besides a well-formed value for the segment test, is running ahead of the registration and the slot acquire, so a rejected call fails no concurrent follower with ErrLeaderPanicked.
• No test at that layer could tell whether the check was present. Deleting it left the package green but for one panic-message assertion: the existing malformed-digest cases each build their path from the malformed value, so the segment test lets the call through, the call reaches the wrapped Fetcher as a leader, and that Fetcher's own check raises the identical panic value. A follower cannot reach that Fetcher, so TestSingleFlight_Fetch_FollowerMalformedExpectedDigest_Panics is the case that pins it — the leader holds its flight open across the rejected call, and the assertions hold the route down as well as the panic.
• The segment guard panicked on a bare constant, so a caller that tripped it learned only that the two disagreed, not which one it got wrong. It now names both, as the two form panics six lines above already do.
• mustBeCanonicalDigest cited "the container cache-key parser", and two parsers answer to that name: datastore.parseContainerRemoteCacheKey refuses a sha512 reference, while oci.parseRemoteCacheKey reads it as a tag. Named the one that does.
• The Transform-pairing test called its guard Fetch's first statement. The first statement is the ExpectedDigest != "" test; what the test actually asserts is that the guard runs before any request is issued.
• StandaloneOptions.ExpectedDigest pointed at one of Fetch's two panics, and this struct reaches both. It now points at the full contract, as FlowOptions.ExpectedDigest already did.

Four more came out of review, on the cross-check itself.

• ErrUpstreamDigestMismatch's doc promised an observability record only one of its two arms has. The paragraph tells a composing flow it needs no count of the detection, because the upload session's terminal path observes upload_session_duration_seconds{outcome="digest_mismatch"} and emits the paired wide event with both digests. That holds for the Commit arm. The pre-commit arm reaches Cancel, whose terminal emit is outcome="cancelled" with both digest fields empty — the record a client disconnect or an abandoned fill leaves — so nothing below names the mismatch and a flow that wants it visible has to emit it itself. The doc now says which arm is which.
• The claim-drop enumeration did not place an upper-case sha256 claim. "Unparseable" is where it lands, and that is a form this service's own OCI grammar accepts on the wire, so the silence read as the check covering it. Named in the doc comment and in S13.
• S13 said the claim is "never exposed to a caller" without saying which surface. True of the header and the result field; the failure message interpolates it deliberately. Narrowed.
• S16's Follow-ups tied the 503 mapping to the by-digest wiring. remote.ErrUpstreamDigestMismatch reaches the container error mapper from the tag cross-check as much as from a by-digest commit, and isRefusedUpstreamContent lists neither, so the mapping arm is owed whether or not any slice passes ExpectedDigest. Stated that way now, so a slice author deferring the digest does not defer the mapping and ship a 500 on the tag path.

Six more came out of the second review round, five of them on this MR's own delta.

• The by-digest claim-drop was pinned by no test. TestFetcher_Fetch_ExpectedDigest_DropsUpstreamClaim_Commits never set CrossCheckUpstreamDigest, so the guard returned on its first clause and opts.ExpectedDigest != "" decided nothing — deleting that clause left the suite green. Both subtests now set the opt-in, which is what puts the clause in charge of the outcome. The same doc comment's claim that dropping the claim unread "keeps the second hash pass off every blob-sized fill" is gone too: no blob-sized fill opts in, so the default is what does that.
• Neither seam projection was asserted. Deleting either StandaloneOptions.flowOptions's line or StandaloneFlow.fetchAndDispatch's left internal/remote, internal/format/oci and internal/format/npm all green, and TestFlowOptions_CarriesEveryFetchOption says in its own doc why a field count cannot catch that. Both now drive a fill the upstream contradicts and read the verdict off the session.
• The claim-drop table had no fixture for upper-case sha256 hex, the one form the doc comment and S13 both single out and the one an upstream can plausibly emit, since this service's own OCI grammar accepts it on the wire. digest.Parse rejects it, so the fill commits on the computed value.
• Fetch's doc read as though both checks run. They exclude each other — an opted-in fetch carrying an expected digest drops the claim unread — and this method doc is what a caller composing Fetch reads. One line was also left unwrapped at 94 columns.
• SingleFlight's follower-inheritance list omitted the new field, and the paragraph below it called ExpectedDigest "the one option" with that hazard. The boolean shares it in both directions and no panic can guard it, because nothing in the path key says what its value should be. S13 carried the uniformity rule for expected_digest and now carries it for this flag too.
• The manifest fill's doc counted the obligations its S16 Follow-ups entry assigns. That entry now lists asking for the cross-check on the tag fill as well, so "one of two" is wrong where it is read.

Three stale lines in docs/plans/2026-07-30-container-remote.md are not corrected here: they tell Steps 14 and 15 the disagreeing-header criterion is fully gated and that an allowlist exemption alone would not be enough. Step MRs do not edit the plan file, and !1831 is the standing single writer for it.

✂️ Comment caps

main landed hard per-site line caps on Go comment blocks after this branch had merged it — scripts/ci/check-comment-caps.sh plus docs/dev/go-style.md § Comment caps, enforced as the lint:comment-caps job and as a pre-commit hook, on the blocks a diff touches. Merging current main brought the gate in and this branch failed it: 41 blocks, 547 comment lines against an allowance of about 85. 326 of those lines were this branch's own; 221 were older prose in blocks it happened to touch. The pass deletes 584 comment lines and adds 75.

This was a red pipeline, not a style preference. lint:comment-caps is not allow_failure, and it failed pipeline 2790889137 as the only failing job, so the caps had to be met before this merge request could go anywhere.

Overflow is deleted rather than relocated, following the precedent of style(comments): fit the container remote bulk pass to the comment caps, whose own message records the ruling that docs/dev pages are written intentionally and are not a comment-overflow home. What survives each block is the claim a reader of that symbol needs, plus a pointer to where the reasoning already lives: S13 § Fetch for the seam's behavior and the whole claim-drop set, S13 § Single-flight on Fetch for the coalescing rules and the panics, S16 § Digest verification for the pre-commit ordering, S16 § Follow-ups for what the container arm still owes.

Little is lost, because the specs carried nearly all of it before this pass. Every rule in SingleFlight.Fetch's 56-line doc is in S13 § Single-flight on Fetch. The claim-drop set is in S13 § Fetch. The blob's after-commit comparison is in S16 § Follow-ups. The metric names and outcome values are in docs/dev/storage.md. What does go is the rationale for design choices — why the caller decides the opt-in, why a by-digest read must never pay the cross-check, which mutation each test catches. That argument is this merge request and its threads, and it is the history of a decision rather than a standing rule, which is what a comment holds worst.

One claim had no spec home and is promoted rather than dropped. The pre-commit mismatch arm reaches the upload session's cancel path, whose terminal emit carries outcome="cancelled" with both digest fields empty — the record a client disconnect also leaves — while only the commit's own comparison emits outcome="digest_mismatch" with both values. A composing flow that wants the pre-commit detection visible has to emit that signal itself. Left unwritten, every such flow under-reports the refusals it serves. S13 § Fetch now states it, in the section this branch already edits.

Three more had no spec home and are compressed to their facts, losing their reasoning: FetchResult's nil-body-on-every-non-stream-hit pairing, the follower outcome and error taxonomy on SingleFlight.Fetch, and that a post-commit upsert failure surfaces from the streamed body's Read and must be classified cache-side.

Two consequences worth naming rather than leaving to be found.

• Fetch's pointer is re-aimed. The round-three fix pointed at FetchOptions.CrossCheckUpstreamDigest for the full claim-drop set. That block is itself capped at 2 lines and can no longer hold a list, so the pointer now names S13 § Fetch, which is the canonical statement and where the list already sits.
• commitUpstreamManifest loses its call-site justification. Its cap covers the doc comment and the body head together at one line, so the explanation of the false it passes cannot stay at the call site in any form. The one-line doc names S16 § Digest verification, which carries it.

Comment-only, and mechanically so: every one of the 14 Go files in that commit has a byte-identical go/scanner token stream across it, and no //nolint, //go: or build-tag line was added, dropped or altered. The 15th file is docs/specs/S13-virtual-remote-foundation.md, carrying the one promoted claim. The reviewable-line count below is unmoved, because it never counted comments.

📚 Spec coverage

Specs: docs/specs/S13-virtual-remote-foundation.md, docs/specs/S16-container-remote.md

Acceptance criteria

Criterion	Tests
S16 § Cache integrity and body caps — on a tag fill the upstream's claim becomes neither the commit's reference nor the stored blob ref	TestFetcher_Fetch_TagDockerContentDigest_MatchesOrIsAbsent_Commits/matching_upstream_digest
S16 § Cache integrity and body caps — "Disagreeing upstream digest header is never cached"	TestFetcher_Fetch_TagDockerContentDigestMismatch_DiscardsStaging
S16 § Manifest and blob proxy — an upstream sending no digest still fills	TestFetcher_Fetch_TagDockerContentDigest_MatchesOrIsAbsent_Commits/absent_upstream_digest
S16 § Digest verification — the header is not consulted on a by-digest read, agreeing or not	TestFetcher_Fetch_ExpectedDigest_DropsUpstreamClaim_Commits (both subtests)
S13 § Fetch — the cross-check is a per-fetch opt-in, off by default	TestFetcher_Fetch_WithoutCrossCheckOptIn_IgnoresUpstreamClaim, TestFetchBlob_UsesTheBlobCapWithNoOverride
S13 § Fetch — the opt-in is the caller's declaration, carried through unchanged	TestFetchManifest_PassesTheCrossCheckOptInThrough (both values), TestRemoteManifestServe_ByDigestMiss_AnswersFromTheCommittedRow
S13 § Fetch — the flag reaches the fill across both composition seams	TestStandaloneFlow_Deliver_CarriesCrossCheckOptInToTheFill, TestStandaloneOptions_CarriesCrossCheckOptInToTheFill
S13 § Fetch — a claim the fill cannot read leaves the computed value authoritative	TestFetcher_Fetch_UnreadableUpstreamDigestClaim_DropsTheClaim (unparseable, non-sha256, and upper-case sha256 hex), TestFetcher_Fetch_TransformedFill_DropsUpstreamDigestClaim

Error cases

Condition	Tests
An upstream 200 whose claim contradicts the fill fails as an integrity error naming both digests, stays outside ErrCacheFill, and caches nothing	TestFetcher_Fetch_TagDockerContentDigestMismatch_DiscardsStaging
A contradicting claim on a fill that named a digest, or on one that did not opt in, commits normally	TestFetcher_Fetch_ExpectedDigest_DropsUpstreamClaim_Commits/claim_contradicting_the_requested_digest, TestFetcher_Fetch_WithoutCrossCheckOptIn_IgnoresUpstreamClaim

Security considerations

Concern	Tests
The claim is cross-checked before any blob is finalized or any cache entry can reference it	TestFetcher_Fetch_TagDockerContentDigestMismatch_DiscardsStaging
A foreign-algorithm claim cannot render a path permanently unpullable	TestFetcher_Fetch_UnreadableUpstreamDigestClaim_DropsTheClaim/well-formed_non-sha256_claim
A format that does not define the header cannot be failed by it	TestFetcher_Fetch_WithoutCrossCheckOptIn_IgnoresUpstreamClaim
A correct body cannot be refused over the upstream's own bookkeeping	TestFetcher_Fetch_ExpectedDigest_DropsUpstreamClaim_Commits/claim_contradicting_the_requested_digest, TestRemoteManifestServe_ByDigestMiss_AnswersFromTheCommittedRow
A follower's own expected digest is rejected at the coalescing seam rather than dropped unchecked	TestSingleFlight_Fetch_FollowerMalformedExpectedDigest_Panics

Not covered here

One half of S16's "Disagreeing upstream digest header is never cached" is outstanding: the 503 UNAVAILABLE + Retry-After mapping, which isRefusedUpstreamContent does not yet answer for remote.ErrUpstreamDigestMismatch.

The store-and-serve half of "Tag fill stores and serves the computed digest" is outstanding for the same reason, and the acceptance row above is narrowed to what its test reaches. That criterion asserts three container-slice facts — the computed sha256 reaches container_remote_manifests.digest, it is the response Docker-Content-Digest, and a following by-digest GET hits that row — and no test here can reach them while settleUncachedRead answers a tag with 501. What the named test pins is the part this MR changes: the upstream's claim becomes neither the commit's expected-digest input nor the blob ref the cache row carries.

The revalidation half — "on a revalidation the tag keeps pointing at the manifest it already named, and upstream_checked_at is not bumped" — is already held at this seam: the cross-check runs before Commit, so UpsertCacheEntry is never reached, and the only BumpUpstreamCheckedAt call in internal/remote is in the 304 handler, which a 200 never enters.

The mapping belongs to the remote manifest handler, and both it and its miss-and-fill path are on main — fillByDigest and commitUpstreamManifest, wired through oci.WithManifestFill at cmd/artifact-registry/wire_oci_remote.go. What keeps the 503 out of scope here is not that the code is absent but that nothing reaches the sentinel: no production fetch asks for the cross-check, so remote.ErrUpstreamDigestMismatch cannot arrive at the mapper from this seam, and no status moves either way. The fetch that asks is the one that owes the mapping with it — which is how S16's Follow-ups entry now reads.

✅ Verification

Command	Result
go build ./...	clean
go vet ./... and go vet -tags=integration ./...	clean
go test ./...	clean, with GOOGLE_APPLICATION_CREDENTIALS set so the three TestWireStorage_CloudCDN* tests can build their GCS driver; without it those three fail identically on main
go test -tags=integration ./internal/remote/...	3 packages ok
golangci-lint run --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false ./internal/remote/... ./internal/format/oci/... ./internal/format/npm/...	0 issues
scripts/ci/check-comment-caps.sh --base origin/main	OK — every block a diff touches is within its cap
gofmt -l internal/	clean

Mutations confirmed red and restored by checksum: removing the CrossCheckUpstreamDigest gate, hardcoding either value in FetchManifest, re-admitting the claim on the ExpectedDigest arm, deleting either seam projection's line, inverting the agreement check, and dropping the err != nil, the non-sha256 and the Transform arms of the claim-drop condition.

The ExpectedDigest-arm and seam-projection mutations are the ones the second review round found unpinned; both were measured green before the fix and red after.

🔄 Merge order

Nothing gates this. !1806 (merged) and !1808 (merged) have both merged, so the diff against main is the cross-check and its corrections alone. Shedding their squash-merged copies took a synthetic merge base — git commit-tree with main's tree and both histories as parents, then a rebase onto it — so the branch history reaches those commits while contributing none of their content. main has since been merged in, which is what brought fillByDigest and commitUpstreamManifest into the tree and made the opt-in's reach measurable against a real caller.

Two open MRs touch the same files and will need one line each: !1873 and !1875 both call FetchManifest, which now takes a fourth argument, and the tag arm each of them lands is where true belongs. Whichever merges second carries that. !1873 also states in a doc comment that the upstream's Docker-Content-Digest is not readable yet, which this MR makes false.

MR	Scope	State
!1806 (merged)	spec corrections against what main already ships	merged
!1808 (merged)	FetchOptions.ExpectedDigest reaching the cache-fill commit	merged
this one	the upstream Docker-Content-Digest cross-check	open
!1325	the combined reference these three were extracted from	never merged; close once this lands

📏 Size

435 reviewable Go lines, under the 500 the development model asks a description to answer for. Unchanged by the comment-caps pass, which deleted 584 comment lines and counts none of them. Tests carry 385 of them: fetch_test.go 263, standaloneflow_test.go 48, remote_operations_test.go 37, singleflight_test.go 32, remote_manifest_test.go 5. Production Go is 50: fetch.go 35, remote_operations.go 5, and two apiece in standaloneflow.go, standalone.go, singleflight.go and remote_manifest.go, one each in operations.go and npm's tarball route. errors.go changes comments only and counts zero.

🧪 e2e scenarios

No scenario is added or affected. Both docs/testing/e2e/oci.md and docs/testing/e2e/docker.md list "Virtual and remote (proxy/cache) repositories" under "Out of scope until the capability ships".

Related to #433