Add a dated dependency-audit policy and reseal tooling - #385
Merged
Merged
Conversation
…eal wrapper RUSTSEC-2026-0285 against rustls (fixed in 9558a31) blocked all 9 open PRs on an unrelated advisory publish, because cargo-audit had no ignore policy and no way to distinguish "this PR introduced a finding" from "the world changed underneath it". This adds: - .cargo/audit.toml: a dated, reasoned ignore entry per standing warning (pkcs11, glib, proc-macro-error, the five unic-* crates), each checked for actual reachability on shipped targets via `cargo tree -i <crate> --target <triple>`. chacha20 (yanked) is deliberately left unignored -- it has no RUSTSEC id to ignore and, per an actual `cargo tree` check, is an orphaned Cargo.lock entry unreachable from any workspace target. - scripts/check-advisory-delta.mjs: resolves the advisory-id set at the merge base and at HEAD from the same advisory-database snapshot, and fails only on ids new at HEAD. Proven against real history in scripts/check-advisory-delta.test.mjs using the actual pre-fix/post-fix commit pair for RUSTSEC-2026-0285. - scripts/reseal.sh: wraps the documented rehash-surface -> seal-surface -> repoint-matrix sequence (docs/release-process.md) so the order cannot be silently violated by hand, plus a --pins-changed inversion and a --verify mode for CI. Covered by scripts/reseal.test.mjs. Both new *.test.mjs files skip themselves with a clear reason when their Rust toolchain / cargo-audit prerequisite is absent, since the "Frontend build" CI job that runs `pnpm test` currently has neither -- see docs/proposed-dependency-policy.md for the proposed CI wiring (not implemented here; changes to .github/ are out of scope for this change). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…trix Reconciles the AUTHORED half (surface.files by path, matrix.claims by claim_id) via a real three-way merge before resealing, so it only auto-resolves the case docs/release-process.md already says is safe (disjoint changes) and falls back to plain git merge-file -- ordinary conflict markers -- when both sides genuinely touched the same entry differently. Wired via .gitattributes + a documented one-time git config line; LOCAL ONLY, GitHub's server-side PR merge does not run it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…index stages Index stages 1/2/3 for a path are only populated by git AFTER a configured merge driver has run and reported failure, not while it is still running -- so reading them from inside the driver itself always came back empty. Switched to reading the other (non-%P) file via HEAD / .git/MERGE_HEAD / their merge-base, and reading %P's own file directly from the %O/%A/%B temp files git already handed us (always reliable, any git operation). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…uring a merge A real two-branch merge experiment caught the previous version sealing a WRONG hash: it ran rehash-surface against the working tree mid-merge, and git does not guarantee every other path has already been checked out to its final post-merge content by the time this driver runs for compatibility-surface.json. computeCorrectSurfaceFiles() now determines each pinned file's correct post-merge content directly from git refs (base/ours/theirs via git diff --name-only + git show), which depends only on commit objects and cannot exhibit that race. seal-surface and repoint-matrix are invoked directly (bypassing scripts/reseal.sh's rehash-surface step entirely) since the hashes are already correct. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ardcoded .git/ path In a linked git worktree, <root>/.git is a plain file pointing at the real git-dir elsewhere, not a directory -- MERGE_HEAD never lived where the previous version looked, so detectMergeRefs() always returned null there and the driver silently always fell back to plain git merge-file. Caught by re-running the merge experiment (this repo's own working copy is a worktree). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…_HEAD MERGE_HEAD does not exist yet while a merge driver is running (git only writes it after the whole tree-level content merge finishes, which is after every driver invocation) -- confirmed by ENOENT against a real merge run using the git rev-parse --git-path fix from the previous commit, which resolved the right path but still read it too early. gitattributes' merge.*.driver placeholders %S/%X/%Y (documented as 'conflict labels' for the common ancestor / local head / other head) come through as resolvable revisions in ordinary usage and are available immediately -- confirmed against the same real merge, which now completes cleanly with byte-correct pinned-file hashes end to end. Also updates the git config line: merge.bridge-compat-reseal.driver now passes %S %X %Y (docs/release-process.md's one-time git config line is updated to match). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Builds two disposable branches from the current commit, merges them for real, and deletes both branches (and restores the original HEAD and any pre-existing merge.bridge-compat-reseal git config) whether the test passes or fails. All three real bugs found while building the driver this session were specific to actual git merge behavior a mocked environment would not have reproduced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
docs/release-process.md gets two new subsections under the existing compatibility-surface reseal material: scripts/reseal.sh (the wrapper) and the merge driver (local-only, with an explicit statement that GitHub's server-side PR merge does not run it). docs/proposed-dependency-policy.md is new and covers everything that needed a maintainer decision or a .github/ change this session was scoped not to make directly: the differential advisory-audit gate's proposed workflow wiring (with real before/after cargo-audit --json proof), a scheduled daily master audit with issue tracking, CI wiring so the three new toolchain- dependent *.test.mjs files have somewhere to actually run, and an honest structural-option analysis (splitting manifest_sha256 into a sibling file) that is explicitly NOT implemented because it needs new cross-file integrity enforcement to not regress the current single-file atomicity guarantee. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…licy-and-reseal-tooling
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
1 participant
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.
Two structural problems, both diagnosed from incidents that happened today.
1. The audit gate had no policy and froze the whole repo
cargo auditran bare — nodeny.toml, noaudit.toml, no ignore policy anywhere across 622 transitive dependencies. On 2026-09-15, RUSTSEC-2026-0285 published againstrustls(transitive viareqwest) and, with no code change on our side, a required status check went red and blocked all 9 open PRs. The bump (#381) was right; the blast radius was not..cargo/audit.toml— the 9 standing warnings, dated and justifiedReachability was checked per crate with
cargo tree -i, not assumed:pkcs110.5.0 (unsound)unic-*(unmaintained)tauri-utils→urlpatternglib0.18.5 (unsound)proc-macro-error(unmaintained)pkcs11is the entry that matters. It is a direct dependency of thebridgecrate, used 14 times insrc-tauri/src/dsc.rsfor PKCS#11 hardware-token DSC signing, and reachable on both shipped targets.cryptoki0.12 is already a dependency and used 5 times in the same file — so DSC signing currently runs through two PKCS#11 bindings, the older of which is unsound and unmaintained. The right fix is completing thecryptokimigration and droppingpkcs11, not renewing this entry on autopilot. That is stated in the file.Nothing here silences a real vulnerability. These are accepted warnings;
cargo auditstill fails on anyerror, which is what caught the rustls bug.Honest limitation, stated in the file itself:
[advisories].ignoreis a flat list of IDs withdeny_unknown_fields, so there is no structured field for reason, reach, or review date. Each justification is a comment. A comment is not machine-checkable — only a human reviewing this file enforces it.Also documented: cargo-audit resolves
./.cargo/audit.tomlrelative to the process working directory, not the lockfile. Moving it undersrc-tauri/would silently stop it being read, with no config-loading error at all.chacha200.10.1 (yanked) is deliberately NOT ignoredIt can't be — a yanked-crate warning has no advisory ID. On inspection it is an orphaned
Cargo.lockentry:cargo tree -i chacha20 --target allresolves to nothing. That is a lockfile cleanup, not an accepted risk.scripts/check-advisory-delta.mjs— differential auditingA PR should fail when it introduces an advisory, not when the world changed underneath it. Resolves the advisory-ID set at merge-base and HEAD against one shared DB snapshot and fails only on IDs new at HEAD.
This is custom scripting —
cargo audithas no native differential mode. Proved firing both ways against the real RUSTSEC-2026-0285 commit pair (8b17f95c→9558a316): exit 1 with the exact advisory when the direction is reversed, exit 0 on the real fix direction.2. The compatibility seal serialises every PR
Diagnosis proven by experiment, not asserted. Two throwaway branches were built off master, each editing a different pinned file, each resealed, then merged. Only
manifest_sha256(surface) andcompatibility_surface_sha256(matrix) conflicted — the entire 212-entry, 853-linefilesarray merged with zero conflicting lines.So two single lines force a conflict between any two concurrent PRs, and each merge to master re-stales every other PR's seal. That cost four manual merge-and-reseal cycles in one session today.
scripts/reseal.shWraps the order-dependent three-command sequence.
docs/release-process.mdwarns: "Do not run step 2 without step 1: sealing a manifest whose file hashes are stale produces a valid-looking digest over stale source content." A footgun that yields a valid-looking-but-wrong seal should be wrapped, not documented. Handles the--pins-changedinversion and adds--verifyfor CI.Verified by driving the failing branch, not just the happy path: an order violation produces
sha256_invalidwith no partial write, and--verifyagainst a mutated pinned file fails with an exact digest diff, then passes again once restored.scripts/reseal-merge-driver.mjs+.gitattributesThree real bugs were found by running the merge, not by inspection:
.git/MERGE_HEADdoes not exist yet either — and a hardcoded.git/path additionally breaks in a linked worktree, where.gitis a redirect file.The driver does a real per-entry three-way reconciliation, not a blind take-one-side, and refuses (leaving ordinary conflict markers) when the same entry genuinely conflicts.
Stated plainly and not oversold: a
.gitattributesmerge driver is LOCAL-ONLY. GitHub's server-side merge will not run it. This reduces the pain; it does not remove it.Deliberately not implemented
Splitting
manifest_sha256into a sibling file so the big list merges cleanly is written up indocs/proposed-dependency-policy.mdbut not done — it changes a security-relevant artifact, and it trades single-file atomicity for a cross-file consistency obligation the gate does not currently enforce. That is your call, not a subagent's.Not verified
.github/was out of scope for the agent that wrote it.*.test.mjsfiles skip (verified as skip, not silent pass) in the currentpnpm testjob, which has no Rust toolchain. They only execute once the proposed CI wiring is adopted.reseal.shis Unix-only; Windows users run the four raw commands, as the doc says.Unrelated gap noticed
docs/adr/0016-master-binding-authority.mdis not in the pin set, though ADRs 0004/0005/0014/0015 are. That is the master-binding contract #331 just enforced in code, sitting outside the seal. Not changed here — flagging it.🤖 Generated with Claude Code