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 |
|---|---|
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