Skip to content

fix(gates): no-admin-idor — recognize a guard one hop inside a resolved helper - #518

Merged
rubenvdlinde merged 3 commits into
mainfrom
fix/idor-gate-one-hop-guard-body
Aug 20, 2026
Merged

fix(gates): no-admin-idor — recognize a guard one hop inside a resolved helper#518
rubenvdlinde merged 3 commits into
mainfrom
fix/idor-gate-one-hop-guard-body

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What was actually wrong

The premise this started from — "the gate's regex doesn't recognize ->canAccess(, causing 82% false positives" — turned out to be false for the production checker, and I'd rather record that than quietly ship against it. _GUARD_HELPER_NAME_RE has recognized canAccess by name since #360. Verified empirically: the real check_no_admin_idor.py over all 86 shillinq controllers reports 0 findings. The 86/105 false-positive rate came from a naive audit script quoted inside shillinq's own change design doc, not from this gate.

The residual gap that is real

A same-class helper whose name carries no auth token — shillinq/decidesk's own idiom (resolveScope(), mayAccessReturn(), guardDraftAccess()) — but whose body reaches $this->context->canAccess(...) one hop out. Neither the name regex nor the old body regexes (throw / 401 / 403 / 404 / authorize* / require* / ensure*) matched that shape.

The fix, and the hole avoided while making it

Added _GUARD_VERB_CALL_RE (is|has|can|may + Admin|Access|Permission|Permitted|Owner|Allowed|Authori[sz]ed, reusing the vetted token set) into _HELPER_GUARD_BODY_RE and _STRICT_GUARD_BODY_RE — deliberately not into _GUARD_BODY_RE.

A first draft did add it there and broke test_shape_a_without_the_helper_is_still_reported: _GUARD_BODY_RE is applied to the routed method with no existence check, so it would have cleared a call to a nonexistent canAccess-shaped method by name alone, deleting Pattern 1's must-exist invariant for the whole verb family. The two helper-body regexes are only consulted for a method already confirmed to exist, so the same widening is safe there.

The no-op-stub case stays caught (shillinq shipped exactly one: DBAController::ensureAdministrationAccess() matched ensure* while never denying anything). Proven, not asserted: new test_POSITIVE_CONTROL_helper_with_no_auth_token_name_and_no_guard_body_is_flagged — same call site, stub body, still 1 finding.

Fleet survey

hrmq / decidesk / pipelinq / procest / openregister use hasAccess, mayAccess, isAllowed, isAuthorized*, hasPermission, hasAccessToOrganisation — all already in the vocabulary. canManageOrganisationMembers / canDelete carry no object token and are a documented deliberate non-match, flagged as follow-up rather than widened into.

Controls

  • Unit tests 177/177 (171 pre-existing + 6 new).
  • Acceptance matrix 172/172, 65/72 gates fixtured (baseline unchanged); the existing auth-guards bundle extended (not a parallel scheme) — planted fails 3/3 incl. the new archive(), clean passes 3/3.

Handback to hydra: hydra-gate-no-admin-idor/SKILL.md should note the one-hop body recognition and why it is deliberately absent from the top-level regex.

🤖 Generated with Claude Code

Conduction Release Bot added 3 commits August 20, 2026 11:07
@rubenvdlinde
rubenvdlinde merged commit 6015d84 into main Aug 20, 2026
37 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/idor-gate-one-hop-guard-body branch August 20, 2026 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant