Skip to content

fix(gates 1-11): gate-11 was dead fleet-wide, eight gates passed over an empty scope, and gate-10 excused itself from four real findings - #275

Merged
rubenvdlinde merged 4 commits into
mainfrom
fix/gates-1-11-audit
Aug 9, 2026
Merged

fix(gates 1-11): gate-11 was dead fleet-wide, eight gates passed over an empty scope, and gate-10 excused itself from four real findings#275
rubenvdlinde merged 4 commits into
mainfrom
fix/gates-1-11-audit

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

What this is

Every gate in the 1–11 band was put through an acceptance test: plant one textbook true positive of exactly what the gate exists to catch, in a real fleet repo; the gate must FAIL and NAME it; remove the plant and the prior verdict must return; a clean fixture must still PASS; and no subject in scope must read na, never PASS.

Two further arms were added because a plant only fires when the gate runs — it cannot reveal a crashed checker or a gate that excused itself.

Measured across four repo shapes: larpingapp (vue-router 3, standalone), docudesk, scholiq (vue-router 4, AppHost-adopting), nldesign (zero .vue, no router, frontend in js/), plus openregister and a 16-repo static sweep for every widening. Package under test reported by the runner's own line: [hydra-gates] gate package: <sha>.

Result

gate planted TP fired? repaired?
1 spdx-headers lib/ PHP with no @license/@copyright ✅ named it empty scope → na
2 forbidden-patterns var_dump ($x); · die; · exit; all three PASSed rewritten over a masked copy
3 stub-scan empty run() · marker comment · ignored $uid ✅ all three empty scope → na
4 composer-audit — (separates "no advisories" from "did not run") none needed
5 route-auth routed method with no auth attribute ✅ — but declared a shipped file absent two-root resolver
6 orphan-auth zero-caller authz method ✅; quiet on one caller none needed
7 no-admin-idor IDOR under a fully-qualified attribute PASSed FQ form recognised
8 unsafe-auth-resolver tab-indented catch(\Throwable){return null;} ⚠️ fired by over-capture, flagged correct code brace-walked
9 semantic-auth #[NoAdminRequired] + requireAdmin() ✅ — but flagged the escalation branch narrowed
10 initial-state two-step getElementById.dataset PASSed, then declared na over 4 real findings widened surface + rule
11 admin-router doriath /settings → AdminRoot in the real router PASSed — the gate was DEAD routers discovered

gate-11 was dead fleet-wide

It read four hard-coded paths. One fleet app of fifteen has a file at any of them; the rest build their router in src/main.js. The doriath c7c72e9 defect re-planted verbatim into larpingapp's real router reported PASS; the identical line in src/router.js reported FAIL.

Proven on three shapes: larpingapp (new VueRouter), scholiq (createRouter, AppHost), nldesign (no router at all → na, verified honest). Anti-widening: the path rule resolves the enclosing route object, because a bare path: '/settings' grep flags openconnector's ADR-079 hand-off — the remediation — as the defect. 0 findings across 16 repos.

gate-10 declared na over a repo full of its own subject matter

nldesign's src/ holds one file, manifest.json. Its entire hand-written frontend is in js/. gate-10 announced "this repo ships no frontend" over a repo whose js/admin.js does exactly what it exists to catch:

var settingsEl = document.getElementById('nldesign-settings');
var tokenSets  = JSON.parse(settingsEl.getAttribute('data-token-sets'));

na removes a gate from coverage accounting, so this left the denominator. Surface is now src/ + js/, minus *.min.js.

Widening exposed a bug in my own two-step rule: one var btn = document.getElementById(...) put every btn in a 1700-line file into scope, including three forEach(function (btn) {...}) callbacks. An ambiguous name is now dropped — it can only under-report. 7 raw hits → 4, all genuine.

gate-5 called a file absent from inside the repository containing it

