[orshih6]

The cache-hit path wrapped the stored artifact with checks.wrap. That reader reports a digest mismatch to its callback but still returns io.EOF, so the client got the full corrupted body with a 200 and a matching Content-Length. The upstream streaming path used wrapFailOnMismatch, which does return an error, but only together with the final read, after every byte had already been written.

Changes:

• verifyingReader in fail-on-mismatch mode now runs one chunk behind its source. It holds the latest chunk until the next read shows it isn't the last one, or until EOF, when the digest is checked. If the digest matches, the held chunk is released. If it doesn't, the chunk is dropped and ErrArtifactDigestMismatch is returned. Only one chunk is buffered, sized to the caller's read buffer, so streaming is kept.
• wrapFailOnMismatch now keeps the seekable wrapper like wrap does, so range requests on cached artifacts still work.
• The cache-hit path in readStored uses wrapFailOnMismatch. The mismatch callback still logs, records the metric and discards the entry. copyArtifactBody already panics with http.ErrAbortHandler when the copy errors, so the client gets a body shorter than Content-Length and a read error.

The non-failing wrap behaves as before. Its existing tests are unchanged.

Testing:

• TestCorruptCachedArtifactAbortsResponse seeds a cached artifact, flips its last stored byte, and serves it through httptest.Server to a real client. It asserts that the body read fails, that the body is shorter than Content-Length, and that the cache record is cleared.
• TestFailOnMismatchHoldsBackFinalChunk covers the reader with whole reads, one-byte reads and data returned together with EOF, for both matching and mismatched bytes.
• Both tests fail without the change. go test ./internal/handler/ and go vet ./internal/handler/ pass.

🤖 Generated with Claude Code

Fixes #498

---

This change was prepared with an AI coding agent (Claude) and reviewed by me before submitting. I'm happy to make any changes you'd like.

[andrew]

The final-chunk buffering prevents a digest mismatch from completing a successful download, and the HTTP-client regression test covers the corrupted cache-hit response. No blocking findings.

[andrew]

Please update the integrity regression test for the current storage API. The combined code no longer compiles against main because storage.ArtifactPath was removed after the earlier approval.

[andrew]

Current main removed storage.ArtifactPath, so this test no longer compiles when the PR is merged. Reproduced against the combined tree with go test ./internal/handler -run '^TestCorruptCachedArtifactAbortsResponse$' -count=1: internal/handler/integrity_test.go:364:25: undefined: storage.ArtifactPath. Read the seeded artifact's StoragePath from the database and corrupt that stored object instead of reconstructing its path, then rerun the regression test against current main.

[orshih6]

@andrew Thanks, fixed. The test now gets the seeded artifact's StoragePath from the database and corrupts that object, so it no longer uses storage.ArtifactPath. I merged the branch with current main locally and ran go test ./internal/handler -run '^TestCorruptCachedArtifactAbortsResponse$' -count=1, and it passes. The rest of go test ./internal/handler passes on the merged tree too.