fix(gate-7): a guard helper may spell its object noun after the auth token - #353
Merged
Merged
Conversation
…token
`_GUARD_HELPER_NAME_RE`'s first alternative required the auth token
(Admin/Access/Permission/Permitted/Owner/Allowed/Authorised) to be the
FINAL CamelCase segment, so `canAccess` matched but `canUserAccessAgent`
did not. That rejected the very common verb-object spelling, and every
routed method delegating to such a predicate was reported as an
unguarded IDOR.
MEASURED on ConductionNL/hermiq @ development (cd23f547), full-scope run
31490144919 / job 93776678440: gate-7 FAIL with 3 methods, all three
false positives —
- AgentsController::index — filters every result through
`canUserAccessAgent($agent, $userId)` in-body;
- AgentVersionController::index and ::diff — both delegate to
`loadAccessibleAgent()`, which calls `canUserAccessAgent()` and
returns null, on which the caller returns Http::STATUS_NOT_FOUND —
the 404-style tenancy refusal this gate's own FAIL message endorses.
Gate-7 was proven NOT blind on that repo before concluding this: a
textbook IDOR planted into the TRACKED file AgentVersionController.php
(gate-7 enumerates via `git ls-files -- lib/Controller`, so an untracked
plant is never scanned) took the count 3 -> 4. Plant removed.
An auth token is still REQUIRED; only its POSITION is relaxed.
`canRender` / `hasChanges` still do not match — they carry no auth token
in any position.
Tests: 86 existing pass unchanged; 7 added under
VerbObjectGuardHelperNames covering both hermiq shapes (in-body filter,
and the Pattern-4 transitive closure through a loader), plus four abuse
controls. The three PASS-shape tests were confirmed to go RED against
the pre-patch regex. The canRender/hasChanges controls were confirmed to
be real boundaries rather than structurally-always-flagged: renaming the
helper to canUserAccessWidget / hasOwnerPermissionForDraft, with a
byte-identical body, flips each to cleared.
Fleet sanity sweep, before/after over `git ls-files lib/Controller`:
hermiq 3 -> 0 (the three above, each verified to carry a genuine
per-object guard), docudesk 2 -> 2, hrmq 1 -> 1, and 16 other apps
0 -> 0. Nothing else in the fleet changed verdict; no unguarded method
was cleared anywhere.
This was referenced Aug 11, 2026
Merged
rubenvdlinde
added a commit
that referenced
this pull request
Aug 11, 2026
Closes #360. #353 relaxed WHERE the auth token may sit in a guard-helper name but left the segment before it mandatory, so hasPermission() and canAccess() were still reported as unguarded IDOR. Making that segment repeatable and optional admits the token in any position; the token SET is unchanged and a token is still required. Measured strict superset: 400,000 fuzzed identifiers produced zero names the old regex matched and the new one does not, so this can only turn findings green and cannot redden any repo. Observed red-then-green: the gate-7 acceptance arm went 2 findings -> 0 and still goes red (2) under the reconstructed pre-fix regex; test_check_no_admin_idor.py 93 -> 95 tests, the new one failing on the old regex.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
_GUARD_HELPER_NAME_RE's first alternative required the authorisation token (Admin/Access/Permission/Permitted/Owner/Allowed/Authori[sz]ed) to be the final CamelCase segment:That rejects the very common verb-object spelling where the object noun trails the auth token —
canUserAccessAgent(),hasOwnerPermissionForRun(). These are genuine authorisation predicates, so every routed method delegating to one was reported as an unguarded IDOR.The fix
The auth token may now be a complete CamelCase segment anywhere after the
is|has|can|mayprefix. A token is still required; only its position is relaxed.Evidence
ConductionNL/hermiq @
development(cd23f547), full-scope run31490144919, job93776678440— gate-7 FAIL, 3 methods, all three false positives:AgentsController.php:127 index$this->canUserAccessAgent($agent, $userId)in-bodyAgentVersionController.php:105 indexloadAccessibleAgent()→canUserAccessAgent(), returnsnull; caller returnsHttp::STATUS_NOT_FOUNDAgentVersionController.php:140 diffThe 404-on-denial shape is the deliberate tenancy refusal this gate's own FAIL message endorses.
Gate-7 was proven NOT blind on that repo first. A textbook IDOR (
#[NoAdminRequired] plantedIdor(string $id)doing a bareobjectService->find(id: $id)) was planted into the tracked filelib/Controller/AgentVersionController.php— gate-7 enumerates viagit ls-files -- lib/Controller, so an untracked plant is never scanned — and the count went 3 → 4. Plant removed.Verification
Existing suite: 86 tests, all pass unchanged.
Added: 7 tests under
VerbObjectGuardHelperNames— both hermiq shapes (in-body per-object filter; Pattern-4 transitive closure through a loader whose own name carries no token and whose body has no strict deny signal), plus abuse controls.Revert check: with the pre-patch regex restored, the three PASS-shape tests go RED:
The first draft of the Shape-A fixture passed identically under the old regex — the method took no caller input, so the session-scoped/zero-reference exemption cleared it before the guard-helper pattern was ever consulted. The fixture now reads pagination params, so the object reference is real and the helper's name is the only thing standing between it and a finding. That near-miss is recorded in a comment on the fixture.
The negative controls are real boundaries, not structurally-always-flagged. With byte-identical bodies, only the helper name changed:
canRendercanUserAccessWidgethasChangeshasOwnerPermissionForDraftA positive control of the unguarded-fetch shape already exists twice (
RealIdorViolationTest.test_no_guard_at_all_is_flagged, docblock form;ZeroInputReadOnlyEndpoints.test_tp_a_method_taking_an_id_is_still_reported, attribute form) and is deliberately not duplicated.Fleet sanity sweep
Ran before and after over
git ls-files 'lib/Controller/*.php' 'lib/Controller/**/*.php'in every local app with alib/Controller:Only the three hermiq false positives were cleared, fleet-wide. Each was opened and confirmed to carry a genuine per-object authorisation guard. The remaining findings (
docudeskAnonymizationController::updateRelation,EmlPreviewController::preview;hrmqAdministrationController::setActive) are untouched.Note:
canUserModifyAgent— cited in passing while scoping this — correctly still does not match, sinceModifyis not an auth token.