Repository navigation
feat(artifact-cas): verify downloads against the requested digest via disk staging - #3461
Merged
Merged
Conversation
… disk staging Downloads now go through the same integrity gate #3365 introduced for uploads. The backend content is spilled to an unlinked file under staging_dir, hashed in the same pass and compared with the requested digest; only a verified file is streamed to the client, so no unverified byte leaves the CAS in either direction. - Add a shared stageAndVerify helper (spill / hash / compare / rewind) and refactor the upload path onto it with no behaviour change. - gRPC ByteStream.Read verifies before the first Send instead of streaming while hashing and reporting a mismatch after the last byte. - HTTP GET /download no longer buffers the whole artifact in memory; it streams the verified staging file with Content-Length from the verified size, so CAS memory is bounded regardless of artifact size. - A download mismatch is reported as gRPC DataLoss / HTTP 500 with both digests, replacing Unauthorized / 401: the fault is backend content, not the caller. - Update staging_dir docs and the Helm chart comments to cover downloads; bump the chart patch version. Closes #3460 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: 4b106940-b636-4b56-bfb9-d325464483ca
Contributor
AI Session Checks — 🟢 86% · ✅ 0 failing
|
| Status | Policy | Material | Messages |
|---|---|---|---|
| ✅ Passed | ai-config-ai-agents-allowed |
ai-coding-session-4b1069 |
- |
| ✅ Passed | ai-config-no-dangerous-commands |
ai-coding-session-4b1069 |
- |
| ✅ Passed | ai-config-no-secrets |
ai-coding-session-4b1069 |
- |
| ✅ Passed | ai-config-mcp-servers-allowed |
ai-coding-session-4b1069 |
- |
Security Checks — ⚠️ 1 failing
✅ secret-scan
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | secrets-detection |
- |
✅ sast-scan
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | owasp-top10-2025 |
- |
| ✅ Passed | sast |
- |
| ✅ Passed | cwe-top25 |
- |
| ✅ Passed | cwe-top26-40-cusp |
- |
⚠️ iac-scan — 1 failing
| Status | Policy | Messages |
|---|---|---|
iac-misconfiguration |
|
PR info
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | pr-min-approvals |
- |
| ✅ Passed | pr-description-required |
- |
| ✅ Passed | pr-user-story-linked |
- |
⏭️ 2 scans not applied
| Scan | Reason |
|---|---|
vulnerability-scan |
no manifest/lockfile changed |
github-actions-scan |
no workflow files changed |
View attestation ↗
View attestation ↗
Powered by Chainloop and Chainloop Trace
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 17 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The download adapter was renamed from streamWriter to sendWriter in the staging refactor; the test double's comment still used the old name. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: 4b106940-b636-4b56-bfb9-d325464483ca
matiasinsaurralde
approved these changes
Sep 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3460
What
The Artifact CAS now verifies downloads the same way #3365 made it verify uploads: the backend content is staged on local disk under
staging_dir, hashed in the same pass and compared against the requested digest, and only then streamed to the client.This is not a security patch. Digest verification of downloads already happened before this change and a tampered or corrupted blob was already detected on every path: the HTTP handler verified before writing the response, the gRPC
Readhandler returned an error after the last chunk, and the CLI re-hashes what it receives. What this PR changes is where the server-side check happens and what it costs:stageAndVerifyhelper and verify before forwarding a single byte. The gRPC download previously streamed content to the client while hashing and only reported the mismatch at the end; it now behaves like the HTTP path and like uploads.Content-Lengthfrom the verified size, so CAS memory during downloads is bounded regardless of artifact size, matching what feat(artifact-cas): verify uploaded content against the declared digest #3365 achieved for uploads.Other changes:
DataLoss/ HTTP 500 with astored content does not match the requested digest: got=…, want=…message, replacing the previousUnauthorized/ 401. The fault is backend content, not the caller.Operator notes
emptyDirnow also holds in-flight downloads. Size ephemeral-storage requests for concurrent uploads and downloads times the largest artifact. Comments inconf.proto, the CAS dev config and the Helm chart were updated accordingly and the chart is patch-bumped.chainloop artifact download) show no progress until the server has finished fetching and verifying the artifact, then stream at full speed.AI disclosure
This change was developed with the assistance of Claude Code.
🤖 Posted by Maximus bot (Claude Code) on behalf of @migmartri