Repository navigation
fix(attestation): hash evidence counts as u64 on every platform - #262
Merged
Merged
Conversation
The evidence hash fed files_read to SHA-256 as usize::to_le_bytes, so a 32-bit build hashed 4 bytes where a 64-bit build hashed 8, and the same scan produced different evidence hashes. files_read is now widened to u64 like bytes_read and wall_clock_ms. On 64-bit platforms the bytes, and so every existing hash, are unchanged. The computation moves to a pub evidence_hash function with a known-answer test whose expected value comes from coreutils sha256sum. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFaqv6iBXrt3W5Akd1Fq5k
hyperpolymath
enabled auto-merge (squash)
November 5, 2026 00:05
Contributor
|
Caution Review failedThe pull request is closed.
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. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
hyperpolymath
disabled auto-merge
November 5, 2026 15:41
|
hyperpolymath
added a commit
that referenced
this pull request
Nov 5, 2026
Closes two follow-ups from #247. ## Problem `attest verify` checks that the seal's three stored digests chain together, and that `decision_hash` equals `report_hash`. Both of those values are stored inside the sidecar. **Nothing rehashed the report itself**, so a report edited after sealing still verified: ``` $ attest verify r.attestation.json # r.json edited after sealing [OK] Attestation verified ``` Legacy seals (no `hash_scheme`) also verify without binding their intent and evidence, and there was no way to require binding. ## Change - **`--report <REPORT>`** rehashes the report file and fails with `report_hash mismatch: stored=… computed=…` if the hashes differ. - **`--require-bound`** fails on legacy seals with `legacy hash scheme: intent and evidence are not bound to the seal`. - The output now has a `Report:` line: `rehashed, matches the seal` or `not checked (pass --report)`. - **Library:** - `VerifyOptions { report, require_bound }`, used with new `verify_attestation_file_with` / `verify_attestation_str_with`. - The existing `verify_attestation_file` / `verify_attestation_str` keep their behaviour; they call the new functions with default options. - `VerifyResult::Ok` gains `report_checked`, so a caller can't mistake an unchecked report for a verified one. ## Validation - **New tests:** - `the_sealed_report_verifies`: `report_checked` is true with `--report` and false without it. - `an_edited_report_is_detected` - `require_bound_rejects_legacy_seals_only`: a genuine legacy seal fails, a JCS seal passes. - **End to end with the built CLI** (`assail examples --attest`): - The genuine report verifies (`Report: rehashed, matches the seal`). - A report edited after sealing fails with `report_hash mismatch`; the same file without `--report` still says `[OK]`, which is the gap this closes. - A legacy sidecar, rebuilt with `jq` plus a `sha256sum` chain hash, passes by default and fails with `--require-bound`. - A missing report file gives a clear `reading …: No such file or directory`. - `cargo fmt --check`, plus `cargo clippy --locked --all-targets -D warnings` with and without `--all-features`: clean. - `cargo test --locked --all-features`: all suites green (lib: 526 passed). This touches `CHANGELOG.adoc` like the other open PRs (#261, #262). Whichever merges later needs a trivial merge of main, and I'll handle it. --- _Generated by [Claude Code](https://claude.ai/code/session_01PFaqv6iBXrt3W5Akd1Fq5k)_ --------- Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
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
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 the "
evidence_hashhashesusize::to_le_bytes" follow-up from #247.Problem
EvidenceAccumulator::finalizefedfiles_readto SHA-256 asusize::to_le_bytes. A 32-bit build hashed 4 bytes where a 64-bit build hashed 8, so the same scan produced different evidence hashes on different platforms, and a verifier on another platform couldn't recompute them.Fix
files_readis widened tou64before hashing, likebytes_readandwall_clock_msalready are.attestation::evidence::evidence_hashfunction, so a verifier can recompute it, and the doc comment now states the encoding.Validation
evidence_hash_known_answer. The expected value comes from coreutils, not the crate:cargo fmt --check, pluscargo clippy --locked --all-targets -D warningswith and without--all-features: clean.cargo test --locked --all-features: all suites green (lib: 524 passed).Generated by Claude Code