A\B\C PSR-4-maps to lib/A/B/CController.php, but the resolver rooted every namespaced name at lib/Controller/. On openregister — which ships the fileAppHost\Controller\GenericHealth#index was filed as "its auth attribute lives in the openregister package and is NOT visible from this repository." The gate punted to another package while standing in it, so two routed methods were judged by nobody. Both roots are now tried. Both resolve, both are correctly annotated, count unchanged at 10 — two unjudged endpoints surfaced with no invented finding. The AppHost consumer control still classifies its four absent generics as ADR-040-unresolved.

Eight gates passed over a scope they never opened

A README-only --scope-to-diff run produced PASS from gates 1, 2, 3, 5, 8, 9, 10, 11 — four of them authorization gates. All eight now say na with a reason (na not structural, per #268).

gate-9 flagged the escalation branch — and its advice was the regression

if (!isAdmin) { return 403; }          // admin IS required   -> true positive
if (!isAdmin) { if (!owner) 403; }     // admin NOT required  -> FALSE positive

Live on docudesk SigningController::cancelRequest (its own docudesk#100 fix) and procest InspectionChecklistController::submitResult. The printed remedy would have made per-user endpoints admin-only. Fleet 28 → 26; larpingapp's genuine downloadPdf finding preserved.

Two arms a planted defect cannot reach

  • ARM 4 — an attribute-only change is a change. _filter_preexisting (gates 6, 7, 8 here — not 5/9) compares against the base ref. Before fix(gates): seven of gates 23-33 could not fail on their own subject matter #276 it began at the function NAME( line, so adding #[NoAdminRequired] above an unguarded method left the body byte-identical and the finding was suppressed. Verified both ways on one fixture: cdfbd7agate-7 PASS with the finding in .log.preexisting; 34370f6 → FAIL. Pinned for gate-7.
  • ARM 5 — a broken interpreter is SKIPPED (wiring), never PASS. With python3 exiting 1, gates 2, 3, 10, 11 all skip. gate-3 is the control: it FAILS a real stub run() when python3 works.

Verified and left alone

gate-3's and gate-5's #269 repairs HOLD (inert $unused = 1; no longer closes gate-3; fail-safe delegation no longer trips it; prose does not satisfy gate-5; @auth admin-only <reason> does). gate-7 is CORRECTLY correlated.github#160's anti-correlation does not reproduce. docudesk EmlPreviewController::preview is a genuine IDOR.

Fleet churn (measured)

gate before after
2 1 1
5 10 (openregister) 10
7 21 21
9 28 26 (2 FPs)
10 0 4 (all nldesign, all genuine)
11 0 (dead) 0 (live)

Full-tree verdicts vs cdfbd7a: larpingapp and scholiq identical; docudesk differs only by the gate-9 fix; nldesign gains gate-10 FAIL and gate-11 na — previously two greens over, respectively, four real defects and zero bytes.

Tests

Five new/extended suites in run-helper-suites.sh's discovery path (7 arms), mutation-checked against whole pre-fix files: pre-fix runner → 19 failures; src/-only surface → 2; single-root resolver → 2; pre-fix check_semantic_auth.py3; pre-fix check_no_admin_idor.py1. Full run: 56 passed, 0 failed, 2 pre-existing quarantines.

No waivers, @spec exclude, .skip, continue-on-error, baselines or threshold changes. Exemptions added are code-level, not prose-level: : never (a type PHP checks), a dataset key the same file writes, and an identifier that is also a parameter.

rubenvdlinde and others added 4 commits August 8, 2026 23:58
…an empty scope

Every gate in the 1–11 band was put through one acceptance test: plant ONE
textbook true positive of exactly what the gate exists to catch, in a real
fleet repo, and require the gate to FAIL and NAME it; then remove the plant and
require the prior verdict back. Measured against gate package cdfbd7a.

gate-11 admin-router was DEAD
-----------------------------
It read four hard-coded paths — src/router/index.{js,ts}, src/router.{js,ts}.
ONE fleet app of fifteen (softwarecatalog) has a file at any of them; the other
fourteen build their router in src/main.js. Proof it was dead rather than
merely unexercised: the doriath c7c72e9 defect re-planted verbatim into
larpingapp's real router —

    routes.push({ path: '/settings', component: AdminRoot })

— reported PASS. The identical line in src/router.js reported FAIL. The
detection logic was fine; the gate never opened the file.

Routers are now discovered (anything under src/ that constructs one), the four
legacy paths are kept, and a repo with no router is `na`. The path rule moved
into check_admin_router.py, which resolves the enclosing route object: a bare
`path: '/settings'` grep would have flagged openconnector's ADR-079 hand-off —
the remediation — as the defect. Fleet churn: 0 findings across 16 repos.

EIGHT GATES REPORTED PASS OVER A SCOPE THEY NEVER OPENED
--------------------------------------------------------
A README-only commit run with --scope-to-diff produced PASS from gates 1, 2, 3,
5, 8, 9, 10 and 11 — four of them authorization gates. Gates 4/6/7 already said
NOT APPLICABLE for the identical situation, which is what made the other eight
readable as a result rather than an absence. All eight now say `na` with a
reason (`na`, not `structural`, per #268: an empty ADR-020 scope is not a gap
the author can close).

gate-9 semantic-auth flagged the ESCALATION BRANCH
--------------------------------------------------
The `!isAdmin` block was searched for a denial token AT ANY DEPTH, conflating
two opposite postures:

    if (!isAdmin) { return 403; }            admin IS required   -> true positive
    if (!isAdmin) { if (!owner) 403; }       admin NOT required   -> false positive

In the second a non-admin OWNER proceeds. Live on docudesk
SigningController::cancelRequest (its own docudesk#100 fix) and procest
InspectionChecklistController::submitResult. The remedy gate-9 printed for both
— remove @NoAdminRequired, or switch to #[AuthorizedAdminSetting] — would have
made per-user endpoints admin-only and deleted the owner checks' reason to
exist. Now only an UNCONDITIONAL denial counts. Fleet: 28 -> 26 findings; the
two removed are exactly those, and larpingapp's genuine
CharactersController::downloadPdf finding is preserved.

gate-2 forbidden-patterns failed in BOTH directions (#184)
-----------------------------------------------------------
Six raw-text greps. Invisible: `var_dump ($x)` (PHP allows whitespace before
the argument list), `die;` (a LANGUAGE CONSTRUCT, not a function), and `exit`
in every form — die's exact synonym, absent from the pattern list, so one name
was banned and the other left open. Reported: a comment saying "never use
var_dump( here", and the string literal "select dd(x)". Now judged over a
comment- and string-masked copy. `: never` exempts exit/die — a TYPE PHP itself
checks, deliberately not the @SuppressWarnings docblock sitting above the one
fleet instance, because a docblock is the load-bearing prose #196 was about.
Fleet churn: 1 -> 1.

gate-8 unsafe-auth-resolver used HARD-CODED INDENTATION
-------------------------------------------------------
Body extraction terminated on `/^    \}/`, the catch block on `/^        \}/`.
On a tab-indented file neither matches, so "the body" ran to end of file and a
resolver that correctly RETHROWS was reported as a fail-open because an
unrelated cache method further down returned null from its own catch. The
apparent detection of tab-indented fail-opens was the same over-capture by
luck. Now brace-walked over a comment-masked copy; the procest ZgwService
fail-closed shapes stay excluded by construction rather than by indentation.

gate-10 initial-state knew only the single-line shape
-----------------------------------------------------
`getElementById(...)...\.dataset` requires lookup and read on ONE line, so the
TWO-STEP form — what the doriath line becomes after any refactor — plus
querySelector and getAttribute('data-*') all reported PASS. Widened along that
one axis only; NOT to a bare `.dataset`, which would flag every legitimate
event.target.dataset in the fleet. Two measured exclusions, each a rule rather
than a waiver: a dataset key the same file also WRITES is the component's own
bookkeeping, and data-requesttoken is not IInitialState data (its accessor is
getRequestToken(), so gate-10's remedy would not apply). Fleet churn: 0 -> 0.

gate-7 no-admin-idor missed the FULLY-QUALIFIED attribute
----------------------------------------------------------
The look-back matched `#[NoAdminRequired` only. Under
`#[\OCP\AppFramework\Http\Attribute\NoAdminRequired]` a textbook IDOR fell out
of scope and reported PASS. No fleet file uses that spelling today, which is
why it needed closing deliberately — a false negative on a security gate leaves
no log.

WHAT WAS VERIFIED AND LEFT ALONE
--------------------------------
gate-3's #269 repair HOLDS in both directions: an inert `$unused = 1;` no
longer closes it, and canonical fail-safe delegation no longer trips it.
gate-5's #269 repair HOLDS: prose naming an attribute does not satisfy it, the
`@auth admin-only <reason>` declaration does, and a too-short reason does not.
gate-4 already distinguishes "no advisories" from "audit did not run".
gate-6 detects a zero-caller authorization method and goes quiet on one caller.
gate-7 is CORRECTLY correlated — .github#160's anti-correlation does not
reproduce: a leak FAILs and all four correct-fix shapes PASS. docudesk
EmlPreviewController::preview is a genuine IDOR (un-redacted EML by
caller-supplied fileId, no ownership check).

Regression tests land in run-helper-suites.sh's discovery path and are
mutation-checked against WHOLE pre-fix files, not single-site reverts: the
pre-fix runner fails 19 assertions of test_gate_1_11_empty_scope_is_na.sh
(including the gate-11 dead-gate case), the pre-fix check_semantic_auth.py
fails 3, the pre-fix check_no_admin_idor.py fails 1.

Full-tree verdicts before/after are IDENTICAL on larpingapp and openregister,
and differ on docudesk only by the intended gate-9 false positive.
Merging rather than rebasing: the shared-branch guard refuses a force-push, and
a merge keeps the already-pushed 5051c4c an ancestor so this pushes
fast-forward.
…s own comment

test_gate_route_auth.sh asserted PASS for a package.json-only diff while its
own comment said "a PASS here is scoping, not absence" — a fact PASS does not
state and NOT APPLICABLE does. Reclassified per #268. 6b/6c remain the
controls: the identical finding must still FAIL once the diff touches the
controller or appinfo/routes.php.
…to check, and gate-5 punted to a package it was standing in

Three additions after the first commit, all measured against gate package
34370f6 and reported by the runner's own `[hydra-gates] gate package:` line.

gate-10 DECLARED `na` OVER A REPO FULL OF ITS OWN SUBJECT MATTER
-----------------------------------------------------------------
The surface was `src/` only. nldesign's `src/` holds exactly one file —
`manifest.json` — and its entire hand-written frontend lives in `js/`. So
gate-10 announced "this repo ships no frontend" over a repo whose
`js/admin.js` does precisely what the gate exists to catch:

    var settingsEl = document.getElementById('nldesign-settings');
    var tokenSets  = JSON.parse(settingsEl.getAttribute('data-token-sets'));

The doriath AdminRoot defect, two-step form, in an ADMIN settings script. Four
real findings. `na` REMOVES A GATE FROM COVERAGE ACCOUNTING, so this did not
merely under-report — it left the denominator. The surface is now `src/` + `js/`
(Nextcloud's conventional shipped-script directory), minus `*.min.js`: a
committed bundle is not authored code. Fleet total 0 -> 4, all four in
nldesign, all four genuine.

AND A SCOPE-BLINDNESS BUG IN MY OWN TWO-STEP RULE
--------------------------------------------------
Widening the surface exposed it. The two-step rule matches by NAME across the
whole file. In `js/admin.js`, line 1114 binds
`var btn = document.getElementById('nldesign-save-btn')`, and that one line put
EVERY `btn` in a 1700-line file into scope — including three
`forEach(function (btn) {...})` callbacks whose `btn` is the clicked element,
i.e. the component's own markup. Four false positives from one binding. Rather
than implement JS scoping, an AMBIGUOUS NAME IS DROPPED: if an identifier is
ever a function parameter, the file cannot tell the two apart and declines to
guess. That can only under-report, which is the right direction for a rule
whose false positives would otherwise bury the real finding. 7 raw hits -> 4,
and the 4 that remain are the server-rendered reads.

gate-5 CALLED A FILE ABSENT FROM INSIDE THE REPOSITORY CONTAINING IT
---------------------------------------------------------------------
Nextcloud resolves a route name `A\B\C` against `OCA\<App>\A\B\C`, which PSR-4
maps to `lib/A/B/CController.php`. The resolver rooted every namespaced name at
`lib/Controller/` — correct for `Settings\FileSettings`, wrong for
`AppHost\Controller\GenericHealth`, whose file is
`lib/AppHost/Controller/GenericHealthController.php`.

Measured on openregister, the repository that SHIPS those classes. The derived
path did not exist, `_apphost_serves` then matched the name, and both entries
were filed as

    "served by the OpenRegister AppHost generic controller (ADR-040); its auth
     attribute lives in the openregister package and is NOT visible from this
     repository"

inside openregister. The gate punted to another package while standing in it,
so `AppHost\Controller\GenericHealth#index` and `GenericMetrics#index` had
their auth posture judged by nobody. A derived path is a GUESS; both roots are
tried and an existing one wins. Both now resolve, both are correctly annotated
(`#[PublicPage]` + `#[NoCSRFRequired]`), and openregister's gate-5 count is
unchanged at 10 — this surfaced two unjudged endpoints without inventing a
finding. The AppHost CONSUMER control still classifies its four absent generics
as ADR-040-unresolved.

TWO ACCEPTANCE ARMS A PLANTED DEFECT CANNOT REACH
--------------------------------------------------
A plant only fires when the gate RUNS, so two classes of defect are invisible
to it. Both are now asserted:

  ARM 4  an ATTRIBUTE-ONLY change is a change. `_filter_preexisting` (gates 6,
         7, 8 in this band) compares a method against the base ref and moves
         unchanged entries out of the verdict. Before #276 the comparison began
         at the `function NAME(` line, so ADDING `#[NoAdminRequired]` above an
         existing unguarded method left the body byte-identical and the finding
         was suppressed — the single edit that changes a method's auth posture
         was the one the filter could not see. Verified BOTH ways on one
         fixture: package cdfbd7a gives `[gate-7] no-admin-idor: PASS` with the
         finding in `.log.preexisting`; package 34370f6 gives FAIL. #276 fixed
         the shared helper; this keeps it fixed for gate-7, whose entire scope
         is decided by that attribute.

  ARM 5  a BROKEN INTERPRETER is `SKIPPED (wiring)`, never PASS. With python3
         exiting 1, gates 2, 3, 10 and 11 all skip. gate-3 is the control: on
         the same fixture it FAILS a real stub `run()` when python3 works, so
         the skip is the gate losing a verdict it demonstrably had.

TWO REPO SHAPES, NOT ONE
------------------------
gate-11's repair is proven on three: larpingapp (vue-router 3, `new VueRouter`),
scholiq (vue-router 4 `createRouter`, AppHost-adopting) and nldesign (no router
at all -> `na`, verified honest: no `createRouter`/`new VueRouter` anywhere in
the repo). gate-5's resolver is proven on openregister (ships the generics) and
an AppHost consumer (does not).

Full-tree verdicts vs cdfbd7a: larpingapp and scholiq IDENTICAL; docudesk
differs only by the intended gate-9 false-positive removal; nldesign gains
gate-10 FAIL (4 real findings) and gate-11 `na` (previously a green over zero
bytes).
@rubenvdlinde rubenvdlinde changed the title fix(gates): gate-11 was dead fleet-wide, and eight gates passed over an empty scope fix(gates 1-11): gate-11 was dead fleet-wide, eight gates passed over an empty scope, and gate-10 excused itself from four real findings Aug 9, 2026
@rubenvdlinde
rubenvdlinde merged commit 4eb12dc into main Aug 9, 2026
31 checks passed
rubenvdlinde added a commit that referenced this pull request Aug 9, 2026
The branch was rebased from cdfbd7a onto 48c88ba after four gate-package
releases landed mid-session (#272, #275, #276, #280/#282). The rebase gave
the same content a new history, which is a force-push, and force-push is
blocked on shared branches for good reason. The tree here is IDENTICAL to
the rebased HEAD — this commit only re-attaches the old tip as a second
parent so the push is a fast-forward.
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