fix(node): gate GET /ipfs/{cid} tree objects so a withheld subtree's structure can't leak (#135)

[beardthelion]

What

GET /ipfs/{cid} served tree objects of a withheld subtree to callers denied that subtree. A git tree body is <mode> <name>\0<raw-oid> per entry, so fetching the tree CID of a withheld directory returned every child filename and child oid in cleartext, recursively. Blob content was already protected; this closes the structure leak so the CID surface matches what get_tree enforces on the REST path.

Approach

Tree objects are now gated against a caller-aware allowed-tree-set, the mirror of the existing allowed_blob_set_for_caller:

• A shared object_paths walk (git ls-tree -rzt per reachable commit) that blob_paths and the new tree_paths both filter, so the per-path classification is derived once rather than twice. blob_paths output is byte-identical, so its callers are unaffected. The two gates are not identical, and deliberately so: blob_paths runs assert_all_refs_are_commits and fail-closes a repo's whole walk when any ref peels to a non-commit, while tree_paths goes through reachable_commit_oids, which tolerates such a ref and simply excludes what is reachable only through it. On clean fixtures they agree, and a unit test pins that; on a repo carrying pushable non-commit refs they diverge, which is the availability tradeoff that keeps one annotated tag of a tree from 404ing every CID in the repo.
• tree_paths is the kind == "tree" slice plus each reachable commit's root tree (resolved in one git log --format=%T pass, since ls-tree never emits a commit's own root).
• get_by_cid gates a blob against the allowed-blob-set and a tree against the allowed-tree-set (lazy per repo, off the async runtime, fail-closed on any walk error). Commits and tags stay served: they expose only root-level metadata the caller already cleared the / gate for.

The withheld directory's own tree (path /secret) is denied, not just its descendants, so parity with get_tree holds.

Reachability and scope

get_by_cid resolves a CID to its git oid through the pinned_cids table, so the route is live on any node that has pins, whatever its object format. An earlier revision of this branch treated the CID digest as an oid directly, which only matched in sha256 repos and made the endpoint close to dormant against --object-format=sha1 production repos; that is no longer how it resolves, so treat this gate as covering a reachable surface rather than a pre-emptive one. The replication/pin path exports the same withheld-tree structure to IPFS independently of the object format and is tracked separately in #172.

Tests

Deny paths driven through the real handler:

• Withheld subtree tree CID returns 404 for anon and non-readers; listed reader and owner still read it (200).
• Root and ancestor trees, commits, and tags stay served; a dangling tree fails closed for anon and owner; a tree with no path-scoped rule is served.
• The withheld directory's own path denies (glob parity with get_tree).
• Content-dedup: a tree reachable at both an allowed and a withheld path is served (allowed wins).
• blob_paths output is asserted byte-identical after the walk refactor.

Closes #135.

Summary by CodeRabbit

• New Features

  • Added improved per-path IPFS visibility controls that gate directory trees and file blobs.
  • Enhanced /ipfs/{cid} to resolve via pinned CID → git-object OID mapping with visibility-aware tree handling.
  • Introduced a per-client rate limiter for IPFS full-history walk requests.

• Bug Fixes

  • Strengthened fail-closed behavior so unverifiable access and dangling objects are withheld (404).
  • Hardened object enumeration/parsing to prevent visibility leaks from malformed/unreachable data.

• Tests

  • Expanded CID gating coverage with raw-byte tree verification, updated fixtures, and added denied-directory and dangling-tree fail-closed tests.

• Chores

  • Added a database index and CID→OID lookup helper to speed up resolution.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Path-scoped IPFS visibility now applies to blob and tree objects using caller-specific reachable allow-sets. CID resolution uses pinned_cids, traversal fails closed, full-history walks are rate-limited, and tests cover withheld, allowed, root, shared, and dangling objects.

Changes

Path-scoped object visibility

Layer / File(s)	Summary
Pinned CID resolution crates/gitlawb-node/src/db/mod.rs, crates/gitlawb-node/src/api/ipfs.rs	CID pins are indexed and resolved to Git OIDs through pinned_cids before repository scanning.
Reachability and allow-set computation crates/gitlawb-node/src/git/visibility_pack.rs, crates/gitlawb-node/src/visibility.rs	Shared fail-closed traversal enumerates reachable commits, blob paths, tree paths, and root trees for caller-specific allow sets, including subtree own-path rules.
IPFS blob and tree gating crates/gitlawb-node/src/api/ipfs.rs	The handler memoizes separate blob and tree allow sets and skips repositories when computation fails or panics.
Rate limiting and regression coverage crates/gitlawb-node/src/main.rs, crates/gitlawb-node/src/state.rs, crates/gitlawb-node/src/test_support.rs, crates/gitlawb-node/src/git/visibility_pack.rs	IPFS full-history walks use per-source limits, while tests cover pinned CIDs, withheld and dangling trees, raw tree contents, metadata objects, and rate-limit behavior.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant IPFSHandler
  participant Database
  participant VisibilityPack
  participant GitRepo
  Caller->>IPFSHandler: Request CID
  IPFSHandler->>Database: Resolve CID through pinned_cids
  Database-->>IPFSHandler: Candidate Git OIDs
  IPFSHandler->>VisibilityPack: Compute caller blob or tree allow-set
  VisibilityPack->>GitRepo: Enumerate reachable commits and paths
  GitRepo-->>VisibilityPack: Reachable object paths
  VisibilityPack-->>IPFSHandler: Allowed object OIDs
  IPFSHandler-->>Caller: Serve object or return 404

Loading

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related issues

