feat(remote): carry the request's digest to the cache-fill commit

🎯 What

Carries the digest a request named through to the cache-fill commit, so a body that disagrees with it is discarded instead of cached.

Second of three MRs extracted from !1325, which had grown to 16 files across four packages and is now kept as the combined reference rather than merged. This one is internal/remote plus two spec edits, and one line in the npm tarball route — main's standalone read state machine landed a parity guard between FetchOptions and FlowOptions while this branch was open, so the new field has to cross all three projections onto FetchOptions rather than only the one.

🤔 Why

A container pull by digest names the digest in its request path, so the fill has a reference value to verify against. S13's tee committed with nil, on reasoning stated in the code: "an upstream commits to no digest upfront, and an ETag is not one, so the server-computed SHA-256 is authoritative."

That holds for Maven and npm. It does not hold for container.

🔨 How

FetchOptions.ExpectedDigest reaches the expectedDigest argument storage.Session.Commit already takes. Commit already runs the comparison and already rolls the fill back on a mismatch, so this adds no verification machinery — only the value. A format that names no digest upfront leaves it unset and gets today's behavior unchanged.

The failure is deliberately outside ErrCacheFill. The cache-side machinery worked here: it detected bad content and refused it. What failed is the upstream's bytes against a digest naming them, so counting it toward a cache-fill SLI would blame this service for another's integrity failure. It surfaces as ErrUpstreamDigestMismatch from the stream body's Read, and the single-flight follower path carries the sentinel through unchanged so N-1 coalesced callers are not charged for it either.

Only a canonical lowercase sha256 is admitted, and Fetch panics on anything else. Commit compares the value as a raw string against a digest this service always computes that way, so an upper-case reference forwarded verbatim, or one under sha384 or sha512, could only ever be refused there — and refused as the upstream's integrity failure for a fault that is the caller's. The guard runs on Fetcher.Fetch, not only on the coalescing seam: internal/format/oci builds a remote.Fetcher directly and composes no SingleFlight, so a check living only on SingleFlight would miss the first flow that sets the option. Admitting sha256 alone matches oci.ParseDigest and the container cache-key parser.

Setting it alongside a Transform panics. The tee stages the transform's output, so the pair can only disagree — a programming error at the call site rather than a runtime condition.

A follower inherits the leader's ExpectedDigest, and the seam enforces that this is safe. SingleFlight.Fetch additionally panics unless the path's last segment is exactly the digest. The path is the flight key, so a digest inside it makes two callers on one key provably name the same digest; a by-digest fetch satisfies that for free, and a tag path cannot. Without the guard, whichever caller won the race picked the verification criterion for everyone behind it — silently in both directions.

🚫 What is not here

The cross-check a tag read needs — comparing the upstream's own Docker-Content-Digest, which names no value the request supplied — is the third MR. It builds on this one's sentinel and expectedDigest field.

📚 Spec coverage

Spec: docs/specs/S13-virtual-remote-foundation.md

Acceptance criteria

Criterion Tests
Only FetchOptions.ExpectedDigest reaches storage.Session.Commit as its expected-digest input TestFetcher_Fetch_ExpectedDigest_ReachesCommit, TestFetcher_Fetch_NoExpectedDigest_CommitsWithNilPointer
The leader's value reaches Commit, and one flight commits once under it TestSingleFlight_Fetch_LeaderExpectedDigestReachesCommit, TestSingleFlight_Fetch_CoalescedByDigestFetchCommitsOnce
ExpectedDigest alongside Transform is a programming error TestFetcher_Fetch_ExpectedDigestWithTransform_Panics
ExpectedDigest on a path that does not name it is a programming error TestSingleFlight_Fetch_ExpectedDigestNotNamedByPath_Panics
A malformed or non-sha256 ExpectedDigest is a programming error, on the bare Fetcher as well as under coalescing TestFetcher_Fetch_NonCanonicalExpectedDigest_Panics, TestSingleFlight_Fetch_MalformedExpectedDigest_Panics, TestSingleFlight_Fetch_NonCanonicalAlgorithmExpectedDigest_Panics
A digest mismatch is never fallback-eligible, including wrapped in a url.Error TestFallbackEligible
A matching ExpectedDigest commits the blob and fills the cache against a live BlobStore TestFetcher_Fetch_Integration_ExpectedDigestMatch_CommitsRealBlob
A digest over a transparently decoded body still fills, against a real gunzip TestFetcher_Fetch_GzipUpstreamCachesDecodedBytes
The digest crosses both standalone projections onto FetchOptions and reaches Commit TestStandaloneFlow_Deliver_CarriesExpectedDigestToTheCommit, TestStandaloneOptions_CarriesExpectedDigestToTheCommit

