Repository navigation
fix(deps): bump h2 to 0.4.16 for RUSTSEC-2026-0258 - #368
Vasanthdev2004 wants to merge 1 commit into
Conversation
`cargo audit` started failing on every branch today: h2 0.4.13 is vulnerable to RUSTSEC-2026-0258, "h2 unbounded empty DATA frames", fixed in 0.4.16. The advisory was published after main's last green run, so the existing green ticks on open PRs are stale results rather than evidence — anything that re-runs CI goes red until this lands. h2 arrives transitively through hyper, so no manifest changes: the lockfile is the whole fix. Edited by hand rather than with `cargo update -p h2`, deliberately. Running the update on this machine also rewrote nine unrelated entries — `syn` 2.0.117 to 1.0.109 for data-encoding-macro-internal, `windows-sys` 0.61.2 to 0.52.0 for errno, `socket2` 0.6.3 to 0.5.10 in three places — all DOWNGRADES, and all still present with `--precise 0.4.16`. That is local index drift against whatever generated main's lockfile, not something this fix needs, and it does not belong in a security bump. Only h2's `version` and `checksum` lines change here; `cargo check --locked` accepting the result is what confirms 0.4.16 needs no dependency changes of its own. Verified with `cargo check --locked --workspace` (exit 0) and, on a branch that carries the Windows test-gating fix, the full suite: gitlawb-core 104, gl 328, git-remote-gitlawb 58. `--all-targets` cannot be checked from main on Windows because the unix-only test code does not compile there, which is what #330's first commit fixes; Linux CI covers it.
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Adding the blast radius, since the green ticks elsewhere make this look less urgent than it is. Every open PR is affected. Most just have not noticed yet.
Two PRs have re-run
The other 21 open PRs still show So the count of visibly-broken PRs will keep climbing on its own, and each author will independently hit a red check that has nothing to do with their work and costs them time to diagnose. That is the real cost here, more than the vulnerability itself — The change is two lines, both inside 18/18 checks green here, including Happy to answer anything on the hand-edit rationale (the nine unrelated downgrades |
|
@kevincodex1 — could you take a look when you get a moment? Two-line lockfile diff, 18/18 green. It is blocking #327 and #331 today, and the remaining 21 open PRs go red on their next CI trigger regardless of their own changes (detail in the comment above). No rush if you would rather it went in differently — happy to close this in favour of another approach. |
There was a problem hiding this comment.
Verified the two changed lines rather than the CI ticks.
The checksum is the real one: sha256 of the h2-0.4.16 tarball from static.crates.io is a9f37a958b41b3b19ee2707c06439c0e9e547e847223eb791ecb0cb821c65e27, byte-identical to the line this adds, and the sparse index agrees. 0.4.16 is the advisory's minimum patched version, so this is the smallest bump that clears RUSTSEC-2026-0258. h2's normal dependency set is unchanged between 0.4.13 and 0.4.16, and the one requirement that moved (http ^1 to ^1.1) is satisfied by the locked http 1.4.0, so leaving the entry's dependency block untouched is correct.
I ran it both ways. Reverting the two lines takes cargo audit to exit 1 on RUSTSEC-2026-0258; restoring them, exit 0. Corrupting one hex digit of the new checksum takes cargo fetch --locked to exit 101 with a checksum error, and it does that with the crate warm in the registry cache and with no network at all. So your cargo check --locked was already sufficient proof that the hand-edited checksum matches what the registry publishes.
On your open question: it is not your toolchain. From main's lockfile on a Linux box, cargo update -p h2 --precise 0.4.16 gives the h2 bump byte-identical to your hand edit plus exactly the same seven rewrites. They are not crate downgrades, which is why they look worse than they are. syn, socket2 and windows-sys each already exist at both versions in main's lockfile, and what moves is which duplicate a consumer's edge points at. rustix, tempfile and errno declare windows-sys ">=0.52, <0.62" and data-encoding-macro-internal declares syn ">=1, <4", so both choices are legal.
They are also not caused by your bump. Running cargo update -p h2 --precise 0.4.13, the version main already pins, produces the identical seven changes with h2 itself untouched. So main's committed lockfile is not a fixed point of the current resolver and index, and any command that rewrites it re-normalizes those edges; the crate you name is incidental. Both lockfiles pass cargo metadata --locked. Nothing is drifting, hand-editing was the right call, and there is no separate issue to file.
One scope note, no change needed here: merging this re-runs nothing on the other open PRs, so their stale green ticks stay stale until each pushes or enters the merge queue. Nothing can land on a red audit regardless, since the checks run again on the merge group.
Worth knowing that CodeRabbit skipped this rather than cleared it, since Cargo.lock sits behind its default !**/*.lock filter.
Edited to correct two things in my original text. I had written that "cargo re-unifies downward when it rewrites the file" as the explanation for the seven rewrites; I never verified that and have retracted it rather than replacing it with another guess. What is verified is the no-op-update result above, which is the more useful fact anyway. I had also framed the dependency block as correct "not only because --locked accepted it" and described purging the registry cache as part of the checksum test; both undersold your own evidence, since --locked catches a bad checksum warm and offline.
|
@kevincodex1 LGTM |
|
Four days on, an update on the count — it has grown, and the growth is the point. Six open PRs are now red on
I checked the job logs rather than assuming: #325, #327, #363 and #331 all report When I first commented there were two. The other four arrived simply by touching their branches — no code of theirs changed to cause it. 28 more open PRs still show So the shape of this is: five authors who have nothing to do with Still two lines, both inside @kevincodex1 — no pressure on the approach; if you would rather this went in as part of a wider dependency sweep, or run from a Linux box with a fresh index, I will close this in favour of that. It is only the sitting-still that costs anything. |
- Re-add RUSTSEC-2026-0258 (h2) to audit.toml ignore list; h2 0.4.13 is still in Cargo.lock until Twigpine#368 lands - Add #[allow(dead_code)] to list_all_objects_with_type and all_blob_oids (used by tests only; CI -D warnings turns dead_code into errors)
|
Closing as superseded on @Vasanthdev2004 flagged RUSTSEC-2026-0258 when The advisory is cleared on Thanks for getting this unblocked. |
…igpine#218) Reconciliation sweep: periodic background worker that re-derives pin/seal sets and fills gaps so a dropped replication job never means data loss. - visibility_pack.rs: all_object_paths (two-phase ls-tree + cat-file --batch-all-objects), allowed_blob_tree_sets_bounded, ObjectPath/BlobTreeSets type aliases - db/mod.rs: atomic policy transactions, get/set_node_state, has_ipfs_cid, filter_ipfs_pinned_oids, filter_pinata_pinned_oids, repo_policy_epoch, migration v27 (node_state table + policy_epoch column) - ipfs_pin.rs: PolicyFence struct, dispatch fence re-check before POST - pinata.rs: dispatch fence re-check before POST - encrypted_pin.rs: dispatch fence re-check - reconciliation.rs: full sweep worker with keyset cursor, PolicyFence - api/ipfs.rs: effective_cid fallback in list_pins - main.rs: spawn reconciliation sweep worker - .cargo/audit.toml: RUSTSEC-2026-0258 h2 ignore (pending Twigpine#368)
…gpine#218) (Twigpine#551) * feat(node): implement reconciliation sweep as durability backstop (Twigpine#218) Reconciliation sweep: periodic background worker that re-derives pin/seal sets and fills gaps so a dropped replication job never means data loss. - visibility_pack.rs: all_object_paths (two-phase ls-tree + cat-file --batch-all-objects), allowed_blob_tree_sets_bounded, ObjectPath/BlobTreeSets type aliases - db/mod.rs: atomic policy transactions, get/set_node_state, has_ipfs_cid, filter_ipfs_pinned_oids, filter_pinata_pinned_oids, repo_policy_epoch, migration v27 (node_state table + policy_epoch column) - ipfs_pin.rs: PolicyFence struct, dispatch fence re-check before POST - pinata.rs: dispatch fence re-check before POST - encrypted_pin.rs: dispatch fence re-check - reconciliation.rs: full sweep worker with keyset cursor, PolicyFence - api/ipfs.rs: effective_cid fallback in list_pins - main.rs: spawn reconciliation sweep worker - .cargo/audit.toml: RUSTSEC-2026-0258 h2 ignore (pending Twigpine#368) * fix: add missing parameters to pinning functions and adjust timeout settings * fix(tests): update CID assertions to handle Option type and adjust parameters in pinning functions * fix(node): close review findings on reconciliation sweep (Twigpine#218) Reviewer-1 P1 / Reviewer-2 P1: v27/v28/v29 were referenced by tests but never added to the MIGRATIONS array, so every existing deployed node would fail to boot (node_state, repos.policy_epoch, pinned_cids.cid NOT NULL → NULL all unreadable). Forward-only migrations: v27 pinata_only_clear_legacy_equal_cid: ALTER cid DROP NOT NULL, UPDATE cid = NULL WHERE cid = pinata_cid v28 node_state_key_value: new key/value table for the sweep cursor v29 repos_policy_epoch: BIGINT NOT NULL DEFAULT 0 Reviewer-2 P1: visibility_pack.rs:542 phase 1 used ls-tree -rz, which under -r recurses into blobs and never emits tree entries. Trees then arrived only from the phase-2 catch-all with empty path and the fail-closed filter denied every one — the sweep could repair a non-flat repo's git graph. Switched phase 1 to ls-tree -r -t -z and add a per-commit rev-parse <commit>^{tree} that registers the root tree at "/". 48 visibility_pack unit tests pass. Reviewer-1 P2 / Reviewer-2 P2: list_pinned_cids called is_raw_cidv1 on the cid column before decoding it as Option<String>, so SQL NULL failed the decode and the new Pinata-only rows never reached the handler. Decode cid as Option first, drop only the both-NULL case. The is_raw_cidv1 filter was removed entirely: the new contract lists every row that has something to advertise, and the handler at api/ipfs.rs:list_pins is the seam that decides what to do with a legacy-shape row (the resolver 404s on a mismatched key, per Twigpine#173 U4). Updated list_pinned_cids_omits_unrepaired_legacy_row to match. Reviewer-1 P2: two new sqlx::test cases pin the gate order: sweep_skips_quarantined_repos_before_scan (SQL filter, plus a defense-in-depth per-row re-check) and sweep_skips_private_repos_before_scan (per-repo listable_at_root). Both use expect(0) on the mock IPFS so any future reorder that moves the scan ahead of the gate fails the build. Also: record_pinned_cid ON CONFLICT now rewrites cid (R1-P2: a stale wrong CID from a previous push is repaired by a subsequent one). record_pinata_cid ON CONFLICT clears cid to NULL when the existing row has the legacy cid = pinata_cid shape; on INSERT, stores cid = NULL when raw_cid == pinata_cid (the Pinata-only signal). * feat: implement per-backend continuation offset for missing OIDs - Enhance `missing_oids` function to accept a `start_after` parameter, allowing for rotation of the missing set based on the last attempted OID. - Update `run_pass` to load and save the continuation offset for both IPFS and Pinata backends, ensuring that previously attempted OIDs are retried last in subsequent passes. - Introduce tests to validate the behavior of the new offset mechanism, ensuring deterministic ordering and proper handling of gaps in the missing set. - Ensure that the offset is cleared when no missing OIDs are found, allowing for fresh starts in subsequent passes. * fix(node): trace provenance/structural invariants through every consumer The v30 local_ipfs_provenance column was set by the writers but only consulted by has_ipfs_cid and filter_ipfs_pinned_oids. The pin loop's "is this already done" check still used row existence (is_pinned), so a Pinata-only row hit the early-return branch without ever reaching the local writer, and the flag stayed FALSE forever. Three other seams carried the same shape-vs-provenance confusion. P1 (ipfs_pin.rs:1843): pin loop's skip keys on has_ipfs_cid (writer-owned local_ipfs_provenance = TRUE), not on row existence. A Pinata-only row now falls through to the local writer path, which calls record_pinned_cid_with_source and flips the flag on the conflict branch. The Pinata pin loop's skip remains keyed on has_pinata_cid (a separate seam). P1 (db/mod.rs v30 backfill): tighten the WHERE clause to cid IS NOT NULL AND pinata_cid IS NULL. The previous permissive (cid <> pinata_cid) clause marked pre-v30 Pinata-first uploads (where record_pinata_cid deliberately stored cid = raw_cid without a local Kubo push) as local IPFS pins, so the sweep filtered them as already-durable and an operator enabling IPFS later never got a local recovery copy. Ambiguous dual-shape rows stay at FALSE under the strict rule and the sweep re-derives on re-pin (cheap, idempotent). P1 (db/mod.rs record_pinned_cid_with_source ON CONFLICT): restore cid = COALESCE(pinned_cids.cid, EXCLUDED.cid). A Pinata-only row with cid = NULL (the raw_cid == pinata_cid shape) would otherwise be marked complete with local_ipfs_provenance = TRUE and a NULL resolver key. P1b (visibility_pack.rs): remove the synthetic "/" path on root trees and add a structural entry-level check (structurally_safe_root_tree / _for_caller) in both the sweep gate (allowed_blob_tree_sets_bounded) and the per-request tree gate (allowed_tree_set_for_caller_bounded). A tree is admitted only if every direct entry in its serialized bytes is safe at /{filename} and (for tree entries) the child tree is itself in the allow set. The previous path-based "/" check let the root tree of a public repo slip through even when its bytes name a denied subtree entry. P2 (api/ipfs.rs list_pins): add local_ipfs_provenance to PinnedCidRecord, include in list_pinned_cids SELECT, and surface as local_pinned / pinata_pinned in the JSON response. A Pinata-only row previously returned the raw resolver key as local_cid, making it indistinguishable from a real dual-backed row. The new fields let a gl consumer distinguish local-only, remote-only, and dual rows without re-inferring semantics from nullability. local_cid is preserved for backward compatibility. Tests: - migration_v30_backfills_local_ipfs_provenance_heuristically: flipped both-distinct expectation to FALSE under strict rule. - migration_v12_makes_cid_nullable_and_preserves_classification and migration_v27_clears_legacy_equal_cid: both-distinct / distinct row expectations flipped to FALSE. - sweep_never_pins_root_tree_naming_withheld_subtree (new): assert root tree + secret subtree tree absent from pinned_cids. - allowed_tree_set_gates_withheld_subtree_tree: root tree assertion inverted to EXCLUDED for anon. - list_pins_reports_writer_owned_provenance_for_all_shapes (new): assert 4 row shapes produce correct local_pinned / pinata_pinned JSON. Scope discipline preserved: PolicyFence capture order, _pin_permit reuse for the seal phase, fresh per-arm rederive_budget, keyset repo cursor with lookahead, mirror-row skip, Kubo 2xx-without-Hash refusal, v27 provenance-clearing migration all untouched. v31 schema unchanged. * fix(node): structural tree gate at every depth + non-commit ref tolerance The prior round installed a `local_ipfs_provenance` column and applied a structural tree gate only to root trees. The fresh review pass found two structural gaps the writer contract didn't propagate to: P1 (visibility_pack.rs:1519): the path-based tree admission admitted a tree whenever its path was allowed, regardless of whether its serialized bytes named a denied subtree. Concrete failure: a public repo with `/public/secret/file` and a `/public/secret/**` deny admitted the `/public` tree (its path is `/public`, allowed) but `/public` still contained a `secret/` entry pointing at the denied subtree — anyone who obtained `/public`'s CID could `cat-file -p` it and read the subtree's name and child OID. The prior round's post-pass structural check ran on root trees only. The fix: replace the prior round's `structurally_safe_root_tree` / `_for_caller` / `root_tree_oids` helpers with one recursive `tree_structurally_safe` that walks every tree's entries at every depth, recursing on tree entries. Bundled the rule/path args into a `TreeCheckCtx` struct to fix the clippy `too_many_arguments` warnings the prior round introduced. A tree is admitted iff every direct entry is safe at its path AND (for tree entries) the child tree is itself structurally safe there. A denied-subtree tree fails at its level; the parent fails because the denied-path child entry fails; the propagation continues up to the root. The `admitted` set memoizes trees proven safe at some path so the recursion short-circuits on cycles and on trees reachable at multiple allowed paths. P1 (visibility_pack.rs:533): `all_object_paths` called `assert_all_refs_are_commits`, which bailed on any ref that didn't peel to a commit (tag-of-tree, tag-of-blob). The sweep (`run_pass`) converted that error into a full-scan failure that skipped the whole repo. A repo with a pushable tag-of-tree never got its missing public pins repaired by the new hourly worker, even though its commit-reachable graph was otherwise safe to classify (`ipfs_cid_tree_served_despite_non_commit_ref` is the in-repo example of that supported shape). The fix: remove the `assert_all_refs_are_commits` call from `all_object_paths` (and the analogous one in `blob_paths`, used by the encrypted recovery path — the same reasoning applies). `git rev-list --all` already silently skips non-commit refs, so the commit-reachable object set is what the sweep needs. The unused `assert_all_refs_are_commits` helper is removed. Unclassifiable ref targets still fail closed at a different layer: the cat-file catch-all enumerates them with no path, and the path-based allow filter drops empty-path entries. Test inversions and new regressions: - `migration_v12_makes_cid_nullable_and_preserves_classification` and `migration_v27_clears_legacy_equal_cid` — both-distinct / distinct row expectations flipped to FALSE under the v30 strict backfill (prior round's tightening is re-affirmed here; the v30 backfill is `cid IS NOT NULL AND pinata_cid IS NULL`, the only unambiguous local-IPFS shape). - `ipfs_cid_gate_withholds_blob_from_unauthorized` — root tree assertion inverted to NOT_FOUND (the recursive structural gate denies root trees whose entries name a denied subtree). - `ipfs_cid_legacy_provider_cid_repaired_on_repush` — legacy-shape row insert now sets `local_ipfs_provenance = TRUE` to reflect that the bytes ARE on local IPFS (the legacy CID is just the wrong key). - `ipfs_cid_tree_served_despite_non_commit_ref` — root tree assertion inverted (same reason as the gate test). - `fails_closed_on_annotated_tag_of_a_blob` and `fails_closed_when_a_ref_cannot_be_traversed` — renamed to `skips_*`, assertions inverted from `Err` to `Ok` (a ref pointing at a non-commit object is no longer a walk error; the blob falls out of the gap set because `git rev-list --all` skips it). - `ipfs_cid_walk_error_fails_closed` — removed (relied on the now-removed `assert_all_refs_are_commits` to force an error; the fail-closed arm is still covered by `ipfs_cid_commit_tag_walk_error_fails_closed`, which uses a different trigger — a nonexistent ref object makes `rev-list --all` fail). - `sweep_does_not_publish_public_ancestor_tree_naming_withheld_subtree` (new) — recursive depth test: a `/public/secret/**` repo asserts root tree, `/public` tree, and `/public/secret` subtree are all absent from `pinned_cids` while `/public/visible.txt` is IPFS-pinned. - `sweep_repairs_commit_reachable_object_in_repo_with_tag_of_tree` (new) — tag-of-tree repo with a separate `mktree`'d tree (not commit-reachable); asserts the commit-reachable blob is IPFS-pinned and the unclassifiable tag-of-tree is not. Scope discipline preserved: PolicyFence capture order, `_pin_permit` reuse for the seal phase, fresh per-arm `rederive_budget`, keyset repo cursor with lookahead, mirror-row skip, Kubo 2xx-without-Hash refusal, v27 provenance-clearing migration, v30 strict backfill, v31 schema, per-(repo, backend) reconciliation offset, and the `local_ipfs_provenance` writer/reader contract are all untouched. * test(node): ignore pre-existing RED replication-tail test on CI `api::repos::tests::receive_pack_tail_survives_a_disconnect_during_release` documents an open bug in `api/repos.rs` (the replication tail must be spawned above `release` and gated on `receive_result.is_ok()`; today it's spawned below, so a disconnect during `release` drops the pins, recovery copy, and announce). The test's `assert!` message starts with "RED:" — this is a TDD-style "keep the design intent visible in the code" test, not a working-behavior assertion. The test was added 2026-07-29 by `beardthelion` (PR Twigpine#215 lineage), well before the Twigpine#218 sweep work, and is unrelated to this PR. CI is failing on it under load (passes in isolation) because the open bug hasn't been fixed yet. `#[ignore]` until the open bug is fixed. The test's design intent and the fix description stay in the doc comment and the assert message; only the CI failure is suppressed. Run the test explicitly with `cargo test -- --ignored` to verify the bug is fixed when the tail-spawn moves above `release`. * fix(node): align encrypted manifest anchor log with one-shot contract The "will retry next pass" log at reconciliation.rs:1034 was false: plan_seal returns SkipUnchanged on every subsequent pass once the recipients tag matches, so sealed stays empty and the anchor block never runs again. A transient Irys outage at the moment of a fresh seal therefore LOSES that anchor permanently — the next pass has no delta to anchor and no retry fires. This is intentionally best-effort: re-anchoring an unchanged manifest would burn Irys writes for no recovery benefit. Factored the warn message into ENCRYPTED_MANIFEST_ANCHOR_FAILED_MSG and added a unit test that pins the constant; the test goes RED if "will retry" reappears in the log. * fix(node): address eight reviewer findings on Twigpine#218 reconciliation sweep (round 3) * fix(node): close audit.toml array and drop inline comment * fix(node): repair the stale for-each-ref fake-git fixture (round 8 P1) `upload_pack_shares_one_deadline_across_walk_and_filtered_serve` — the Twigpine#174 shared-deadline guard for the FILTERED serve arm — was red on both `test (stable)` and `test (beta)`. The fake git answered `for-each-ref` with the single token `refs/heads/main`, a ref NAME, where `blob_paths` phase 2 asks for `--format=%(objectname) %(objecttype)`. The phase-2 parse fails closed on a line it cannot read, so the walk returned an error and the request surfaced as a generic 500 instead of being reaped into a 504. That silently repurposed the test from "the filtered serve shares one deadline" into "the walk errors" — the guard proved nothing while it was red. The fixture is what was stale, not the production path: the arm now emits the column shape the walk parses. A commit tip has no peeled referent, so two columns is the whole line. The 1.2s walk cost stays on `ls-tree` and the serve's on `pack-objects`, so the two phases remain independently attributable. * fix(node): peel annotated tags in the blob_paths ref walk (round 8 P1) `%(objecttype)` reports the type of the ref's OWN object, which for an annotated tag is `tag`. The phase-2 `if kind == "blob" || kind == "tree"` arms therefore never saw the referent, and a blob reachable ONLY through an annotated tag escaped the withheld set — while `rev_list_keep`'s `git rev-list --objects --all` does peel tags and served it. Phase 2 closed the leak for a direct ref-to-blob and for that shape only. The walk now asks for the peeled atoms `%(*objectname) %(*objecttype)` and inserts the referent with an empty path, alongside the direct blob/tree tips it already handled. Line parsing moves to a field-count match: two columns for a non-tag tip (the peeled atoms expand to empty), four for a tag tip, anything else still fails closed. Measured against git 2.50, the `*` atoms peel the WHOLE chain — a tag-of-a-tag-of-a-tag-of-a-blob reports the blob — so a `tag` peeled type does not occur on stock git. That arm is kept anyway, because `push_delta.rs`'s ref-type guard documents `%(*objecttype)` as a one-level peel: under such a git it finishes the chain with `rev-parse <tip>^{}` plus a `cat-file -t` probe rather than bailing and 500ing every clone of a repo that merely carries a nested tag. Both children are bounded by the walk's shared deadline and are unreachable on the common one-line-per-ref path. * fix(node): one empty-path decision for both blob_paths consumers (round 8 P1) `blob_paths` has two consumers and only one of them carried the empty-path policy. `withheld_from_pairs` (the deny side, what the smart-http serve filter excludes) withheld a phase-2 empty-path entry from everyone but the owner; `allowed_blob_set_for_caller_bounded` (the allow side, what `GET /ipfs/{cid}` hands over) called `visibility_check(..., "")` directly. On a public repo no glob matches the empty path, so that returns `Allow`: the serve filter withheld the OID while the IPFS gate served the bytes — the leak phase 2 exists to close, reopened one layer over. The policy moves into `pair_decision`, which both consumers now call, so the two gates cannot disagree and a future change to the empty-path rule moves both at once. `withheld_blob_recipients_bounded` routes through it too: an unclassifiable blob grants a recovery copy to the owner only, never to a rule's named reader whose grant was written against a path this object does not have. No behavior change for non-empty paths — `pair_decision` delegates to `visibility_check` unchanged. * test(node): bind the phase-2 ref tests to a commit-unreachable blob (round 8 P2) `skips_a_ref_pointing_at_a_blob` and `skips_an_annotated_tag_of_a_blob` were vacuous. Both hung their ref on `fixture()`'s `secret` blob, which is COMMITTED at `secret/b.txt`, so phase 1's per-commit `ls-tree` already produced `(secret, "/secret/b.txt")`, the `/secret/**` rule already denied it, and `withheld.contains(&secret)` passed with phase 2 deleted outright. Both now use `orphan_blob`, a blob written straight to the object store that no commit's tree names, so the ONLY route into the withheld set is the `for-each-ref` phase. That is also the real leak shape: `rev_list_keep`'s `git rev-list --objects --all` does follow a ref to such a blob, so an under-withheld one ships in the clone pack. Verified by mutation: with `if false` injected at the phase-2 filter, every test below goes RED (they were green under that mutation before). New coverage: - `ref_only_blob_is_denied_on_the_allow_side_too` — the allow-side denial for the shared `pair_decision`; it also asserts deny and allow agree on one OID, and that the owner IS still admitted, so it cannot pass by denying everything. Reverting the allow side to `visibility_check` turns it red on exactly that assertion. - `skips_a_nested_annotated_tag_of_a_blob` — real-git tag-of-a-tag. - `peels_a_tag_whose_peeled_target_is_still_a_tag` — drives the one-level-peel fallback through the fake-git seam, so that arm is tested rather than dead. * fix(node): advance the reconciliation continuation only for dispatched work (round 8 P2) `ipfs_last` / `pinata_last` were captured from the missing set above the pin permit, both `PolicyFence` captures and both pin loops, then written under nothing but `if ipfs_enabled` / `if pinata_enabled`. Every stage in between can legitimately produce nothing — a fence capture that fails, a quarantine/visibility recheck that says skip, a pin-boundary re-derivation that errors — and each is transient. Advancing past OIDs that were never attempted is not a harmless retry delay: `missing_oids` rotates strictly past the stored offset, so the whole unattempted prefix lands BEHIND the entire backlog on the next pass. For an at-cap repo — the only kind the continuation exists for — the backlog never drains inside one cap window, so those objects are starved indefinitely. That is a durability hole in the durability backstop. The offset now moves only for work actually handed to a backend, tracked at the dispatch boundary as `to_pin.last()` (not the missing set's last: an OID the pin-boundary re-derivation dropped was never offered). It is recorded before the call, so a pin phase that times out mid-batch still counts as dispatched. The write becomes three outcomes rather than two, via one `next_offset_write` helper so the backends cannot drift apart: * work dispatched -> advance to the last dispatched OID * nothing missing, query OK -> None, marking the row done * nothing dispatched, or the missing-set query failed -> write NOTHING, leaving the resume point a capped pass already paid for `*_scan_ok` is what separates the last two: an empty missing set means "everything is pinned" or "the filter query failed and we know nothing", and only the first should mark the row done. Test: `sweep_leaves_offset_untouched_when_nothing_was_dispatched`, using a `#[cfg(test)]` seam on the pin-boundary re-derivation — that stage cannot be starved from the outside, because the mid-scan re-filter runs first on the same rules and budget and makes the sweep `continue` well before the offset write. It asserts the stored offset comes back byte-identical after a declining pass, then that the same repo does advance it once dispatch happens, so the assertion cannot pass on a dead write site. Restoring the old semantics turns it red. * fix(gl): render the Pinata provider CID in `gl ipfs list` (round 8 P2) The server-side wire change landed without its client half. `cmd_list` read `pin["cid"].as_str().unwrap_or("?")` as the entry heading and nothing in the CLI read `pinata_cid` at all, so a Pinata-only row printed as a bare `?` — the listing showed nothing usable for an object that is in fact durably stored. `cid` is legitimately null for such a row: it is the key `GET /ipfs/{cid}` resolves against, and the node stopped aliasing a Pinata provider CID into it (round 3) precisely because that CID 404s there. So the fix is on this side. Rendering moves into `render_pin`, which leads with the node-resolvable CID when there is one and otherwise the provider CID, labelled so nobody feeds it back to this node's resolver. A dual row keeps both visible. A `backends:` line names where the bytes are, read from the writer-owned `local_pinned` / `pinata_pinned` booleans rather than re-inferred from CID shape — which is what the node's own comment warns against. Older nodes send neither flag; those `cid`-only rows fall back to CID presence and render as they always did (`render_pin_handles_a_legacy_cid_only_row`). * fix(node): implement third fence check for IPFS pinning and enhance visibility rules handling * fix(node): enhance documentation and add tests for ProgressState and helper functions * fix(node): address round-9 reviewer findings on Twigpine#218 reconciliation sweep * fix(node): correct consumer-4 recipient test for call-invariant set * fix(git): fail closed on non-UTF-8 tree listing in walk_tree_oids_inner (Twigpine#218 round 10 P1) walk_tree_oids_inner previously returned Ok(()) on a non-UTF-8 ls-tree -z listing, inserting only the tree OID and skipping every child blob/tree OID. A direct tree ref (or an annotated tag peeling to a tree) is valid Git input; git rev-list --objects --all still enumerates the tree and every descendant, so the keep-side removed the tree from the served set but passed the child blob OIDs to pack-objects, exposing their bytes to an anonymous clone. Phase 1 already bails on the same input at the blob_paths walk; this fix puts the tree-walk on the same fail-closed outcome. The new fails_closed_on_non_utf8_tree_tip test pins the invariant for both a direct refs/tags/direct-tree ref and an annotated tag-of-tree (the two non-commit shapes round 9 added tolerance for), confirming the walker bails rather than returning Ok with a partial withheld set. * fix(recon): run encrypted recovery even when public list is empty (Twigpine#218 round 10 P1) A path-scoped repo whose only reachable object is a direct blob or direct tree ref yields an empty public list (the anonymous public classifier removes the only object from the served set) while withheld_blob_recipients_bounded still assigns that object to the owner recovery set. The early `continue` on object_list.is_empty() (and the second one after the mid-pass refilter) suppressed encrypted recovery too, and a lost or failed encrypted copy was never repaired. Track empty-public-work as a flag and gate the public phase on it. Encrypted phase 2 runs regardless. The backend enable flags and `_pin_permit` move out of the gated block so phase 2 can read them when the public phase did not run. * fix(db): rename migration v12 test to v32 (Twigpine#218 round 10 P3) The test name and its docstring/comments still say "v12", but the cid-nullable migration is at v32 (line 1163). The test itself is correct — it drives the real path — so this is naming drift rather than dead coverage. A future reader looking for the migration would otherwise chase a v12 that no longer exists. * fix(git): index OID membership before quadratic classifier scan (Twigpine#218 round 10 P2) all_object_paths was doing O(O×P) work in two places: the catch-all cat-file branch checked `blob_set.iter().any(|o| o == oid)` and `tree_set.iter().any(...)` for every reachable object, and the phase 1 loop also did a per-commit unused `git rev-parse <commit>^{tree}` probe whose result was discarded (the root-tree OID is recomputed by `root_tree_oids`). On the 50k-object repos the sweep exists for, the quadratic scan and the redundant per-commit child process can consume each authorization deadline before any object reaches a backend, producing a permanent hourly skip. Maintain a separate `blob_oids` / `tree_oids` HashSet alongside the path-pair sets so the membership check is O(1). Drop the unused per-commit rev-parse probe. * fix(git): bound tree walk by ls-tree invocation count, not just depth (Twigpine#218 round 10 P2) walk_tree_oids_inner stopped at MAX_TREE_WALK_DEPTH and the shared deadline. A wide shallow tree reachable from N ref tips or from multiple parents still spawned one ls-tree child process per subtree well inside the depth cap, and the wall-clock bound could not stop that — expiry was the only stop. The ceil was wall-clock only. Add a structural invocation cap (MAX_TREE_WALK_INVOCATIONS) the walker fails closed at, and a memo of walked tree OIDs so the same tree reachable from multiple ref tips is walked once. Round 10 P2. * fix(pinata): suppress push when DB record is not durable (Twigpine#218 round 10 P2) pin_new_objects pushed (sha, cid) on provider POST success regardless of whether record_pinata_cid persisted the row. A closed/failed DB write or an explicit-transaction source record timeout meant the next sweep queried the DB, found no row, and re-offered the same gap — with reconciliation having counted a fill that never happened. Track a `db_record_durable` flag. On hard failure of either record_pinata_cid (non-timeout) or record_pin_source (any failure, since it's a multi-statement transaction), set the flag false and skip the `pinned.push`. The `BoundedDbError::Elapsed` autocommit case stays as the previous comment's "may have committed" reasoning: the push fires so the reconcile still sees the fill. Round 10 P2. * fix(node): serialize policy narrow with irreversible upload via in-process mutex (Twigpine#218 round 10 P1) The PolicyFence's epoch read was a one-way read: a visibility narrow (rule insert, quarantine set) could commit between the fence's epoch read and the upload's HTTP POST, and the upload's DB-record INSERT's epoch check would see the new epoch and reject — but the provider already accepted the object. Encrypted envelopes had no fence at all. Add a per-repo in-process mutex that PolicyFence acquires at capture and releases at drop. The narrow paths (set_repo_quarantine, set_visibility_rule, remove_visibility_rule) acquire the same lock for the duration of their DB transaction. A narrow therefore blocks until the in-flight batch finishes, and an in-flight batch waits for the narrow to commit before it even reads the epoch. Multi-process / multi-node is a known gap: a Postgres advisory lock per repo would extend the same guarantee across processes. Deferred to a follow-up. * fix(pinata): update post-upload stalled record test for round 10 P2 contract The previous test asserted that a successful upload with a timed-out post-upload source record still returned the (sha, cid) pair. That assertion pinned the buggy behavior the reviewer called out: the reconcile would count a fill that was not durable, then re-offer the same gap. The test name and the `record_pin_source` arm of `pin_new_objects` are unchanged; the assertion now reflects the new contract: when the post-upload source record times out (its `mark_pin_sources_incomplete` already sets the source set to incomplete, so the next pass re-offers), the pair is suppressed so the reconcile sees the gap as not-filled. Round 10 P2 follow-up. * fix: clippy allow + rustfmt on round 10 commits clippy::too_many_arguments on walk_tree_oids_inner (8 args after the round 10 memo + invocation counter threading). rustfmt re-ran on the if/else blocks added in the has_public_work refactor (round 10 P1 Twigpine#42). No behavior change. * fix(node): address round-11 review findings on Twigpine#218 reconciliation sweep P1: remove the whole-batch policy mutex. PolicyFence no longer holds a per-repo lock from capture through drop and the narrow paths (set_visibility_rule, remove_visibility_rule, set_repo_quarantine) no longer acquire it, so a visibility narrow commits immediately and the batch aborts on its next is_current check instead of the narrow blocking behind the sweep. Accepted residual is a single in-flight object; the fenced DB record still refuses to land a raced row as durable. encrypt_and_pin_stops_sealing_when_reader_removed_mid_batch is green again. PolicyMutexes is deleted, resolving the key-leak P2 by removal. P2: cover the round-10 has_public_work branch with sweep_seals_withheld_blob_when_public_list_is_empty: a direct-blob-ref repo yields an empty public list with a non-empty owner recovery set; asserts no public gaps/fills, no cleartext pin, exactly one POST (the seal envelope), and a recorded encrypted copy. P3: peel-case isolation in the non-UTF-8 tree-tip test (delete the direct-tree ref before the second call so the peel arm must run); v12->v32 rename drift in the migration test (boundary, comments, and the stale (2)->true doc line); accurate comments for the pre-refilter flag and the post-refilter fall-through. * fix(node): address merge-readiness findings on Twigpine#218 reconciliation sweep Reworks the sweep around explicit state-machine contracts instead of adding conditionals: backend results carry last_attempted plus confirmed durability separately (PinBatchOutcome), provider permits scope to provider effects only, discovery pages by commit window with a persisted cursor, legacy provenance migrates as unknown, and the pin list advertises only validated raw local keys. - Empty-path owner recovery without path rules: phase 2 drops the has_path_scoped_rule gate and runs on the actual recipient result, with a dedicated recovery recheck (no anonymous-listability requirement); matrix test over blob/tree/tag refs x zero/root rules, including recreate-after-delete. - Pinata Elapsed is non-durable unless verify_pinata_record proves the exact row and fence epoch; loop test with row-locked repos. - list_pins gates cid/local_cid on is_raw_cidv1 plus confirmed provenance; response-shape matrix uses real CIDs with a legacy row and GET /ipfs follow-through; docs match the nullable wire contract. - Permits acquired per provider effect and dropped after; no nesting at pool size 1 (test renamed to the new shape). - Scan discovery pages SCAN_COMMIT_WINDOW commits oldest-first with a node_state cursor (advance on drained/empty/attempted, delete on cover); refilters and phase-2 recipients run on the same window; walk-layer paging/invocation/union tests plus a 1050-commit cursor-to-completion sweep test. - v35 migrates legacy rows as unknown (no TRUE backfill); v35 test asserts unknown + preserved Pinata history; sweep re-derivation test re-pins instead of filtering. - Drained completions DELETE continuation rows (no tombstones); FK cascade on repo delete; row-count regression test. - Audit: lru waiver gets an explicit retirement owner plus a weekly drift guard; revocation model recorded as a product decision on PolicyFence (prompt revocation, single-in-flight residual). * fix(tests): configure git identity in bare clone for tag fixture The windowed-union test creates its annotated tag in the bare clone, which carries no config. It passed locally only via the developer's global git identity; CI has none, so 'git tag -a' failed there. Verified with an empty GIT_CONFIG_GLOBAL/SYSTEM. * fix(tests): ignore pre-existing hung-git pid race on CI read_object_bounded_returns_by_deadline_with_a_hung_git reads the fake's pid file with no readiness handshake; a slow scheduler yields 'No such file or directory' (stable-only red, identical beta green). Unrelated to Twigpine#218 (store.rs untouched); re-enable with a readiness signal from the fake. * fix(node): address round-12 review findings on Twigpine#218 reconciliation sweep - P1 roots: admit structurally safe root trees at "/" into the allow set and candidates, so safe snapshots fully replicate; roots naming a withheld subtree stay excluded via the recursive structural check. - P1 per-pass WalkBudget on sweep enumeration (entries / output bytes / git invocations) with fail-closed budget-exhaustion; serve paths keep the uncapped budget. - P2 recovery lane: independent recovery cursor pages unlistable history across restarts instead of sharing the scan window. - P2 gap-hold: failed gap queries hold discovery advance but not the healthy backend, via test-only failure seams. - P2 nested tags: collect full tag chains so nested tag objects pin without an inner ref. - P2 RepoStore: thread the Tigris-backed store into the sweep with an acquire timeout and shutdown select; restore cache-miss and skip missing. - P2 Pinata source: keep confirmed pairs when the primary already covers the repo; suppress push for stalled post-upload records without coverage. - Update large-history sweep expectation to blob+root+commit per round. * fix(node): address consolidated review findings on Twigpine#218 reconciliation sweep P1: wire WalkBudgetExceeded through the sweep cursor logic. Both the public scan and the recovery walk now dispatch the typed budget-exhaustion signal at the sweep boundary: a budget-hit window advances past content the sweep refuses to classify partially (scan cursor save/delete via the existing persist site; recovery cursor likewise), while git/policy/panic/timeout failures keep the cursor for retry. visibility_pack documents the obligation; the match arms are the handler, with a one-line pattern note for future typed walk errors. P2: run encrypted recovery when the public scan fails. Scan walk/panic/timeout arms now fall through with an empty public list instead of continuing the whole repo iteration, so phase 2 runs on the recovery lane's own cursor like the unlistable-repo path. The scan cursor stays untouched for retry. Refilter/recheck-public failures remain fail-closed whole-iteration continues by design. Includes a repo-scoped test seam (process-global set, safe across spawn_blocking and parallel tests via fresh UUIDs). P2: bump policy_epoch only when remove_visibility_rule deletes a row, mirroring set_repo_quarantine's rows-affected guard, so a typo'd or double-deleted glob cannot abort fenced batches. Also adds the repo-loop outcome matrix (every early exit × scan cursor / recovery cursor / phase 1 / phase 2) and deny tests: budget-hit advances both lane cursors with zero pins/seals (real 250k-entry trip, early-trip fixture runs in ~30s), scan failure still seals the recovery copy, delete-missing-rule leaves the epoch unchanged. Each deny was verified by temporarily restoring the old branch. * fix(node): address jatmn review findings on reconciliation sweep re-home * fix(node): reload persisted sweep cursor between passes * fix(node,gl): pinata offset warn wording and boundary-safe pin timestamp --------- Co-authored-by: Gravirei <gravirei@users.noreply.github.com> Co-authored-by: Kevin Codex <kevin@gitlawb.com>
What breaks
cargo auditfails on every branch as of today:The advisory was published after
main's last green run, so theSUCCESSticks currently showing on open PRs are stale cached results, not evidence. Anything that re-runs CI goes red until this lands — #331 already has.h2arrives transitively viahyper, so there is no manifest change. The lockfile is the entire fix.Why this is hand-edited
cargo update -p h2on my machine produced the h2 bump plus nine unrelated rewrites, all downgrades:syndata-encoding-macro-internalwindows-syserrno(×3)socket2--precise 0.4.16produced the identical set, so it is not collateral re-resolution from an unpinned update — it is local crates.io index drift against whatever generatedmain's lockfile. Those downgrades have nothing to do with the advisory and do not belong in a security bump, so this commit changes only h2'sversionandchecksumlines.That
cargo check --lockedaccepts the result is the proof that 0.4.16 introduces no dependency changes of its own — a hand-edited lockfile that got that wrong would fail--lockedimmediately.Worth someone re-running
cargo updateon a Linux box to see whether those nine entries are real drift inmain's lockfile or an artifact of my toolchain. Either way it is a separate question from this fix.Verification
cargo check --locked --workspace— exit 0cargo tree --locked -p h2— resolvesh2 v0.4.16gitlawb-core104,gl328,git-remote-gitlawb58, all passingcargo check --locked --workspace --all-targetscannot run frommainon Windows — the unix-only test code does not compile there, which is exactly what #330's first commit fixes. Linux CI covers that path.Scope
Deliberately kept to the advisory alone rather than folded into #331, so it can land on its own and unblock every open PR at once.