Repository navigation
fix(deps): bump ijson-jcs to 03df9ed for RFC 8785 number ties - #261
Merged
Merged
Conversation
The pinned ijson-jcs (3a9a4ee) rearranged Rust's float formatting and printed RFC 8785 Appendix B vector 0x43143ff3c1cb0959 as 1424953923781206.3. JCS requires 1424953923781206.2: ECMAScript Number::toString chooses the even digit on a decimal tie. ijson-jcs main now formats with ryu-js, the ECMAScript algorithm, and passes every Appendix B vector. A test in crate::json pins the vector; on the old pin it fails with the .3 output. New transitive dependency: ryu-js 1.0.3 (Apache-2.0 OR BSL-1.0). cargo audit: no new advisories. 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:01
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. |
This was referenced Nov 5, 2026
hyperpolymath
disabled auto-merge
November 5, 2026 15:42
hyperpolymath
enabled auto-merge (squash)
November 5, 2026 16:56
hyperpolymath
disabled auto-merge
November 5, 2026 17:30
Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com>
|
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>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f2b9394b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| `+usize+`. Hashes are unchanged on 64-bit platforms; 32-bit builds now agree | ||
| with them. The computation lives in `+attestation::evidence::evidence_hash+`, | ||
| with a known-answer test whose value comes from coreutils `+sha256sum+`. | ||
| ==== Fixed — JCS numbers on decimal ties |
There was a problem hiding this comment.
This replaces the existing Unreleased section for the platform-independent evidence hash instead of inserting the new JCS section alongside it. The evidence-hash change remains part of the unreleased code, so this omission would produce incomplete release notes; retain the prior section below the new entry.
Useful? React with 👍 / 👎.
hyperpolymath
added a commit
that referenced
this pull request
Nov 5, 2026
## Problem `assail` ignored `--output-format kyaml` and always wrote the JCS report. ## Change **KYAML copy** - With `--output report.json --output-format kyaml`, assail also writes `report.kyaml` beside the JSON. - The JSON stays the report of record. `--attest` seals its bytes, so neither the seal nor `attest verify --report` (#263) changes. - Other output formats are unchanged. **One alias check over every output** (`kyaml_copy_path`, run before anything is written) - `--output` must end in `.json`, in any letter case. The report, the copy and the attestation sidecar (`S.json`, `S.kyaml`, `S.attestation.json`) therefore have distinct names, even on case-insensitive filesystems. - Each output path is looked up once. The run stops with "nothing was written" if any path is: - a symbolic link; - an entry that is not a regular file; - a path that cannot be looked up, such as a name over the length limit; - the same file as another output (hard links). - One shared helper, `attestation_sidecar_path`, gives the sidecar path to both the check and the writer. ## Scope decision Earlier revisions added rename-based atomic writes. On the maintainer's call, beeaea4 moves them to a stacked follow-up, and this PR now writes with `fs::write` as main does. The follow-up publishes each run as a **generation directory** with an atomically swapped pointer, in every output mode. It is meant to close three things: - the window between a check and its write, where a link could be swapped in; - partial files left by a crash; - interleaving between concurrent runs. Its design note will also cover what a replacement keeps: ACLs, xattrs, security labels and ownership. The doc comment and CHANGELOG state these limits. Those review threads are answered and resolved. ## Validation - Integration tests (`tests/coverage_php_tests.rs`) cover: - the sealed JSON together with an equal KYAML copy; - the `.json` requirement, in any letter case; - a symlink at each output, including dangling ones; - hard links, including the sidecar; - a directory at an output path; - an overlong derived name. - On beeaea4: `cargo fmt --check` and `cargo clippy --all-targets` are clean, and `cargo test` is green (20 suites, 0 failures). This touches `CHANGELOG.adoc` `[Unreleased]`, as #261–#263 do. Whichever of them merges later needs main merged in first. 🤖 Generated with [Claude Code](https://claude.com/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 "ijson-jcs tie case" follow-up from #237/#247.
Problem
The pinned
ijson-jcs(3a9a4ee) built ECMAScript number output by rearranging Rust's float formatting. Rust doesn't guarantee ECMAScript's choice on a decimal tie, so RFC 8785 Appendix B vector0x43143ff3c1cb0959came out as1424953923781206.3. JCS requires1424953923781206.2. A report or attestation holding such a float would hash differently from other JCS implementations.Fix
Bump to ijson-jcs
main(03df9ed). It now formats numbers withryu-js, which implements ECMAScriptNumber::toString, and passes every Appendix B vector (cargo test --test rfc8785in ijson-jcs: 5/5). The fix was already upstream, so only the pin needed to move. The lockfile was updated withcargo update -p ijson-jcs.Validation
json::tests::numbers_follow_rfc_8785_on_decimal_tiespins the vector. It fails on the old pin withleft: "1424953923781206.3\n"and passes on the new one.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).cargo audit: no new advisories; only the four known upstreamunmaintainedwarnings remain, tracked as W1/W2 in standards' upstream-dependency watch.ryu-js1.0.3, licensedApache-2.0 OR BSL-1.0, which is compatible with MPL-2.0.Existing golden fixtures are unchanged: none of them hold a tie-case float.
Generated by Claude Code