From d83c786eeca5f857e9db03cd190f084ca4834ae3 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Tue, 4 Aug 2026 12:11:49 +0200 Subject: [PATCH] fix(hydra-gates): gate-7 follows delegation; gates 6/7 stop passing on an empty scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two blockers to enabling the Hydra gates fleet-wide. 1. gate-7 (no-admin-idor) false-positived on ordinary delegation. It only recognised a guard written in the routed method's own body, so any controller that centralises authorisation in an injected responder — or routes thin public actions through private helpers first — went red on correct code. Measured on decidesk development: 11 findings, ALL 11 guarded. 8 reach $this->responder->staffAction() -> requireStaff() (currentUid() !== null AND isStaff()), 3 reach citizenAction() (401 for an anonymous caller), and validateProposal took three hops (validateProposal -> approveProposal / rejectProposal -> applyProposalDecision -> staffAction). Zero real exposure. Pattern 4 resolves this with evidence, not with a naming convention: a. Cross-class resolution. The controller's typed constructor-promoted properties give $prop -> ClassName; the class is resolved to a real file under the app's lib/ tree, confirmed by an actual `class ` declaration, and PARSED. A call $this->prop->method( clears the routed method only when `method` is demonstrably guard-bearing in the collaborator's own source. An unresolvable class clears nothing. b. Transitive closure to a fixpoint over same-class calls, so a delegation chain of any length is followed. Propagation uses a STRICTER signal than the existing one-hop Pattern 1: a bare `throw` and a 404 do not seed a chain. NotFoundException is not an authorisation guard, and chaining it would let "this can fail" stand in for "this checks who you are" arbitrarily far up the call graph. Every hop must also occur before the caller's first data mutation. Both directions are covered by tests, because a clear that cannot be shown to still fail is only evidence about itself. 9 new tests: 3 assert the decidesk shapes now pass, 6 assert the gate still catches a plainly unguarded method, a collaborator method that EXISTS but is not a guard (respond()), an unresolvable collaborator type, a chain terminating in nothing, a guard that runs after the write, and a bare-throw collaborator. 64/64 green. 2. gates 6 and 7 reported PASS when the SCOPE was empty. Zero files in the diff meant an empty findings log, a count of 0, and a PASS over nothing inspected — the same failure family as .github#147, where a missing helper made gate-7 report PASS over 11 real unguarded endpoints. They now _skip, which keeps them out of _EMITTED_GATES so the coverage summary lists them under GATES THAT DID NOT RUN. Verified against decidesk with --base HEAD: "GATES THAT DID NOT RUN: 4 6 7 24 33". Full suite: 17 helper suites pass, 2 pre-existing quarantines unchanged, 25/25 bin entry-point tests pass. --- .../scripts/lib/check_no_admin_idor.py | 285 ++++++++++++++ .../scripts/lib/test_check_no_admin_idor.py | 371 ++++++++++++++++++ hydra-gates/scripts/run-hydra-gates.sh | 17 + 3 files changed, 673 insertions(+) diff --git a/hydra-gates/scripts/lib/check_no_admin_idor.py b/hydra-gates/scripts/lib/check_no_admin_idor.py index cb20156..d8a750b 100644 --- a/hydra-gates/scripts/lib/check_no_admin_idor.py +++ b/hydra-gates/scripts/lib/check_no_admin_idor.py @@ -42,6 +42,44 @@ an explicit controller-level guard (or a Pattern-1 helper), so a real IDOR in a leaf app is never masked. See "Pattern 2 boundary" below. + Pattern 4 — delegated guard reached through a chain and/or a collaborator. + Patterns 1–3 all require the guard to be *one* call-hop away and on + ``$this``. Real controllers routinely centralise authorisation in an + injected responder/guard collaborator, and route several thin public + actions through private helpers before reaching it. Measured on decidesk + 2026-08-04, that shape produced 11 findings and **every one was guarded**: + 8 reached ``$this->responder->staffAction()`` → ``requireStaff()`` (which + checks ``currentUid() !== null`` AND ``isStaff()``), 3 reached + ``citizenAction()`` (401 for an anonymous caller), and ``validateProposal`` + took three hops (``validateProposal`` → ``approveProposal`` / + ``rejectProposal`` → ``applyProposalDecision`` → ``staffAction``). + + Pattern 4 therefore does two things, both evidence-based: + + a. *Cross-class resolution.* The controller's typed constructor-promoted + properties and property declarations give ``$prop -> ClassName``. The + class is resolved to a real file under the app's ``lib/`` tree (PSR-4 + basename, confirmed by an actual ``class `` declaration) and + **that file is parsed**. A call ``$this->prop->method(`` clears the + routed method only when ``method`` is demonstrably guard-bearing in + the collaborator's own source. Nothing is assumed from the name of + the property or of the class — an unresolvable class clears nothing. + + b. *Transitive closure.* A same-class method that reaches a guard (by + (a) or by a strict in-body signal) before its first data mutation is + itself guard-bearing, and the closure is iterated to a fixpoint, so an + arbitrarily long intra-class delegation chain is followed. + + Propagation deliberately uses a **stricter** signal than the one-hop + Pattern 1 (``_STRICT_GUARD_BODY_RE``): a bare ``throw`` or a 404 does not + seed a chain, because a method that throws ``NotFoundException`` is not an + authorisation guard and chaining would let that leak arbitrarily far. Only + an explicit deny (401/403, ``OCSForbiddenException``, ``isAdmin``, an + ``authorize*``/``require*``/``ensure*``/``assert*``/``guard*`` call, or an + anonymous-session rejection) starts or continues a chain. Every hop must + also occur before the caller's first data mutation, so a guard that runs + only after the write still fails the gate. + Exemptions (method skipped entirely): 1. ``__construct`` — not a routed action, the 20-line look-back window can accidentally catch it when a constructor follows an annotated method. @@ -76,6 +114,7 @@ """ from __future__ import annotations +import os import re import sys @@ -384,6 +423,235 @@ def _find_method_bodies(src: str): r"[A-Za-z0-9_]*\s*\(", ) +# --------------------------------------------------------------------------- +# Pattern 4 — delegation chains and collaborator-hosted guards +# --------------------------------------------------------------------------- + +# The signal that may START or CONTINUE a delegation chain. +# +# Deliberately STRICTER than _HELPER_GUARD_BODY_RE, which is kept as-is for the +# one-hop Pattern-1 clear. Two alternatives are dropped here on purpose: +# +# - a bare `throw`. A method that throws NotFoundException is not an +# authorisation guard; accepting it at one hop is already generous, and +# propagating it transitively would let "this function can fail" stand in +# for "this function checks who you are" arbitrarily far up the call chain. +# - `404` / STATUS_NOT_FOUND. Same reason: not-found is not access-denied. +# +# What remains is an explicit deny decision: a 401/403, a forbidden exception, +# an admin-membership test, a call to an authorize*/require*/ensure*/assert*/ +# guard* predicate, or the rejection of an anonymous session. +_STRICT_GUARD_BODY_RE = re.compile( + r"OCSForbiddenException" + r"|NotPermittedException" + r"|ForbiddenException" + r"|isAdmin\s*\(" + r"|isCurrentUserAdmin\s*\(" + r"|->\s*(?:authorize|authorise|require|ensure|assert|guard)[A-Z][A-Za-z0-9_]*\s*\(" + r"|Http::STATUS_(?:UNAUTHORIZED|FORBIDDEN)" + r"|(?:statusCode:\s*|,\s*)(?:401|403)\b" + r"|(?:getUser|getUID|currentUid|getCurrentUserId)\s*\(\s*\)\s*===\s*null" +) + +# Typed property declarations and constructor-promoted properties: +# private readonly ParticipationResponder $responder, +# protected ?FooGuard $guard; +# Captures (ClassName, propertyName). Scalar/builtin types are filtered out by +# _COLLABORATOR_SKIP_TYPES so `private string $key` never becomes a lookup. +_PROPERTY_DECL_RE = re.compile( + r"\b(?:private|protected|public)\s+(?:readonly\s+)?\??" + r"([A-Za-z_][A-Za-z0-9_]*(?:\\[A-Za-z_][A-Za-z0-9_]*)*)\s+" + r"\$([A-Za-z_][A-Za-z0-9_]*)" +) + +_COLLABORATOR_SKIP_TYPES = frozenset( + { + "array", "bool", "boolean", "callable", "float", "int", "integer", + "iterable", "mixed", "object", "string", "self", "static", "null", + "readonly", "false", "true", "void", + } +) + +_CLASS_DECL_TEMPLATE = r"\b(?:abstract\s+|final\s+|readonly\s+)*class\s+%s\b" + +# Per-repo index of `ClassName -> [path, ...]` under the app's lib/ tree, built +# lazily and cached. Bounded to lib/ so a scan never walks node_modules/vendor. +_CLASS_INDEX_CACHE: dict = {} +# Per-file cache of the strict guard-bearing method set of a collaborator class. +_COLLABORATOR_GUARD_CACHE: dict = {} + + +def _app_root_for(path: str): + """Return the app root for *path* — the parent of its ``lib/`` directory. + + Gate-7 is only ever handed ``lib/Controller/*.php``, so the root is the + directory containing the ``lib`` segment. Returns ``None`` when there is + no such segment (the caller then resolves no collaborators at all, which + fails closed: unresolved means unguarded means flagged). + """ + parts = os.path.abspath(path).split(os.sep) + for i in range(len(parts) - 1, -1, -1): + if parts[i] == "lib": + return os.sep.join(parts[:i]) or os.sep + return None + + +def _class_index(root: str) -> dict: + """Map ``ClassName -> [file, ...]`` for every PHP file under ``root/lib``. + + PSR-4 basename indexing: the fleet's apps all name the file after the + class. Candidates are *verified* by the caller against an actual ``class + `` declaration, so a basename collision cannot silently resolve to + the wrong file. + """ + cached = _CLASS_INDEX_CACHE.get(root) + if cached is not None: + return cached + index: dict = {} + lib_dir = os.path.join(root, "lib") + for dirpath, dirnames, filenames in os.walk(lib_dir): + dirnames[:] = [ + d for d in dirnames if d not in ("vendor", "node_modules", ".git") + ] + for fn in filenames: + if fn.endswith(".php"): + index.setdefault(fn[:-4], []).append(os.path.join(dirpath, fn)) + _CLASS_INDEX_CACHE[root] = index + return index + + +def _strict_guard_methods(cleaned: str, src: str) -> set: + """Names of methods in one class that reach a strict guard, transitively. + + Seeded with methods whose name is an authorisation predicate + (``_GUARD_HELPER_NAME_RE``) or whose body carries a strict deny signal + (``_STRICT_GUARD_BODY_RE``), then closed over same-class calls: a method + that invokes a known guard-bearing method *before its first data mutation* + becomes guard-bearing itself. Iterated to a fixpoint, so a chain of any + length is followed. + """ + spans = list(_all_method_spans(cleaned)) + known: set = set() + for name, body_start, body_end in spans: + if _GUARD_HELPER_NAME_RE.match(name): + known.add(name) + elif _STRICT_GUARD_BODY_RE.search(src[body_start:body_end]): + known.add(name) + changed = True + while changed: + changed = False + for name, body_start, body_end in spans: + if name in known: + continue + if _calls_guard_helper_before_mutation(src[body_start:body_end], known): + known.add(name) + changed = True + return known + + +def _collaborator_guard_methods(class_file: str) -> set: + """Strict guard-bearing method names declared by the class in *class_file*.""" + cached = _COLLABORATOR_GUARD_CACHE.get(class_file) + if cached is not None: + return cached + try: + with open(class_file, encoding="utf-8") as fh: + src = fh.read() + except OSError: + _COLLABORATOR_GUARD_CACHE[class_file] = set() + return set() + result = _strict_guard_methods(_strip_strings_and_comments(src), src) + _COLLABORATOR_GUARD_CACHE[class_file] = result + return result + + +def _collaborator_guard_map(cleaned: str, path: str) -> dict: + """Map ``propertyName -> {guard method names}`` for this class's collaborators. + + Reads the typed constructor-promoted properties and property declarations, + resolves each type to a real file under the app's ``lib/`` tree, confirms + the file actually declares that class, and parses it for guard-bearing + methods. A type that cannot be resolved contributes nothing — the routed + method then stays flagged, which is the fail-closed direction. + """ + root = _app_root_for(path) + if root is None: + return {} + index = _class_index(root) + out: dict = {} + for type_name, prop in _PROPERTY_DECL_RE.findall(cleaned): + short = type_name.rsplit("\\", 1)[-1] + if short.lower() in _COLLABORATOR_SKIP_TYPES: + continue + guards: set = set() + decl_re = re.compile(_CLASS_DECL_TEMPLATE % re.escape(short)) + for candidate in index.get(short, []): + if os.path.abspath(candidate) == os.path.abspath(path): + continue + try: + with open(candidate, encoding="utf-8") as fh: + csrc = fh.read() + except OSError: + continue + if not decl_re.search(_strip_strings_and_comments(csrc)): + continue + guards |= _collaborator_guard_methods(candidate) + if guards: + out.setdefault(prop, set()).update(guards) + return out + + +def _calls_collaborator_guard_before_mutation(body: str, guard_map: dict) -> bool: + """True when *body* calls ``$this->->(`` before its first write. + + *guard_map* comes from :func:`_collaborator_guard_map`, so every method + named here was read out of the collaborator's own source — this is a + resolved delegation, not a naming convention. + """ + if not guard_map: + return False + mutation = _MUTATION_RE.search(body) + mutation_pos = mutation.start() if mutation is not None else None + for prop, methods in guard_map.items(): + for method in methods: + call_re = re.compile( + r"\$this\s*->\s*" + re.escape(prop) + r"\s*->\s*" + + re.escape(method) + r"\s*\(" + ) + for m in call_re.finditer(body): + if mutation_pos is None or m.start() < mutation_pos: + return True + return False + + +def _delegated_guard_methods(cleaned: str, src: str, guard_map: dict) -> set: + """Same-class methods that reach a guard through *any* resolved route. + + Seed = strict in-body guards (``_strict_guard_methods``) plus methods that + delegate straight to a resolved collaborator guard; then closed over + same-class calls to a fixpoint. This is what clears decidesk's + ``validateProposal`` → ``approveProposal`` → ``applyProposalDecision`` → + ``$this->responder->staffAction()`` three-hop chain. + """ + spans = list(_all_method_spans(cleaned)) + known = _strict_guard_methods(cleaned, src) + for name, body_start, body_end in spans: + if name in known: + continue + if _calls_collaborator_guard_before_mutation(src[body_start:body_end], guard_map): + known.add(name) + changed = True + while changed: + changed = False + for name, body_start, body_end in spans: + if name in known: + continue + if _calls_guard_helper_before_mutation(src[body_start:body_end], known): + known.add(name) + changed = True + return known + + # --------------------------------------------------------------------------- # Pattern 2 — OpenRegister data-layer RBAC delegation (ADR-022) # --------------------------------------------------------------------------- @@ -599,6 +867,12 @@ def scan_file(path: str) -> int: cleaned = _strip_strings_and_comments(src) is_or_repo = bool(_OR_NAMESPACE_RE.search(cleaned)) guard_helpers = _collect_guard_helpers(cleaned, src, is_or_repo) + # Pattern 4 context: resolve this class's typed collaborators to real files + # and read their guard-bearing methods out of their own source, then close + # the same-class delegation graph over that. Both are lazy/cached; a file + # with no @NoAdminRequired method never pays for them. + collaborator_guards = _collaborator_guard_map(cleaned, path) + delegated_guards = _delegated_guard_methods(cleaned, src, collaborator_guards) violations = 0 for name, head_start, sig_start, body_start, body_end, line_no in _find_method_bodies(src): @@ -657,6 +931,17 @@ def scan_file(path: str) -> int: if _calls_guard_helper_before_mutation(body, guard_helpers): continue + # ---- Pattern 4: resolved delegation chain / collaborator guard --- + # Either the routed method hands straight to a collaborator method + # that was READ and found guard-bearing in its own file + # ($this->responder->staffAction()), or it reaches one through a chain + # of same-class helpers. Every hop is required to occur before the + # first data mutation, and an unresolvable collaborator clears nothing. + if _calls_collaborator_guard_before_mutation(body, collaborator_guards): + continue + if _calls_guard_helper_before_mutation(body, delegated_guards): + continue + # ---- Pattern 2: OpenRegister data-layer RBAC delegation --------- # Inside the OpenRegister app, data access through ObjectService or a # *Mapper delegates per-object authz to OR's register RBAC + diff --git a/hydra-gates/scripts/lib/test_check_no_admin_idor.py b/hydra-gates/scripts/lib/test_check_no_admin_idor.py index cd09732..896fdc3 100644 --- a/hydra-gates/scripts/lib/test_check_no_admin_idor.py +++ b/hydra-gates/scripts/lib/test_check_no_admin_idor.py @@ -1107,5 +1107,376 @@ class C { self.assertIn("method=show", out[0]) +# --------------------------------------------------------------------------- +# Pattern 4 — delegation chains and collaborator-hosted guards +# --------------------------------------------------------------------------- + +def _scan_app(controller_src: str, collaborators: dict) -> list[str]: + """Scan a controller inside a throwaway app tree with real collaborators. + + Pattern 4 resolves a typed property to a *file* under the app's ``lib/`` + tree and reads that file, so these tests must lay out a real directory: + + /lib/Controller/TestController.php + /lib/Service/.php + + *collaborators* maps ``ClassName -> php source``. + """ + with tempfile.TemporaryDirectory() as root: + ctl_dir = Path(root) / "lib" / "Controller" + svc_dir = Path(root) / "lib" / "Service" + ctl_dir.mkdir(parents=True) + svc_dir.mkdir(parents=True) + for name, body in collaborators.items(): + (svc_dir / f"{name}.php").write_text(body, encoding="utf-8") + ctl = ctl_dir / "TestController.php" + ctl.write_text(controller_src, encoding="utf-8") + # Pattern 4 caches per-root and per-file; a temp dir is unique per test + # but clear anyway so a reused inode can never leak a stale answer. + cni._CLASS_INDEX_CACHE.clear() + cni._COLLABORATOR_GUARD_CACHE.clear() + buf = io.StringIO() + with redirect_stdout(buf): + cni.scan_file(str(ctl)) + cni._CLASS_INDEX_CACHE.clear() + cni._COLLABORATOR_GUARD_CACHE.clear() + return [ln for ln in buf.getvalue().splitlines() if ln.strip()] + + +# The decidesk responder, reduced to the shape that matters: staffAction() +# delegates to requireStaff() which denies with 401/403; citizenAction() +# denies an anonymous caller with 401; respond() is NOT a guard — it only maps +# a result or an exception onto a JSONResponse. +_RESPONDER = """\ +requireStaff() ?? $this->respond($operation, $key, $status)); + } + + public function citizenAction(callable $operation, ?string $key = null, int $status = 200) { + $uid = $this->staffGuard->currentUid(); + if ($uid === null) { + return new JSONResponse(['message' => 'Unauthorized'], Http::STATUS_UNAUTHORIZED); + } + return $this->respond($operation, $key, $status); + } + + private function requireStaff() { + if ($this->staffGuard->currentUid() === null) { + return new JSONResponse(['message' => 'Unauthorized'], Http::STATUS_UNAUTHORIZED); + } + if ($this->staffGuard->isStaff() === false) { + return new JSONResponse(['message' => 'Forbidden'], Http::STATUS_FORBIDDEN); + } + return null; + } + + private function respond(callable $operation, ?string $key, int $status) { + return new JSONResponse([$key => $operation()], $status); + } +} +""" + + +class CollaboratorGuardDelegationTest(unittest.TestCase): + """Pattern 4a — a guard reached through an injected collaborator. + + Regression cover for the decidesk measurement of 2026-08-04: gate-7 + reported 11 findings on ParticipationController / + ParticipationBudgetController and every one was guarded, because the + guard lives on ``$this->responder`` rather than in the method body. + """ + + def test_staffAction_delegation_is_recognised_as_guarded(self): + """$this->responder->staffAction() reaches requireStaff() -> not flagged.""" + src = """\ +responder->staffAction( + operation: fn (): array => $this->lifecycleService->transitionBudgetRound($budgetId, $status), + key: 'budgetRound' + ); + } +} +""" + self.assertEqual(_scan_app(src, {"ParticipationResponder": _RESPONDER}), []) + + def test_citizenAction_delegation_is_recognised_as_guarded(self): + """citizenAction() denies an anonymous caller with 401 -> not flagged.""" + src = """\ +responder->citizenAction( + operation: fn (string $uid): array => $this->budgetService->submitProposal($budgetId, $title, $uid), + key: 'proposal' + ); + } +} +""" + self.assertEqual(_scan_app(src, {"ParticipationResponder": _RESPONDER}), []) + + def test_three_hop_intra_class_chain_to_collaborator_guard(self): + """validateProposal -> approve/reject -> applyDecision -> staffAction(). + + The exact decidesk shape that needed three hops. Transitive closure + must follow it all the way to the collaborator guard. + """ + src = """\ +rejectProposal(proposalId: $proposalId); + } + return $this->approveProposal(proposalId: $proposalId); + } + + private function approveProposal(string $proposalId) { + return $this->applyProposalDecision(proposalId: $proposalId, approve: true); + } + + private function rejectProposal(string $proposalId) { + return $this->applyProposalDecision(proposalId: $proposalId, approve: false); + } + + private function applyProposalDecision(string $proposalId, bool $approve) { + return $this->responder->staffAction( + operation: fn (): array => $this->budgetService->validateProposal($proposalId, $approve), + key: 'proposal' + ); + } +} +""" + self.assertEqual(_scan_app(src, {"ParticipationResponder": _RESPONDER}), []) + + +class CollaboratorGuardStillCatchesRealIdorTest(unittest.TestCase): + """Pattern 4 must not become a blanket clear — the negative direction. + + Every test here is a shape the gate MUST still flag. Without these, the + Pattern 4 clear is only evidence about itself: a delegation-following + gate that follows delegation to *anything* has stopped gating. + """ + + def test_plain_unguarded_method_still_flagged(self): + """No responder, no helper, no guard, caller-supplied id -> flagged.""" + src = """\ +consultationService->deleteReaction($reactionId); + return new JSONResponse(['ok' => true]); + } +} +""" + out = _scan_app(src, {"ParticipationResponder": _RESPONDER}) + self.assertEqual(len(out), 1) + self.assertIn("method=deleteReaction", out[0]) + + def test_collaborator_method_that_is_not_a_guard_still_flagged(self): + """respond() EXISTS on the responder but performs no authorisation. + + The sharpest control: resolution must discriminate between methods of + the collaborator, not clear anything called on a property whose class + happens to contain a guard somewhere. + """ + src = """\ +responder->respond( + fn (): array => $this->consultationService->deleteReaction($reactionId), + 'reaction', + 200 + ); + } +} +""" + out = _scan_app(src, {"ParticipationResponder": _RESPONDER}) + self.assertEqual(len(out), 1) + self.assertIn("method=deleteReaction", out[0]) + + def test_unresolvable_collaborator_class_clears_nothing(self): + """A type with no file under lib/ must fail closed, not fail open.""" + src = """\ +responder->staffAction( + fn (): array => $this->consultationService->deleteReaction($reactionId) + ); + } +} +""" + out = _scan_app(src, {}) + self.assertEqual(len(out), 1) + self.assertIn("method=deleteReaction", out[0]) + + def test_intra_class_chain_ending_in_no_guard_still_flagged(self): + """A three-hop chain whose terminal method has no guard at all.""" + src = """\ +hopOne(reactionId: $reactionId); + } + + private function hopOne(string $reactionId) { + return $this->hopTwo(reactionId: $reactionId); + } + + private function hopTwo(string $reactionId) { + $this->consultationService->deleteReaction($reactionId); + return new JSONResponse(['ok' => true]); + } +} +""" + out = _scan_app(src, {"ParticipationResponder": _RESPONDER}) + self.assertEqual(len(out), 1) + self.assertIn("method=deleteReaction", out[0]) + + def test_collaborator_guard_after_the_write_still_flagged(self): + """The guard must run BEFORE the mutation or it protects nothing.""" + src = """\ +consultationService->deleteReaction($reactionId); + return $this->responder->staffAction(fn (): array => []); + } +} +""" + out = _scan_app(src, {"ParticipationResponder": _RESPONDER}) + self.assertEqual(len(out), 1) + self.assertIn("method=deleteReaction", out[0]) + + def test_bare_throw_does_not_seed_a_delegation_chain(self): + """A collaborator method that only throws NotFoundException is not a guard. + + `throw` is accepted by the one-hop Pattern-1 helper rule; propagation + deliberately requires a STRICTER signal, so "this can fail" never + becomes "this checks who you are" further up the chain. + """ + thrower = """\ +mapper->find($id); + } +} +""" + src = """\ +loader->load($thingId)); + } +} +""" + out = _scan_app(src, {"ThingLoader": thrower}) + self.assertEqual(len(out), 1) + self.assertIn("method=showThing", out[0]) + + if __name__ == "__main__": unittest.main() diff --git a/hydra-gates/scripts/run-hydra-gates.sh b/hydra-gates/scripts/run-hydra-gates.sh index 6e82910..e7d3bf8 100755 --- a/hydra-gates/scripts/run-hydra-gates.sh +++ b/hydra-gates/scripts/run-hydra-gates.sh @@ -581,6 +581,16 @@ while IFS= read -r f; do _oa_files+=("$f") done < <(_enum_tracked '\.php$' lib/Service lib/Controller) _oa_ran=1 +# An EMPTY scope is not a clean tree. With zero files the log stays empty, the +# count is 0, and the gate used to report PASS having inspected nothing — the +# same failure family as .github#147, where a missing helper made gate-7 report +# PASS over 11 real unguarded endpoints. SKIPPED is the honest verdict: it is +# excluded from _EMITTED_GATES, so the coverage summary lists this gate under +# "GATES THAT DID NOT RUN" instead of folding it into ALL GATES GREEN. +if [ "${#_oa_files[@]}" -eq 0 ]; then + _oa_ran=0 + _skip 6 "orphan-auth" "scope was empty — 0 lib/Service or lib/Controller PHP file(s) in this diff, so NOTHING was inspected; orphaned (defined-but-never-called) authorization methods are UNVERIFIED by this run." +fi if [ "${#_oa_files[@]}" -gt 0 ]; then _oa_lib_dir="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")/lib" 2>/dev/null && pwd)" if [ ! -f "${_oa_lib_dir}/check_orphan_auth.py" ]; then @@ -652,6 +662,13 @@ while IFS= read -r f; do _idor_files+=("$f") done < <(_enum_tracked '\.php$' lib/Controller) _idor_ran=1 +# See the gate-6 note above: an empty scope inspected nothing, so it cannot be +# a PASS. SKIPPED keeps it out of _EMITTED_GATES and therefore visible in the +# coverage summary rather than silently green. +if [ "${#_idor_files[@]}" -eq 0 ]; then + _idor_ran=0 + _skip 7 "no-admin-idor" "scope was empty — 0 lib/Controller PHP file(s) in this diff, so NOTHING was inspected; unguarded #[NoAdminRequired] endpoints (IDOR, OWASP A01:2021) are UNVERIFIED by this run." +fi if [ "${#_idor_files[@]}" -gt 0 ]; then _gate_lib_dir="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")/lib" 2>/dev/null && pwd)" if [ ! -f "${_gate_lib_dir}/check_no_admin_idor.py" ]; then