Repository navigation
feat(task): verify remote cache artifacts - #11627
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR adds focused tests for CAS blob digest rejection and URL-scheme validation, marks remote artifact integrity and authenticity complete in the parity tracker, and makes a behavior-preserving manifest serialization refactor.
Confidence Score: 4/5The PR does not appear safe to merge until remote artifacts are checked against an independently trusted expected checksum before local promotion. The current remote read path still computes the manifest checksum from the artifact it just downloaded, so a substituted output tree can satisfy the later checksum comparison and be promoted into the local cache; the new test only proves that an individual CAS response is rejected when it differs from its caller-provided digest. Files Needing Attention: src/task/task_cache_store.rs Important Files Changed
Reviews (12): Last reviewed commit: "fix(task): accept reordered remote cache..." | Re-trigger Greptile |
bd24d25 to
ccdd3bb
Compare
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.
|
ccdd3bb to
954d751
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 954d751. Configure here.
516e7ad to
dbbbe9a
Compare
5e361f1 to
c247d7a
Compare
c247d7a to
cf2d87c
Compare
cf2d87c to
3d2f6e3
Compare
3d2f6e3 to
1a5ff33
Compare
1a5ff33 to
bacb2a8
Compare
bacb2a8 to
c4e8828
Compare
## Summary - validate remote manifest keys, formats, output roots, and mandatory artifact checksums before local promotion - reject corrupt downloaded artifacts as cache misses without poisoning the local cache - require HTTPS for remote services while retaining loopback HTTP for local development - document the transport and integrity requirements in protocol version 1 ## Tests - `cargo test task_cache_store::tests` - `cargo test remote_entries_require_artifact_checksums` - `cargo clippy --workspace --all-features --all-targets -- -D warnings` - `mise run lint-fix` *AI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.* <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes affect remote cache restore and URL validation on a security-sensitive path; corrupt data should miss rather than poison local cache, but misconfiguration or stricter HTTPS rules could break existing setups. > > **Overview** > **Remote task cache reads now treat transport and content integrity as part of the contract**, so bad or tampered CAS payloads fail verification instead of being trusted. > > HTTP blob downloads (in-memory and streamed-to-file) are checked against the declared **blake3/sha256** digest and size; mismatches surface as errors (covered by a new mock test for substituted blob bodies). Remote **action-result** assembly still computes the manifest **artifact checksum** after materializing the output tree, then returns a serialized manifest entry. **Remote URL** validation now explicitly rejects non-HTTP(S) schemes such as `ftp://` in tests, alongside the existing HTTPS / loopback-HTTP rules. > > The parity tracker marks **“Verify remote artifact integrity and authenticity”** as complete. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c4e8828. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->

Summary
Tests
cargo test task_cache_store::testscargo test remote_entries_require_artifact_checksumscargo clippy --workspace --all-features --all-targets -- -D warningsmise run lint-fixAI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.
Note
Medium Risk
Changes affect remote cache restore and URL validation on a security-sensitive path; corrupt data should miss rather than poison local cache, but misconfiguration or stricter HTTPS rules could break existing setups.
Overview
Remote task cache reads now treat transport and content integrity as part of the contract, so bad or tampered CAS payloads fail verification instead of being trusted.
HTTP blob downloads (in-memory and streamed-to-file) are checked against the declared blake3/sha256 digest and size; mismatches surface as errors (covered by a new mock test for substituted blob bodies). Remote action-result assembly still computes the manifest artifact checksum after materializing the output tree, then returns a serialized manifest entry. Remote URL validation now explicitly rejects non-HTTP(S) schemes such as
ftp://in tests, alongside the existing HTTPS / loopback-HTTP rules.The parity tracker marks “Verify remote artifact integrity and authenticity” as complete.
Reviewed by Cursor Bugbot for commit c4e8828. Bugbot is set up for automated code reviews on this repo. Configure here.