• GET /ipfs/{cid} serves tree/commit objects of withheld subtrees, leaking structure get_tree protects (KTD3) #135 — Extends /ipfs/{cid} visibility gating from blobs to withheld tree objects.
• Availability: unauthenticated GET /ipfs/{cid} runs one blocking git cat-file per readable repo #164 — Replaces per-repository CID probes with indexed pinned_cids lookup and adds IPFS rate limiting.
• Replication/pin path exports withheld-subtree tree objects to public IPFS (write-side of #135) #172 — Applies related tree visibility rules to replication and pinning workflows.

Possibly related PRs

• Gitlawb/node#128 — Established per-caller path-scoped gating in the IPFS handler.
• Gitlawb/node#133 — Introduced reachable caller-aware allow-set gating extended here to trees.
• Gitlawb/node#90 — Directly relates to the pinned_cids data flow used for CID resolution.

Suggested labels: sev:high, kind:security, subsystem:api

Suggested reviewers: jatmn

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely identifies the main fix: gating tree objects on GET /ipfs/{cid} to prevent withheld subtree structure leaks.
Description check	✅ Passed	The description clearly states the bug, motivation, implementation, scope, tests, and issue link; it omits template checklists and exact verification commands but remains substantively complete.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-135-ipfs-cid-tree-gate

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (2)

crates/gitlawb-node/src/api/ipfs.rs (1)

143-209: 🚀 Performance & Scalability | 🔵 Trivial

The blob/tree gating, memo selection, and fail-closed arms look correct.

One operational note: /ipfs/{cid} is reachable anonymously, and under path-scoped rules each request against an object that exists in a repo triggers a full-history reachability walk (one git ls-tree -rzt per reachable commit, plus the root-tree pass for trees). The memo is request-scoped only, so a spray of valid blob/tree CIDs against a large-history repo re-runs the walk on every request. Consider a bounded cross-request allow-set cache (keyed by repo id + head oid + caller) and/or rate limiting on this route to cap the cost.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/gitlawb-node/src/api/ipfs.rs` around lines 143 - 209, Mitigate
repeated full-history walks in the /ipfs/{cid} path by adding a bounded
cross-request cache for computed blob/tree allow-sets, keyed by repository ID,
current head OID, object type, and caller identity; invalidate or naturally
bypass entries when the head changes. Implement this around
allowed_blob_set_for_caller, allowed_tree_set_for_caller, and the existing memo
lookup, and consider adding rate limiting for anonymous requests to further cap
abuse.

crates/gitlawb-node/src/git/visibility_pack.rs (1)

391-416: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid ARG_MAX in root_tree_pairs.

Passing every reachable commit to git log on argv scales with history size and can fail on very large repos. When that happens, the tree CID path for path-scoped rules skips the repo and the object falls through to a 404. Feed the commits over stdin instead (git log --stdin --no-walk=unsorted --format=%T).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/gitlawb-node/src/git/visibility_pack.rs` around lines 391 - 416,
Update root_tree_pairs to avoid placing all commit IDs in argv: invoke git log
with --stdin alongside --no-walk=unsorted and --format=%T, write the commits
joined by newlines to the child process stdin, and handle stdin/command errors
consistently with the existing context and status checks. Remove the argument
expansion of commits while preserving the existing tree-pair parsing.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/gitlawb-node/src/api/ipfs.rs`:
- Around line 143-209: Mitigate repeated full-history walks in the /ipfs/{cid}
path by adding a bounded cross-request cache for computed blob/tree allow-sets,
keyed by repository ID, current head OID, object type, and caller identity;
invalidate or naturally bypass entries when the head changes. Implement this
around allowed_blob_set_for_caller, allowed_tree_set_for_caller, and the
existing memo lookup, and consider adding rate limiting for anonymous requests
to further cap abuse.

In `@crates/gitlawb-node/src/git/visibility_pack.rs`:
- Around line 391-416: Update root_tree_pairs to avoid placing all commit IDs in
argv: invoke git log with --stdin alongside --no-walk=unsorted and --format=%T,
write the commits joined by newlines to the child process stdin, and handle
stdin/command errors consistently with the existing context and status checks.
Remove the argument expansion of commits while preserving the existing tree-pair
parsing.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 335f1ebc-783c-4552-bfd6-ebc5894e4d9a

📥 Commits

Reviewing files that changed from the base of the PR and between 2109d08 and 89d4928.

📒 Files selected for processing (4)

• crates/gitlawb-node/src/api/ipfs.rs
• crates/gitlawb-node/src/git/visibility_pack.rs
• crates/gitlawb-node/src/test_support.rs
• crates/gitlawb-node/src/visibility.rs

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P1] Make the CID tests and lookup use the identifier published by the pin path
  crates/gitlawb-node/src/test_support.rs:2064
  These new assertions request cid_for_oid(...), whose multihash is a Git object ID. The real pin path instead stores CID(sha256(raw object content)), and gl ipfs get sends that stored CID back to this handler. Even with SHA-256 Git repositories those values differ because Git hashes "<type> <len>\\0" + content; get_by_cid therefore treats a real pinned CID as a nonexistent OID and returns 404 before this new tree gate runs. Please make the serving lookup use the same CID-to-object mapping as pinning (or make the two identifiers deliberately identical), and cover a CID produced from the fixture object's raw bytes rather than encoding its OID.

• [P2] Do not pass the complete history as git log arguments
  crates/gitlawb-node/src/git/visibility_pack.rs:395
root_tree_pairs adds every reachable commit to the process argv. On a long history this exceeds ARG_MAX (about 32k SHA-256 OIDs on a 2 MiB limit), so spawning git log fails; the handler treats that walk error as a denial and returns 404 even to an authorized caller requesting a reachable/root tree. Please complete CodeRabbit's pending root-tree request by feeding commit IDs through git log --stdin or by batching/root-resolving them during the existing per-commit traversal.

[beardthelion]

Both addressed as of 52b81fd.

P1 (CID → object mapping). You're right that the lookup and the tests diverged from the pin path. get_by_cid was treating the CID's sha2-256 digest as the git oid, but the pin path stores CID(sha256(raw content)), which never equals the oid (git frames the object as "<type> <len>\0" + content before hashing), so a real pinned CID 404'd before the tree gate ever ran. get_by_cid now resolves the incoming CID to its oid through the pinned_cids table (oid_for_cid, backed by an indexed column added as a versioned migration) and gates on that oid; a CID never pinned here is an opaque 404, uniform with a genuine not-found and a visibility denial. The tree-gate tests now build the request CID the way the pin path does (read the object's raw bytes, Cid::from_git_object_bytes, record the pin) rather than encoding the oid, so they exercise the gate on a real production CID. They fail against the old digest-as-oid handler and pass after the fix.

P2 (git log argv). root_tree_pairs now feeds the commit oids to git log --no-walk=unsorted --format=%T --stdin on stdin instead of argv, so a long history can no longer overflow ARG_MAX and fail-close an authorized reader of a reachable/root tree. The oids are written from a separate thread while the main thread drains stdout, so a large input and output can't deadlock on the pipe buffers; a 2500-commit test covers it.

Rebased onto main.

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P1] Bound the new tree visibility walk on the public retrieval route
  crates/gitlawb-node/src/api/ipfs.rs:173
  Every request for a known tree CID in a repo with a path-scoped rule recomputes the complete allowed-tree set: rev-list, one recursive ls-tree per reachable commit, and a root-tree pass. The memo only lasts for that one request, while /ipfs/{cid} is anonymous and the public pins index exposes valid CIDs. An unauthenticated caller can therefore repeat a tree-CID request and saturate the blocking pool/CPU with unbounded full-history work. Please add a bounded, revision-aware cross-request cache and/or a route-level work/rate limit before enabling this path.

• [P2] Try every object recorded for a content CID
  crates/gitlawb-node/src/db/mod.rs:2188
  The new index deliberately permits duplicate CIDs, but this unordered LIMIT 1 chooses only one mapped OID and the handler never tries another. CIDs here hash untyped raw object bytes, so a tree and a blob containing its raw tree bytes have different Git OIDs but the same CID; both can be pinned. If PostgreSQL chooses a withheld, stale, or absent object while another mapped object is readable, the endpoint returns 404 for a valid advertised CID. Return all matching OIDs and run each through the existing repository/visibility checks (with collision coverage) rather than selecting one arbitrarily.

• [P2] Canonicalize parsed CIDv1 values before the database lookup
  crates/gitlawb-node/src/api/ipfs.rs:92
CidGeneric::from_str accepts valid CIDv1 multibase encodings, but this lookup uses the original request spelling. Pins are stored under the canonical base32 string produced by Cid::from_git_object_bytes(...).to_string(), so an equivalent base58/base64 CID passes validation yet misses pinned_cids and returns 404. Look up cid.to_string() (or a binary canonical key) and add an alternate-encoding retrieval test.

[beardthelion]

Ready for re-review on 1e5035c. Both findings are in, and I re-verified them by execution on this head:

• P1 (CID identity). get_by_cid resolves the incoming CID to its git oid via pinned_cids (oid_for_cid) and gates on that oid; a never-pinned CID is an opaque 404. The tree-gate tests build the request CID the pin-path way (Cid::from_git_object_bytes), so they run against a real production CID rather than the oid, forcing the gate open flips a withheld subtree from 404 to a served leak.
• P2 (argv overflow). root_tree_pairs feeds the commit oids through git log --stdin instead of argv, so a long history can no longer overflow ARG_MAX and fail-close an authorized reader; the stdin path returns every root tree at scale.

Ready for another look.

[beardthelion]

Addressed all three on 2fa6449, each verified by execution (reverting the fix reproduces the finding, the fix clears it):

[P1] Bound the tree-visibility walk. The full-history allowed-set walk is now rate-limited per source IP, checked once immediately before it spawns (the resource sink), reusing the same RateLimiter and trusted-proxy key as the push brake (GITLAWB_IPFS_RATE_LIMIT, default 600/hr, 0 disables, bounded key map). It fires only on a real walk: a request-memo hit or a cheap non-path-scoped fetch is never braked, and the key is the client IP, not the farmable DID. ipfs_walk_rate_limited_per_source covers the shed (2nd walk 429), per-source isolation (a second IP still 200), and the must-not (a public non-walk fetch from the exhausted IP still 200).

[P2] Try every object recorded for a content CID. oid_for_cid's LIMIT 1 is replaced by oids_for_cid, which returns every mapped oid; the handler runs each through the existing repo/visibility loop and serves the first readable one, so a withheld, stale, or absent duplicate no longer 404s a CID that has a readable object. Covered by oids_for_cid_returns_all_duplicates and a collision test that pins an absent oid first and a readable one second under the same CID.

[P2] Canonicalize CIDv1 before the lookup. The lookup keys on cid.to_string() (canonical base32) instead of the request spelling, so an equivalent base58/base64 CID resolves. ipfs_alt_encoding_cid_resolves sends a base58 spelling of a pinned CID.

Full node suite green (502).

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/gitlawb-node/src/api/ipfs.rs (1)

143-159: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Stale invariant comment: "one request builds exactly one of the two sets."

The tree analog (#135): a withheld subtree's tree object is gated the same way a withheld blob is, so its structure cannot leak by CID where get_tree protects it. The adjacent claim that Built lazily and only for a tree fetch (a request is one CID = one object type), so one request builds exactly one of the two sets — no double walk no longer holds: the new multi-candidate oid loop can resolve a single CID to both a blob and a tree oid in the same repo (the documented CID-collision case in db/mod.rs's oids_for_cid), which would populate both allowed_blob_memo and allowed_tree_memo within one request.

Update the comment to reflect that both sets can now be built in the collision case.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/gitlawb-node/src/api/ipfs.rs` around lines 143 - 159, Update the
comment above allowed_blob_memo and allowed_tree_memo to remove the claim that
one request builds exactly one set; state that the sets are built lazily and
that both may be populated when a CID resolves to both blob and tree candidates.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/gitlawb-node/src/api/ipfs.rs`:
- Around line 165-279: The walk_rate_checked guard in the per-object gating flow
only limits the first spawn_blocking walk, allowing subsequent repo/type walks
within one request. Replace the one-time check with per-request walk accounting
and enforce a cap before every allowed_blob_set_for_caller or
allowed_tree_set_for_caller invocation, rejecting or skipping once exhausted
while preserving the existing IP limiter check and fail-closed walk handling.

In `@crates/gitlawb-node/src/main.rs`:
- Around line 328-345: The periodic cleanup task must also invoke cleanup on
ipfs_rate_limiter. Update the cleanup loop to call its cleanup method alongside
the other rate limiters, ensuring expired source-IP entries are removed
regularly and the 200,000-key bound does not retain stale clients.

---

Outside diff comments:
In `@crates/gitlawb-node/src/api/ipfs.rs`:
- Around line 143-159: Update the comment above allowed_blob_memo and
allowed_tree_memo to remove the claim that one request builds exactly one set;
state that the sets are built lazily and that both may be populated when a CID
resolves to both blob and tree candidates.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 06879bbd-f899-4792-9943-5bc38b5533fa

📥 Commits

Reviewing files that changed from the base of the PR and between 1e5035c and 2fa6449.

📒 Files selected for processing (6)

• crates/gitlawb-node/src/api/ipfs.rs
• crates/gitlawb-node/src/auth/mod.rs
• crates/gitlawb-node/src/db/mod.rs
• crates/gitlawb-node/src/main.rs
• crates/gitlawb-node/src/state.rs
• crates/gitlawb-node/src/test_support.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/gitlawb-node/src/test_support.rs

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P1] Bound every full-history walk, not just the first one in a request
  crates/gitlawb-node/src/api/ipfs.rs:216
walk_rate_checked consumes one IP quota token and then remains true while the nested CID-candidate × repository loops continue. A shared pinned blob/tree CID can be present in any number of public path-scoped repositories (and a CID can intentionally have multiple OID candidates); when the caller is denied in each, the request starts one allowed_*_set_for_caller full-history walk per repository after paying for only the first. Thus 600 requests per IP can still cause 600 × N rev-list/ls-tree walks and exhaust the blocking pool/CPU. Charge or cap every spawned walk (and add a multi-repository regression) rather than treating the first check as a request-wide authorization for unbounded work.

• [P1] Do not leave resolvable pinned CIDs on the unbounded all-repository probe path
  crates/gitlawb-node/src/api/ipfs.rs:117
  The new pinned_cids lookup makes the CIDs published by the unsigned pin index reach this loop, but the handler still acquires every readable repository and runs the synchronous git cat-file -t probe before it can determine which repository contains the OID. The IP limiter is only consulted later, after a path-scoped blob/tree is found, so a CID for an old, stale, or unscoped pin can repeatedly force O(repositories) local probes (and cold Tigris existence/download work) without consuming a quota token. This reactivates the availability problem tracked in #164 for the now-functional CID contract; associate pins with their owning repository/rows or apply a route-level bound before scanning, and move the blocking probe off the async runtime.

• [P2] Include the IPFS limiter in the periodic expiry sweep
  crates/gitlawb-node/src/main.rs:428
  The new limiter has the same client-controlled key space and one-hour window as the other bounded IP limiters, but the cleanup task clones and cleans only the older limiters. RateLimiter::check performs a global expiry sweep only when its map is already full, so a distributed request burst can retain up to 200,000 expired IP windows indefinitely during normal traffic. Clone state.ipfs_rate_limiter here and call cleanup() with the other limiter cleanup calls.

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P1] Keep quarantined repositories out of the CID search
  crates/gitlawb-node/src/api/ipfs.rs:185
  The new CID-to-OID lookup makes this route reach real pinned objects, but its list_all_repos() scan includes quarantined mirror rows and the loop never applies the quarantine gate used by the normal serve/clone handlers. A root-readable quarantined mirror can therefore return an on-disk pinned object through /ipfs/{cid}, despite quarantine being explicitly defined as hidden from serve, clone, and listings. Filter the scan to non-quarantined rows (or select and skip the flag) and cover a quarantined CID with a 404 regression test.

• [P1] Do not serve unreachable commit and tag objects under path rules
  crates/gitlawb-node/src/api/ipfs.rs:215
  The new resolver also activates CIDs recorded by the full-scan pin path. That path deliberately includes dangling non-blob objects, while this handler only proves reachability for blobs and trees; commits and tags fall through after the root gate. Thus a dangling commit/tag in a path-scoped repository can be pinned and then served anonymously even though it has no authorized reachable path, exposing commit/tag messages and structural metadata. Apply a reachable authorization check to these types too (or exclude unreachable non-blobs from pinning) and add a dangling commit/tag denial test.

• [P1] Debit every full-history walk from the IPFS quota
  crates/gitlawb-node/src/api/ipfs.rs:245
walk_rate_checked charges the source IP only before the first walk, yet the same request can launch sixteen full-history spawn_blocking walks. At the configured default of 600, one source can therefore induce up to 9,600 walks per hour, defeating the stated per-IP walk brake and allowing a known CID across path-scoped repositories to consume the blocking pool and CPU. Consume quota for each spawned walk (or explicitly account for the whole request's walk budget) instead of suppressing the later checks.

• [P2] Continue searching for a walk-free readable copy after the fan-out cap
  crates/gitlawb-node/src/api/ipfs.rs:242
  Reaching the 16-walk ceiling uses break, which exits the entire repository loop for the current OID. Because list_all_repos() is ordered by mutable updated_at, newer path-scoped rows that deny a shared object can consume the budget before an older no-rule public copy is considered; the request then returns an opaque 404 for content that remains publicly readable. This is also the behavior asserted by ipfs_walk_fanout_capped_per_request, despite the surrounding comment claiming a later walk-free public copy is still served. Skip only candidates that require another walk and continue scanning for cheap readable candidates.

• [P2] Sign gl ipfs get requests when an identity is available
  crates/gitlawb-node/src/api/ipfs.rs:119
  Resolving actual pin CIDs makes the endpoint usable for path-authorized objects, but gl ipfs get still constructs NodeClient with None, exposes no identity directory, and always calls the unsigned get. Owners and listed readers therefore receive the opaque anonymous 404 for every private/path-scoped object that the CLI can now resolve publicly. Give Get the same identity loading and signed-read path as List (or explicitly restrict and document it as public-only).

[beardthelion]

All five addressed on a919c13.

Quarantine (F1). The resolver loop gated only on visibility, and list_all_repos() does not filter quarantined rows, so a public quarantined mirror served its pinned objects. Prefetch the quarantined ids and skip them before the visibility check, so the mirror's own owner also 404s. (ipfs_cid_quarantined_repo_withheld_from_anon_and_owner, anon + owner.)

Dangling commit/tag (F2). The per-object gate only covered blob/tree; a dangling commit/tag under a path rule fell through to serve, leaking its message. Added reachable_commit_tag_oids (reachable commits + reachable tag objects, peeling tag chains) and gated commit/tag against it, sharing the same fan-out cap and per-walk quota as the blob/tree walks. Dangling commit and annotated tag now 404 with no leak; reachable ones still serve.

Two design notes on this one. First, commit/tag are now walk-gated, so a shared reachable commit behind more than the cap of path-scoped deniers with the budget already spent will 404 rather than spawn an unbounded number of walks; that is the fan-out ceiling applying to commit/tag too. Second, the commit/tag reachability deliberately does not run the assert_all_refs_are_commits guard the blob/tree walks use: a pushable annotated-tag-of-a-tree would otherwise fail-closed the whole repo and 404 every commit/tag CID. Dropping it recovers availability without admitting any dangling object (a dangling object is absent from rev-list --all and the ref walk regardless), and the walk still fails closed on a genuine git error.

Per-walk quota (F3). The IP brake charged one token per request while a request spawns up to 16 walks. Now debits per spawned walk; a memo hit or walk-free candidate is still never charged.

Fan-out (F4). The cap used a plain break that abandoned a walk-free readable copy behind newer path-scoped deniers, 404ing publicly-readable content. Changed to continue; walks stay bounded to 16.

gl ipfs get (F5). It built NodeClient with None and always sent unsigned, so an owner/reader got the anonymous 404 for path-scoped objects the resolver now serves. Added a --dir identity arg and get_authed (signs when a key is present, unsigned fallback).

Each fix is RED->GREEN with the guard reverted to confirm it is load-bearing.

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P1] Make the new reachability tests independent of the runner's Git identity
  crates/gitlawb-node/src/test_support.rs:2814
  The commit-tree fixture and the annotated git tag -a fixture both rely on Git's ambient author/committer configuration. The beta and stable CI jobs therefore fail at these two new tests with Author identity unknown / Committer identity unknown before exercising the handler (509 passed, 2 failed). Set the identity explicitly for these fixture commands (or in the isolated test repository) so the required suite is portable and green.

• [P1] Bound the CID resolver before it probes every repository
  crates/gitlawb-node/src/api/ipfs.rs:204
  Resolving a real pinned CID now scans every root-visible repository and calls repo_store.acquire() plus the synchronous git cat-file -t probe before reaching the new limiter. A stale/absent CID from the public pins index, or a CID whose copies are in unscoped repositories, therefore bypasses the limiter and can repeatedly trigger O(all repos) blocking subprocesses; cold repositories additionally cause Tigris downloads and disk writes. Apply a route-level budget/rate limit before the scan (and move the blocking probe off the async worker), or retain repository provenance with the pin so this public endpoint does not fan out across the node.

• [P2] Preserve reachable annotated tags when HEAD is detached at one
  crates/gitlawb-node/src/git/visibility_pack.rs:213
  The new commit/tag set includes HEAD in rev-list, which only yields the peeled commit, but seeds tag collection exclusively from for-each-ref. A bare repository can have detached HEAD pointing at an annotated tag with no ref at that tag; that tag is reachable and pinnable, yet never enters worklist, so its CID incorrectly 404s for an authorized reader under a path-scoped rule. Treat a tag-valued HEAD as an additional tag-chain seed and cover the detached-head case.

• [P2] Do not make a valid tag-of-tree disable all allowed tree reads
  crates/gitlawb-node/src/git/visibility_pack.rs:142
allowed_tree_set_for_caller still goes through reachable_commits, whose assert_all_refs_are_commits rejects an annotated tag pointing at a tree. Such tags are valid (the new commit/tag path explicitly handles them), but the resulting walk error makes every tree CID in that repository—including the root and public subtrees—fall through to 404 for its owner and readers. Compute the tree reachability set without rejecting unrelated non-commit tag refs, while retaining a fail-closed policy for objects whose visibility cannot be established.

• [P2] Do not spend the walk ceiling before checking a later allowed scoped copy
  crates/gitlawb-node/src/api/ipfs.rs:279
  The global 16-walk budget is consumed by each newer path-scoped repository containing the same CID. After 16 deny paths, a later repository where the caller is actually allowed also needs an allowed-set walk, but this branch skips it and returns an opaque 404. Since repositories are ordered by updated_at, a user can arrange the deny copies ahead of the authorized one; the current test only covers a later no-rule copy. Preserve the resource bound without turning an allowed path-scoped CID into a false not-found response.

• [P2] Continue scanning when an earlier scoped duplicate has exhausted the IP bucket
  crates/gitlawb-node/src/api/ipfs.rs:294
  Returning 429 immediately for the first walk that exceeds the per-IP quota prevents the resolver from reaching a later unscoped public copy of the same content, even though that copy needs no walk. A newer scoped duplicate can thus make an otherwise ordinary public CID retrieval fail solely due to repository order. Skip the walk-requiring candidate (or otherwise separate the quota decision from walk-free candidates) and retain the existing protection for expensive work.

• [P2] Do not silently discard an explicitly selected identity
  crates/gl/src/ipfs_cmd.rs:98
gl ipfs get --dir <path> converts a missing, unreadable, or corrupt identity.pem into None and sends an anonymous request. For a path-scoped object the authorized user then receives the endpoint's opaque 404 instead of the actionable key-load error, defeating the new signed-read behavior; gl ipfs list correctly propagates this same error. Keep unsigned fallback for intentionally anonymous use, but propagate failures for an explicit --dir and add coverage for it.

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P1] Do not automatically follow a signed redirect without rebuilding its signature
  crates/gl/src/http.rs:42
crates/git-remote-gitlawb/src/main.rs:355
  The new policy deliberately follows same-origin normalization redirects, but reqwest resends the existing RFC 9421 headers. Those headers were made for the original request path (NodeClient::get_signed signs its path at http.rs:118, and the remote helper signs the original URL); the server verifies @path from the redirected request URI in auth/mod.rs:156. Thus a normal /api/v1/... to /api/v1/.../ redirect, or a query normalization, makes authenticated IPFS reads, private fetches, and pushes fail signature verification at the target.

  The root cause is treating a redirect policy as a transport-only decision even though this signature scheme binds request-target semantics. A redirect callback can approve the destination, but it cannot replace the stale signature headers with headers over that destination. Do not fix this by weakening server verification or removing @path from the signature: that would re-open a request-target authorization boundary. Instead, make signed requests stop at redirects, or implement an explicit redirect loop that validates the same-origin target and rebuilds the complete request (including a new signature and, for 307/308, the original body) for each hop. Keep the automatic policy only for unsigned requests if desired. Add end-to-end coverage with the real verifier for a signed, path-changing same-origin redirect on both gl and the remote helper; the existing mock tests only prove that a target was reached.

• [P1] Size the IPFS work bucket for the combined provenance and legacy fallback path
  crates/gitlawb-node/src/state.rs:451
ipfs_work_budget reserves legacy_probes + legacy_scan_pages, but the same limiter is debited for every provenance visibility walk in api/ipfs.rs:1548. A source set at the cap can first spend all 17 path-scoped denying walks, then enter the legacy fallback because that capped set may have omitted a source. Under a supported low GITLAWB_IPFS_RATE_LIMIT, the ensuing legacy probes/pages exhaust the bucket before their configured ceilings; the handler returns 429 without a continuation. After the bucket refills, a retry starts at the first row and repeats the same provenance charges, so a readable legacy holder beyond that point is permanently unreachable.

  The root cause is that one cross-request work bucket accounts for both phases, while the derived "one complete legacy search" floor accounts for only the second phase. The fix needs to establish a budget invariant for the entire request shape, not merely add a token to a single branch: reserve the worst permitted provenance-walk cost in the floor in addition to probe and page costs, accounting for the actual tighter walk cap, or separate the phase budgets so provenance work cannot consume the fallback's guaranteed reach budget. If a work brake can still terminate the fallback, it must return a continuation at the last durable pager cursor; a bare 429 is only safe when the caller already has a usable progress token. Add a test with a capped/incomplete source set of path-scoped deniers, a low rate limit, and a public legacy holder beyond the scan work that remains after those provenance debits.

• [P2] Treat explicit default ports as equivalent to omitted ports in the redirect predicate
  crates/gitlawb-core/src/redirect.rs:47
  The predicate compares Url::port() directly, so an explicitly configured http://node:80 does not follow the documented HTTP-to-HTTPS upgrade to https://node/ (and the corresponding explicit :443 spelling fails in the other direction). These URLs are common proxy configurations and are precisely the same-host upgrade path the new policy says it permits, leaving signed gl and remote-helper traffic at the 3xx response.

  The root cause is comparing URL spelling rather than the policy's intended endpoint identity: Url::port() preserves whether a default port was explicitly written. Normalize a port equal to that URL's scheme default to the same representation as an omitted port before comparing host/port, while retaining the explicit HTTPS-to-HTTP downgrade rejection. Do not simply compare port_or_known_default() values, since an HTTP-to-HTTPS upgrade has different scheme defaults and this policy intentionally permits that upgrade. Add cases for explicit :80 and :443 on both sides of an allowed upgrade, as well as a non-default-port negative case.

[beardthelion]

Head is now a7e8e350. F1 and F2 are fixed, F3 is declined with the evidence below.

F1, the signed request-target. I confirmed the mechanism before fixing it: a mockito target matching only on the signature and signature-input headers is hit after a /api/v1/thing to /api/v1/thing/ hop, so reqwest does resend them. sign_request covers @path as the client sent it and require_signature rebuilds it from the URI it received, which is why the mismatch 401s.

may_follow now requires path and query equality alongside the origin check. Seven matrix rows that rode on a path change were repointed to identical paths, so each still pins the host or port property it exists for rather than going false on the path alone. Deleting the host-equality term reddens the matrix.

On your point that the existing mocks only prove a target was reached: both clients now run the real verification over the request the target actually received and record a verdict, each paired with a control that verifies against a pinned DID. The node side is pinned separately, three cases through the production router (path mismatch, query mismatch, identical control), proven by mutating the @path reconstruction.

One thing the fix does not cover, and I would rather name it than let the clause read as broader than it is. It pins @path, not @method or content-digest. A 301, 302 or 303 still makes reqwest rewrite a signed POST to a bodyless GET while the signature headers ride along, since tower-http's FollowRedirect drops only the Content-* and Transfer-Encoding headers. The ordinary http to https 301 upgrade reaches it. That is pre-existing rather than introduced here, so I would rather close it in its own change than widen this one. Say the word if you want it in scope instead.

F2, the work floor. The floor now carries min(MAX_HISTORY_WALKS_PER_REQUEST, ipfs_max_repos_walked) on top of probes and pages.

I measured the ledger rather than deriving it. Instrumenting the fixture and draining the bucket at four floors: floor 5 gives 429 with 2 walks, floor 6 gives 200 with 3, floor 7 gives 200 with one token spare, and floor 100 still spends 6. So six debits, the 5 to 6 boundary pins the sixth as the holder's own probe, and floor 100 proves nothing else debits. The red is the work-path 429 specifically, asserted on the ipfs retrieval body prefix, because the route brake's shorter string is a substring of it.

Only the provenance phase charges walks to this bucket; the legacy phase pays a probe instead. That is what makes one walk term correct rather than two, and it is now pinned: removing the guard so the scan phase also charges reddens the test.

On the continuation point, correcting the floor restores the one-complete-search-per-window guarantee, which is the contract the floor exists to hold. It does not close the tokenless case for a caller whose first request in a window is truncated, since they have no prior token to resume from. I left that as its own change rather than folding it in here.

F3, explicit default ports. Declined. url::Url::parse strips a scheme-default port, so port() is None for both http://node.example:80/a and https://node.example:443/a, and may_follow already returns true for http://node:80 to https://node and for http://node to https://node:443. No change, and it appears nowhere in the diff.

Suites: gitlawb-core 92, gl 355, git-remote-gitlawb 52 plus 8, gitlawb-node 1064, all passing. fmt, clippy and cargo metadata --locked clean.

[jatmn]

I found issues that need to be addressed before this is ready.

Merge blockers

• [Review gate] GitHub reports mergeStateStatus: BLOCKED with reviewDecision: CHANGES_REQUESTED. Required CI is green on head a7e8e35030cae4e1126cda6c161bc516e3b8ffff, and the branch is mergeable with current main (50d3cbbe97f77b3cab9fab220add9f3a0d0dbc2f), but the PR cannot land until the open review items below are resolved.

• [Merge drift] main and this head diverged at 96d8123 on a parallel merge with #326. Current main (50d3cbb) carries 3993fd1 (distinct-signer threshold counting). This head still has the pre-#326 implementation. Please rebase onto 50d3cbb and take the #326 side of any cert.rs conflict.

Overall guidance — why review keeps dripping, and how to close it

This PR has been open since July, carries 77 commits and ~31k added lines, and has seen many review rounds. The drip persists because of three structural forces, not because individual fixes are missing:

Integration surface. The title is #135 (tree gate), but the branch also integrated #174, pin provenance/repair, scan tokens, redirect policy, gl resume, and push coalescing. Each addition carries its own caller-visible contract and reopens the same 39-file diff. Freeze scope on this PR; defer follow-up work to linked issues so the next review is closure, not another chapter.

Continuation logic has multiple exit paths. The AEAD token system, page-fetch ceilings, and gl ipfs get ladder are solid for the paths they cover. Remaining holes come from taints recorded inside gate_and_serve or from pager.exhausted breaking before the mint arms — three sites kept in sync by comments, not one helper. A shared record_scan_truncation(row, reason, …) at every truncation site, plus tests that misalign page_rows and each ceiling (not page_rows == ceiling), is what stops the next ceiling from dripping through.

Merge hygiene after parallel merges. Semantic conflicts (e.g. #210 advisory key, #326 cert threshold) merge cleanly in git but regress in behavior. Rebase onto current main before each review request and diff unrelated files against main after rebase.

What "done" looks like: rebase onto 50d3cbb with cert.rs intact; fix or file a tracked issue for any remaining contract gap; add the misaligned ceiling test matrix; post a frozen-scope / deferred-work list in the PR body; no new features until that lands.

Findings

• [P1] Restore distinct-signer counting in RefUpdateCert::satisfies_threshold
crates/gitlawb-core/src/cert.rs:138

  What breaks: satisfies_threshold counts signature entries, not distinct signers. One maintainer's valid signature duplicated twice satisfies a 2-of-3 threshold with a single real signer.

  Root cause: Rebase drift onto the 96d8123 side of the #326 fork. main at 50d3cbb has the HashSet implementation and satisfies_threshold_rejects_duplicated_signature; this head does not (git diff 50d3cbb..HEAD is a 64-line regression in this file).

  How to fix at the root: Rebase onto 50d3cbb and keep main's threshold implementation — do not hand-resolve the conflict:

  let distinct_signers: HashSet<&Did> =
      valid.iter().filter(|d| maintainers.contains(d)).collect();
  Ok(distinct_signers.len() >= threshold)

  Restore satisfies_threshold_rejects_duplicated_signature. Copy-pasting one valid signature onto the cert must not satisfy threshold 2.

---

• [P1] Mint a continuation when probe or visit budget exhausts on the final fetched page
  crates/gitlawb-node/src/api/ipfs.rs:929
crates/gitlawb-node/src/api/ipfs.rs:1324
crates/gitlawb-node/src/api/ipfs.rs:1103

  What breaks: On the last DB page (pager.exhausted == true), probe or visit ceiling taints inside gate_and_serve, but the loop breaks at ~930 before the continuation-mint arms (~960–988). The tail returns search_incomplete with continuation: null. gl ipfs get treats no token as ladder-over and stops. A holder on that final page that was never probed is unreachable on the ladder.

  Concrete path: Final page has 3 repos. ipfs_max_legacy_probes = 1. Repo 1 probed; repo 2 hits probe ceiling (~1324), taints, returns Skip; repo 3 never probed. Loop hits pager.exhausted, breaks at 930 without minting. Retry reproduces the same dead end.

  Root cause: pager.exhausted fast-path skips the mint arms. README line 403 promises probe and visit ceilings mint a continuation ("Every per-request ceiling on this path (rows, probes, visits, retained rule bytes) mints one"). Existing ladder tests set page_rows == ceiling, so this final-page branch is never exercised.

  How to fix at the root: Introduce one record_scan_truncation helper used at every taint site. For this case: before if pager.exhausted { break }, if truncated_by is non-empty and scan_continuation is unset, mint from the first skipped row's (created_at_key, repo.id). Alternatively restructure so the mint arms at ~960 run even when exhausted is true (skip only fetch_next_page).

  Test to add: ipfs_legacy_scan_page_rows = 4, ipfs_max_legacy_probes = 1, exactly 3 repos on the final page, holder in repo 3. Assert non-null continuation on first 503 and 200 on ?scan= resume.

[beardthelion]

Head is now 1bdefb18. F2 is fixed and turned out wider than the report, F1 I'm declining with the evidence below, and both merge blockers are cleared.

F2, the ceiling that sheds without a continuation. Confirmed, and the final page is only half of it. The probe and visit ceilings taint inside gate_and_serve and returned Skip, so the loop walked past the row they refused and every row behind it, then sealed the resume position from pager.cursor, which by that point is the end of the fetched page. On the final page pager.exhausted breaks ahead of every mint arm and the shed carries no token at all, which is your case. Mid-page, the seal sits past the refused rows, so the resume skips them too: a two-row page with ipfs_max_legacy_probes = 1 strands the second row on every rung. Every shipped ladder test sets page_rows == ceiling, which puts the break exactly on a page boundary, and that is why the suite could not see either.

Your suggested position, the first skipped row's own key, would not have closed it. fetch_next_page passes the cursor to list_repos_page_for_scan as after, an exclusive bound, so resuming at the refused row's key skips that row. The fix seals the row in front of it instead.

gate_and_serve now returns CeilingStop(reason) rather than tainting on its way out, and the scan loop stops there and records both halves at one site, record_scan_truncation: it taints and seals together, and only ever moves the position forward. Fixing that surfaced a third branch. The wrapped-scan tail cleared the seal on pager.resumed && pager.exhausted, which a ceiling can now reach part way through the last page, so it is gated on nothing having been sealed.

Three tests, each named for the branch it binds: the misaligned mid-page ladder, the ceiling on an unresumed final page, and the ceiling part way through a resumed final page. Each was red on a7e8e350 before the fix, and reverting the guard it names turns it red again. The seventeen existing ceiling and ladder tests pass unchanged, including the aligned-page ones. cargo test --locked is green across the workspace, clippy and fmt are clean.

1bdefb18 on top is review follow-up, no behavior change beyond one log line. Centralizing the taint and the seal had left the truncation itself unlogged, which is worse than the scattered taints it replaced: only the visit ceiling logged anything, and it logs from inside the gate, before the caller decides whether a position gets sealed, so two identical lines could mean the ladder continues or the caller is stranded. There is now one debug line carrying the reason and whether a position was sealed, and never the position itself.

The forward-only rule in record_scan_truncation exists because a later oid candidate re-walks the already-fetched rows from the front with the request's budget spent, so it stops earlier than the candidate before it. I had this reasoned but not run when I first wrote the fix; it has since been driven with a two-oid fixture, and the run also corrected my stated justification. The earlier position is still ahead of the token the caller echoed, so letting it win would not move the ladder backwards, it would shrink each rung toward a single row. The comment now says that instead.

That same run surfaced two residual gaps that are the same class as F2 but are not introduced by this round, so per your scope-freeze I'd rather file them than keep committing here. First, a CID that maps to more than one oid shares a single probe budget, pager, and seal across candidates, so a starved later candidate can be sealed past; I confirmed it strands a holder, and I confirmed the pre-fix code stranded the same row, with controls at a raised probe budget and at a single oid both serving 200. Second, oids_for_cid has no ORDER BY, so which candidate spends that shared budget is nondeterministic across nodes.

Worth stating precisely because it looks like the same bug: a tokenless shed is still reachable when GITLAWB_IPFS_MAX_REPO_VISITS is tuned below the provenance source count, because the provenance phase and the scan share one visit counter and the ceiling then fires before the scan fetches a page. I ran that fixture against this head and against a7e8e350 and got byte-identical answers, 503 visit-ceiling with no continuation on both, so it predates this round. Not reachable at the shipped default of 1024.

F1, the certificate threshold. Declining this one. The branch never touches crates/gitlawb-core/src/cert.rs: git diff 96d8123e..a7e8e350 -- crates/gitlawb-core/src/cert.rs is empty, and git merge-tree --write-tree origin/main a7e8e350 produced a tree carrying main's HashSet implementation and satisfies_threshold_rejects_duplicated_signature, with no conflict. The 64-line delta you saw is the two-dot 50d3cbb..HEAD direction showing main's newer commit as a deletion, not something the merge would land.

Merge state. I merged origin/main (50d3cbb) into the branch rather than rebasing. Seventy-seven commits of already-reviewed history is not worth rewriting for a merge that had no conflict, and the head now carries #326 either way. cargo metadata --locked is clean on the merged tree.

On freezing scope: agreed, and nothing new goes in after this. The remaining follow-up work gets linked issues rather than commits here.

[jatmn]

I found issues that need to be addressed before this is ready.

Merge blockers

• [Review gate] GitHub still reports mergeStateStatus: BLOCKED with reviewDecision: CHANGES_REQUESTED on head 1bdefb18c2bc1e6e3cd370ea61bc7586d09dc76c. Required CI is green and the branch is mergeable with current main (50d3cbbe97f77b3cab9fab220add9f3a0d0dbc2f). The PR cannot land until this review gate clears.

Findings

• [P2] Arm the legacy scan when the visit ceiling truncates the provenance path before every source is tried
  crates/gitlawb-node/src/api/ipfs.rs:831-833
crates/gitlawb-node/src/api/ipfs.rs:889-931
crates/gitlawb-node/src/api/ipfs.rs:1452-1459

  What breaks: walk.visits is shared across the provenance loop and the legacy scan. When GITLAWB_IPFS_MAX_REPO_VISITS is exhausted on the provenance path, gate_and_serve returns CeilingStop("visit-ceiling"), and the provenance handler calls record_scan_truncation(..., None) and continues. Every subsequent provenance source immediately hits the same ceiling. After the loop, needs_scan is computed as:

  sources.is_empty() || at_cap || pin_sources_incomplete
  

  If the object has a non-empty, below-cap, complete-looking provenance set, needs_scan is false and the legacy fallback never runs — even though one or more provenance sources were never visited. The tail still sees truncated_by containing visit-ceiling and returns search_incomplete with continuation: null. gl ipfs get treats a tokenless search_incomplete as ladder-over (ipfs_cmd.rs:354-357), so a holder in an unvisited provenance repo is permanently stranded on retry.

  Concrete path: Object pinned with provenance from repos A and B (pin_sources_for_oid returns both). ipfs_max_repo_visits = 1. Repo A is visited (visit counter → 1). Repo B hits CeilingStop("visit-ceiling") immediately. needs_scan is false. Legacy scan skipped. Response: 503 search_incomplete with no continuation.

  Default-config note: dormant at stock settings. GITLAWB_IPFS_MAX_REPO_VISITS defaults to 1024 while provenance is capped at MAX_PIN_SOURCES + 1 (17), so you cannot exhaust the visit budget on provenance alone without lowering the knob. The existing visit-ceiling ladder tests (get_by_cid_visit_ceiling_ladders_to_a_holder_past_it, get_by_cid_visit_ceiling_stops_scan_with_503) exercise the legacy pager with NULL-provenance pins; none cover provenance + needs_scan.

  Root cause: needs_scan encodes three signals for "the provenance set may be incomplete" (empty, at cap, incomplete marker) but not a fourth: the provenance loop was truncated by a per-request ceiling before every recorded source was visited. The provenance handler also passes None to record_scan_truncation because there is no pager cursor on that path — but that is separate from whether the legacy fallback should still run.

  How to fix at the root: Do not treat a truncated provenance pass as a complete source set. One minimal seam:

  let mut provenance_truncated = false;
  // provenance GateOutcome::CeilingStop => { provenance_truncated = true; ... }
  
  let needs_scan = sources.is_empty()
      || at_cap
      || incomplete
      || provenance_truncated;

  Alternatively, fold provenance_truncated into the same predicate that arms pin_sources_incomplete, so any ceiling that stops the provenance loop before Served forces the bounded legacy scan rather than a tokenless 503.

  Test to add: Mirror ipfs_cid_provenance_path_scoped_walk_gates_withheld_blob setup but with two provenance sources via record_pin_source (or two repos pinning the same oid). Set ipfs_max_repo_visits = 1. Place the readable copy only in the second source. Assert first response is 503 search_incomplete with a non-null continuation or that the legacy fallback runs and serves 200 without requiring a client ladder — either outcome is acceptable as long as the holder is not permanently stranded. Reverting the needs_scan guard should redden the test.

---

• [P3] Update the legacy-pin README to match list_pinned_cids filtering
  README.md:415
crates/gitlawb-node/src/db/mod.rs:3370-3391

  What breaks: The legacy-pin paragraph still says GET /api/v1/ipfs/pins can advertise an unrepaired legacy provider CID that 404s on GET /ipfs/{cid}. list_pinned_cids filters with is_raw_cidv1 before returning rows, and list_pinned_cids_omits_unrepaired_legacy_row asserts unrepaired provider-CID rows are omitted. Operators following the README will look for CIDs in the pins API that the implementation deliberately withholds.

  Root cause: The repair sweep and listing filter landed in code, but the operator-facing paragraph was not updated when the advertise-then-404 window moved from the listing endpoint to the resolver-only path.

  How to fix at the root: Rewrite README.md:415 to describe the current contract:

  • Unrepaired provider-CID rows are not listed by GET /api/v1/ipfs/pins.
  • GET /ipfs/{cid} refuses them via the F2 integrity check.
  • Repair happens via the periodic sweep (run_sweep_rearmed in main.rs), opportunistic re-pin, or push delta — not via the pins listing.
  • Rows whose object bytes are gone stay withheld.

  Align the stale comment in db/mod.rs on repair_legacy_provider_cid ("even though list_pinned_cids still advertises it") with the same wording.

---

• [P3] Add a handler test that #135 tree denial runs on the provenance pin path
  crates/gitlawb-node/src/test_support.rs:11777-11836
crates/gitlawb-node/src/test_support.rs:5591-5657

  What breaks: The primary #135 tree-deny assertions (ipfs_cid_gate_withholds_blob_from_unauthorized and friends) pin via pin_cid_for, which records NULL provenance and exercises the legacy scan. ipfs_cid_provenance_path_scoped_walk_gates_withheld_blob proves the blob walk gate on the provenance path (pin_cid_for_repo + /secret/** rule). There is no parallel test that pins a withheld subtree tree via pin_cid_for_repo and asserts anon 404 with the raw-byte leak witness used in the legacy suite.

  Root cause: Test coverage followed the legacy pin helper first; the provenance-path walk gate was proven for blobs but not extended to trees even though gate_and_serve gates both through allowed_tree_set_for_caller_bounded at ipfs.rs:1726-1734.

  How to fix at the root: Copy ipfs_cid_provenance_path_scoped_walk_gates_withheld_blob and swap the oid:

  1. Use fx.secret_tree_oid (or the tree CID from seed_cid_repos) instead of fx.secret_oid.
  2. Pin with pin_cid_for_repo(&bare, &fx.secret_tree_oid, &state.db, &repo.id).
  3. Assert anon → 404 (status-only deny check is fine; optional: add the listed-reader 200 + raw-byte witness from the legacy suite at 11795-11816 to prove the gate is withholding structure, not just returning any 404).

  Production code already shares one gate_and_serve; this is a coverage gap, not a confirmed leak. The test exists to prevent a provenance-only regression in the tree arm.

[beardthelion]

Head is now 76f63000. Correcting myself first: my last comment said the two residual gaps would go to tracked issues. That was wrong. Both live in code this PR introduces and neither exists on origin/main, so they are not follow-up work, they are this PR shipping broken. They are fixed here, along with a third defect found on the way. Four commits, and I have kept strictly to defects this PR itself introduces so the freeze holds.

The one that matters, reachable at stock config. A CID can map to several oids, and every candidate shared one resume slot that kept the maximum position. An earlier candidate spends the probe budget walking past a repo that holds the object for a later one, seals a position beyond it, and the next rung resumes past a row that candidate never examined, wraps, and sheds tokenless. An absent token is the "ladder is over" signal, so the object is unretrievable, permanently, on every retry. Observed at untouched defaults (probes 256, page rows 128, row ceiling 2048, visits 1024) with 259 repos and a two-oid CID: rung 1 a 503 with a token, rung 2 a 503 with none, holder never served. Controls: raise the probe budget, or use a single-oid CID, and both serve 200 at rung 1.

The token now names which candidate it is resuming. Three rules make that sound, and each one is narrower than it first looks because the obvious version of it breaks something:

Only one candidate per request may seal, and which one depends on where the request started. Resumed: the resumed candidate alone, because the shared pager holds only the suffix from the caller's cursor, so a later candidate walked a suffix and never saw the front. Front-started: the first unfinished candidate, because there every candidate walks from the front and a later candidate's stop is honest coverage. Silencing later candidates unconditionally removes the only thing that mints rung 1 when the first candidate wraps untruncated.

A candidate is finished when its row loop walked every fetched row, or when it owed no scan at all. The second arm is load-bearing: a properly provenanced candidate never wraps, so a rule keyed only on wrapping kills the ladder every rung. And the wrap is witnessed per candidate at the row loop's own two exits, never by reading the shared pager flag at the tail, which any short page sets, and which would let a candidate that truncated mid-page look finished and strand the rows it refused. Both exits matter: instrumenting only the first ends the ladder three rungs early.

Finishing a non-final candidate advances the seal and taints. The tail only emits a continuation when something tainted, so sealing without tainting would suppress the taint and return a definitive 404 while discarding the token it had just minted.

The no-progress echo. On a resumed request whose visit budget was already spent by the provenance phase, the scan sealed pager.cursor, which at that moment is the position the caller just sent, so the node returned the caller's own token rung after rung. Three rungs returned an identical position. A token reads as progress, so gl retries to its resume cap and each of those requests re-runs the full provenance phase, up to seventeen acquires and cat-file subprocesses, advancing nothing before it errors. A seal now has to be strictly ahead of where the request itself started, and a rung that settled nothing sheds without a token, which is the honest answer because the spender runs identically every rung.

Third defect, found while fixing the first. oids_for_cid had no ORDER BY, so Postgres returned candidates in physical heap order and which candidate spent the shared budget was unspecified. Not theoretical: three rows sharing one CID came back [aa, bb, cc], and after an unpin and re-pin of one object, [bb, cc, aa]. The sibling pin_sources_for_oid already orders its union for exactly this reason.

Two decisions I would rather you object to now than discover in review. The advance uses a new taint reason, candidate-advance, rather than reusing scan-wrapped, which would be false on the owed-no-scan path since that candidate never scanned. The cost is that anyone grepping the existing reasons will not match it, and it is one string literal if you disagree. Separately, a throttled candidate counts as unfinished, so the proposer role stays put and the caller's own token resumes it once the bucket refills.

Verification. Every guard across the four commits was proven by re-injecting the defect it names and confirming the specific test reddens for the named reason: twelve of twelve. One came back red for the wrong reason and I resolved it by running the stacked case and observing the black-box assertion fire, not by widening the expectation until it passed. cargo test --bin gitlawb-node --locked is 1085 passing, gitlawb-core 99, gl 355, clippy clean under -D warnings, fmt clean, cargo metadata --locked clean.

One honest gap, flagged as direction rather than coverage: the guard that stops a non-proposer candidate from sealing has no response-level witness. Mutating it reddens exactly one test of seventy-three, through an internal single-write assertion. The guard is unconditional production code and the assertion does fire under cargo test, but the state that would separate the two implementations is unreachable, so I cannot claim response-level coverage for it.

Still deferred, and genuinely pre-existing. The visit budget is charged by both the provenance phase and the scan, so a GITLAWB_IPFS_MAX_REPO_VISITS at or below 17 lets provenance starve the scan before it reads a row. I ran that fixture against this head and against the pre-fix base and got byte-identical answers, so it predates the branch and is not reachable at the default of 1024. That one gets an issue, along with the operator-facing warning on the knob, since fixing it here would be the scope creep you asked me to stop.

[jatmn]

Thanks for the contribution. The #135 withheld-subtree tree gate looks correct; I do not see a merge-blocking code defect in this PR.

Recommended follow-up

These are doc-alignment items worth a small follow-up PR rather than holding this merge:

• README “Legacy-pin window” paragraph contradicts current behavior
  README.md (new production note); crates/gitlawb-node/src/db/mod.rs (list_pinned_cids ~3375–3399); crates/gitlawb-node/src/main.rs (spawn_legacy_cid_sweep)
  The note added here says GET /api/v1/ipfs/pins can still advertise unrepaired legacy provider CIDs that /ipfs will not serve, and describes “a deferred one-shot startup sweep” as what retires the window. list_pinned_cids now filters non-raw keys out of the listing, and spawn_legacy_cid_sweep runs a periodic re-arming task that never returns. Align the README with the implemented listing filter and periodic repair sweep so operators are not chasing advertise-then-404 behavior the pins API no longer exhibits.

• Operator docs still describe pushes as pinning the main query pool after this branch splits write locks onto a separate pool
  crates/gitlawb-node/src/config.rs (validate, db_max_connections field doc); docs/RUN-A-NODE.md (new troubleshooting entry); crates/gitlawb-node/src/main.rs (build_lock_pool / RepoStore::new)
  On main, acquire_write held an advisory lock on the main query pool, so GITLAWB_DB_MAX_CONNECTIONS >= max_concurrent_git_pushes + 8 was accurate. This PR routes write locks through lock_pool instead, but validate() and the new RUN-A-NODE troubleshooting text still say each concurrent push pins a main-pool connection for its whole receive-pack. The max_concurrent_git_pushes field doc already notes the lock pool; extend that story through validate()’s error text, the db_max_connections field doc, and RUN-A-NODE so operators size total Postgres demand (db_max_connections plus the lock pool sized from GITLAWB_MAX_CONCURRENT_GIT_PUSHES) without implying that raising the query pool is what isolates push concurrency.

[beardthelion]

Correcting two things I said in my last comment before this merges, because both are wrong and one of them would mislead whoever reads this thread next.

I said the visit-ceiling gap "predates the branch". It does not. origin/main has no provenance phase at all: no pin_sources_for_oid, no oids_for_cid, no legacy_scan, just a single-phase scan with a local repos_visited counter, so there are not two phases there to share a budget and the starvation cannot happen. The provenance phase arrived in b6bc6989, which lives on this branch and nowhere else. What I actually verified was identical behavior on a7e8e350, an earlier head of this same branch, which makes it not a regression from the continuation work. I then stated that as "predates the branch", which is a different and false claim.

I also said it would be filed as a tracked issue. It is fixed instead, for the reason above: an issue against main would describe code that is not there.

The fix splits the visit budget per phase, which is the argument the walk ceiling already settled a round earlier. provenance_walks and scan_walks are two counters against the same cap because the phases are not alternatives: the fallback exists to reach a source the provenance set dropped, and a shared counter lets the first phase spend what the second needs. Visits were the counter that got missed. New test red then green, the guard proven by re-injecting the defect, node suite 1086 green, fmt and clippy clean.

It is not going into this PR. It lands as a follow-up immediately after this merges. Two reasons. It needs GITLAWB_IPFS_MAX_REPO_VISITS at or below 17 against a default of 1024, so no default node is exposed and there is no confidentiality or authorization dimension, only reachability of public content on a node whose operator already lowered that knob. And a focused diff reviewed on its own is a better review than a fifth commit landing in a thirty-thousand-line PR at round fifteen, which is the scope discipline you asked for and I agree with.

One consequence worth recording since it changes something you reviewed. Splitting the budgets makes the strictly-ahead filter in 76f63000 unreachable: the scan's top-of-loop arms run before the first fetch and every counter they test now starts at zero for the scan, so nothing can seal a position that has not advanced. The filter stays as a guard against a future arm reopening that path, its test is replaced by one asserting the rung now advances, and I removed its entry from our mutation set rather than leave one claiming to prove something it no longer can.

[beardthelion]

@kevincodex1 ready to go. jatmn approved, 18/18 green on 76f6300, threads all resolved.