test(node): submit_bounty claimant gate is a vacuous guard; harden every_in_scope_mutation_has_its_gate #203

Activity

1. beardthelion

   added

   crate:nodegitlawb-node — the serving node and REST APIgitlawb-node — the serving node and REST API

   kind:testTest coverage or harnessTest coverage or harness

   sev:mediumDegraded but workaround existsDegraded but workaround exists

   subsystem:apiNode REST API request/response surfaceNode REST API request/response surface

   on Jul 14, 2026
2. beardthelion commented on Jul 14, 2026

   

   beardthelion

   on Jul 14, 2026

   Last edited by beardthelion

   CollaboratorAuthor

   More actions

   Root cause of why the invariant gate misses this (reproduced by execution: neutering submit_bounty's if !is_claimant gate left the full deny_harness suite at 33/33 green).

   submit_bounty falls through all three nets of the gate:

   1. Runtime deny-sweep (deny_bearing_routes, tests/support/routes.rs) drives only handlers classified as deny-bearing (403/404/401). submit_bounty is classified positive-by-design (src/api/mod.rs:216, Bucket D), so it has no row and is never probed. test(node): invariant deny-prober (driven registry + completeness cross-check) #195 added dispute_bounty as a deny row but left submit/approve/cancel as marker-only.

   2. Read-side completeness (mounted_repo_gets_are_driven_excused_or_declared_public, tests/deny_harness.rs) derives its set from the mounted route table, but gates on if method != "GET" || !is_repo_scoped_path(path). submit_bounty is a POST on a non-repo-scoped path (/api/v1/bounties/{id}/submit), excluded twice over.

   3. Mutation-side completeness (every_in_scope_mutation_has_its_gate, src/api/mod.rs:168) does list it (:218), but asserts body.contains("did_matches(") (:242) over a hand-maintained list (:185-230). submit_bounty binds let is_claimant = ...did_matches(...) on a line separate from its enforcement if (bounties.rs:302 vs 304), so deleting the if leaves the substring in place and the guard stays green. It checks the marker exists, not that it is the enforcement site or binds the right principal.

   The one-sentence cause: the completeness-guard lessons #195 applied to READS (derive the required set from the route table; the guard must go RED when the enforcement line is reverted) were never applied to the MUTATION side, which predates #195 as a hand-list + substring-presence check. The read gate is load-bearing by construction; the mutation gate is presence by construction. approve/cancel happen to place did_matches( inside the enforcement if, so deletion trips their marker, but that is a coincidence, not a designed property.

   Gate-level fix direction (not just three rows):

   • Derive the mutation required-set from the route table (extend scrape_mounts to non-GET / non-repo-scoped) so a new lifecycle mutation cannot be silently omitted from a hand-list.
   • Bind the check to enforcement, not presence: give submit/approve/cancel_bounty single-arm 403 rows in the deny-prober (exactly what dispute_bounty got in test(node): invariant deny-prober (driven registry + completeness cross-check) #195), so reverting each principal check goes RED by execution. Substring-presence cannot be made load-bearing; only a driven probe can.
   • Extend the completeness net past GET + repo-scoped so POST / non-repo-scoped handlers have a home.

   Note: comment_only_marker_does_not_satisfy_a_row (mod.rs:252) shows the substring check's weakness was already known and one evasion (comment-only markers) patched. A live-but-non-enforcing marker is the same class, unpatched.
3. beardthelion commented on Aug 15, 2026

   

   beardthelion

   on Aug 15, 2026

   CollaboratorAuthor

   More actions

   Evidence that the blind spot named here is not hypothetical: it produced four handlers in this same file with no read gate at all, now filed as #341.

   grep -n "pub async fn \|authorize_repo_read" crates/gitlawb-node/src/api/bounties.rs at origin/main 50d3cbb splits cleanly. Every handler through claim_bounty gates: create_bounty (:84), list_repo_bounties (:123), list_all_bounties (:191), get_bounty (:224), claim_bounty (:245). Then nothing between submit_bounty (:282) and bounty_stats (:460), which spans submit, approve, cancel, and dispute.

   Driven with a throwaway #[sqlx::test] against a private repo, three seeded bounties, a stranger keypair, each route probed against an existing id and an absent one:

   submit   EXISTS=400 {"message":"bounty is open, not claimed"}          ABSENT=404
   cancel   EXISTS=403 {"message":"only the bounty creator can cancel"}   ABSENT=404
   approve  EXISTS=403 ...                                                ABSENT=404
   dispute  EXISTS=403 ...                                                ABSENT=404
   claim    EXISTS=404                                                    ABSENT=404
   

   claim_bounty is the negative control, and its two responses are byte-identical because it gates. The other four are distinguishable in both directions, and submit leaks lifecycle state on top of existence.

   This bears on the fix options above in two ways.

   Option 2 (require the enforcement expression rather than any substring) is the one that would have helped, but only partly. These four do enforce their participant check correctly; a stricter marker would still pass them. What is missing is a different gate entirely, and the reason the completeness scan cannot ask for it is the structural point in the issue body: every_repo_scoped_handler_is_gated keys on Path<(String, String), so a global-id Path<String> route is invisible to it no matter how the marker is tightened. So the useful shape is probably a third membership rule, keyed on the handler resolving a repo indirectly (through a looked-up record) rather than on its path type.

   Worth noting for whoever picks this up: closed issue #160 examined these exact four and excluded them, on the reasoning that they "enforce a did_matches creator/claimant check before returning the record, so they don't disclose to an outsider". That is correct about record disclosure and does not cover the status-code channel, since the status check runs before the participant check. A considered exclusion that a marker-based guard had no way to re-examine.