Error cases

Condition Tests
A mismatch stays outside ErrCacheFill and propagates through the flight TestFetcher_Fetch_ExpectedDigestMismatch_FailsOutsideTheCacheFillClass, TestSingleFlight_Fetch_DigestMismatchPropagatesOutsideErrCacheFill
A mismatch commits no real blob against a live BlobStore TestFetcher_Fetch_Integration_ExpectedDigestMismatch_CommitsNoRealBlob
A digest over the compressed wire bytes is refused, with a real decode meeting a real Session.Commit, and neither representation left behind TestFetcher_Fetch_Integration_GzipUpstream_DigestOverWireBytes_CommitsNoRealBlob

Verification

Command Result
go build ./... clean
go vet ./... and go vet -tags=integration ./... clean
go test -count=1 ./... 66 packages ok, zero failures
golangci-lint run --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false ./internal/remote/... ./internal/format/npm/... 0 issues
the same command with --build-tags=integration 51 findings, none on a line this MR adds — 50 in npm integration test files it does not touch, plus the known paralleltest hit in healthjob_integration_test.go
gofmt -l internal/ clean

The full-suite run needs GOOGLE_APPLICATION_CREDENTIALS pointed at any credentials file; without it the three TestWireStorage_CloudCDN* tests fail on the local ADC gap, unrelated to this change.

Mutations confirmed red and restored by checksum:

Mutation Fails
Drop the ExpectedDigest-to-Commit wiring the reaches-Commit tests
Remove the ErrCacheFill exclusion arm TestFallbackEligible
Delete the Algorithm() != digest.Canonical arm the three sha384/sha512 subtests, across both seams
Remove the guard call from Fetcher.Fetch, leaving SingleFlight's TestFetcher_Fetch_NonCanonicalExpectedDigest_Panics only — which is what shows the two seams are covered independently
if false && isGzipEncoded(...) in newCappedBody TestFetcher_Fetch_Integration_GzipUpstream_DigestOverWireBytes_CommitsNoRealBlob, whose mismatch becomes a match
Drop ExpectedDigest from StandaloneFlow.fetchAndDispatch both standalone carriage tests
Drop ExpectedDigest from StandaloneOptions.flowOptions TestStandaloneOptions_CarriesExpectedDigestToTheCommit only — which is what isolates the two projections

🔄 Merge order

!1806 (merged) has merged. This branch is rebased on it, and its diff against main is now this MR's change alone.

MR Scope
!1806 (merged) spec corrections against what main already ships — merged
this one FetchOptions.ExpectedDigest reaching the cache-fill commit
!1807 the upstream Docker-Content-Digest cross-check
!1325 the combined reference these three were extracted from — never merged, closed once they land

📏 Size

Measured three-dot at head, git diff --shortstat origin/main...HEAD reports 15 files, +1,182 / -47. Split by file group:

Group Added Files
Tests 887 5
Production Go 275 8
Specs 20 2

Tests are 75% of the added lines. Two fake-doer tests whose premise did not hold were deleted in review and their coverage rebuilt against a real decode, so the test count moved in both directions.

The production Go is internal/remote plus 8 lines in internal/format/npm/remote_tarball.go, which is one of the three projections onto FetchOptions the parity guard covers. Splitting that line off would leave the guard red on both halves, so it travels here.

🧪 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

Edited by Radamanthus Batnag

Merge request reports

Loading
Loading