fix(gates): eight false-positive defects across gates 3/6/7/26/29/40/44/55 - #329
Merged
rubenvdlinde merged 4 commits intoAug 10, 2026
Merged
Conversation
added 2 commits
August 10, 2026 11:09
…44/55 Every gate here reported findings against correct code, and several prescribed a fix that would have introduced a real defect. Each change is measured full-tree per repo (never from a fleet average) and ships with the true positive it must not swallow. gate-40 form-label-association (#310, #273) `inputId` is NOT a prop on NcTextField/NcInputField — absent from the 9.9.0 typings, zero occurrences in 8.39.0 — so the gate's prescribed `:input-id` was an inert attribute. Meanwhile `id` IS what reaches the <input>: 9.9.0 renders `id: __props.id`, and 8.39.0 is inheritAttrs:false with `computedId() { return this.$attrs.id ? this.$attrs.id : this.inputName }`. The Nc* branch now accepts a plain `id` matching a <label for>, as the native branch already did. The only two edits that closed the finding before were a no-op and an aria-label that OVERRIDES the visible label (WCAG 2.5.3). Also #273, keyed on the spelling that actually occurs: `aria-hidden` matched 0 of 471 fleet findings, `display:none` matched 8 — all of them type="file". Exempted as "not in the accessibility tree", with display:none bounded to type="file" because a style can be toggled by JS. 549 -> 462 (openregister exactly the 11 reported). gate-6 orphan-auth (#290) A routed action has no `->method(` anywhere; the router invokes it from appinfo/routes.php, which was never in the caller corpus. Now cleared by `'<lowerCamelController>#<method>'` or a #[Route]/#[ApiRoute] attribute. Controllers only — a service is not routable. 6 -> 5. gate-29 gitignore-then-commit (#293) The prefix builder STRIPPED a leading `!` and looked the remainder up, so a negation — which un-ignores — was read as an ignore rule. The only edit that closed it was deleting the negations, which would have ignored two real test files. Negations are now skipped and counted separately so the verdict still describes the diff. `git check-ignore` deliberately not used: it exits 0 when a negation matches AND answers from the working tree. gate-26 visual-coverage (#309) The src/views/ rule is a path-shaped proxy, so child components, widgets and NC settings panels were "pages" whose only reachable remedy is a waiver. Replaced with reachability, conservative: dropped only on positive evidence of child-ness. Found while fixing it: `type:"page"` NEVER occurs in a v2 manifest, so the manifest half of this gate was dead on every v2 app (openconnector: 35 declared pages, 0 resolved) — and a v2 `component` names the REGISTERED identifier, which an import may alias to a differently-named file. 510 -> 209; all 6 of openconnector's genuine pages retained, all 7 named false positives gone. gate-44 autocomplete-attr (#319) A bare substring alternation matched `city` inside `capacity`, `tel` inside `hotel`. Token boundaries alone were NOT enough: `email` is a whole token in `nldesign-email-footer-org-name`, which collects a name. The semantic term must now fall in the last two tokens — an identifier names its subject at the end. Window is two, not one, so `emailRecipient` (a real type="email" finding) survives. 17 -> 14, no new findings. gate-3 stub-scan (#291) The body extractor stops at `^ \}`; an interface file has none, so the "body" ran to EOF and every interface method declaring $userId was a stub. Decided on the signature terminator instead: `;` = declaration, `{` = body. The `< 4` skip never protected these — a two-method interface extracts to exactly 4 lines. 7 -> 6. Fixed-indent terminator left alone: measured 0 tab-indented method closes across four repos. gate-55 detail-page-discipline (#304) Message said an unknown icon "renders the '?' fallback"; it resolves to DEFAULT_ICON = ViewDashboard, so the intent is lost INVISIBLY — verified in four shipped builds. Message now says so and points at gate-60 for the app-level half. `--app-dir` (not this helper's interface) made argv[1] a log path, inspected nothing and exited 0; malformed invocations now exit 2 with usage text stating that exit status is not the verdict. gate-7 no-admin-idor (#297, #315) Pattern 3b: zero parameters + no request reads + no mutation + no object access. IDOR requires a caller-controlled reference. The first draft omitted the object-access condition and six EXISTING tests went red on it — a zero-input `findAll()` is an unscoped enumeration and must stay reported. Clears six methods, each read individually, all single delegations to a zero-argument catalogue/config/status call. 24 -> 18. Also #315: the FAIL message now says the guard may live in a mapper or service two frames down, and that a 404-style tenancy refusal is a guard — the note that would have prevented two near-miss vulnerability reports. It is on the runner message, not the finding line, because filter_preexisting_methods.py parses `rule=(?P<rule>.+)$`. Verification: all 53 discovered helper suites green (NODE_PATH set, so test_gate_45_to_55_acceptance.sh does not report phantom gate-53 defects). Two new runner-driven suites, test_gate_29_gitignore_negation.sh and test_gate_3_bodiless_declaration.sh, are auto-discovered and add ~7s.
SC2034 — `rc` and `out` were captured and unused. The runner's exit status is deliberately NOT the verdict here (it aggregates 60+ gates), so `rc` is dropped outright and `out` is now used for what it is worth: surfacing the gate's OWN verdict line on stderr, so a human debugging these suites can tell an unrun gate from a clean one — which otherwise look identical. SC2016 — `$userId` inside the PHP fixtures is source text, not a shell expansion, so the single quotes are required. Marked with a reason rather than reworded, since the parameter name is the fixture's whole point.
This was referenced Aug 10, 2026
Open
added 2 commits
August 10, 2026 11:16
The assignment went in the previous commit but the declaration stayed, so ShellCheck still flagged it. The runner's exit status is not this suite's verdict — the log is — so there is nothing for it to hold.
…nd-gates-3-6-7-26-29-40-44-55
rubenvdlinde
deleted the
fix/false-positive-band-gates-3-6-7-26-29-40-44-55
branch
August 10, 2026 09:27
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.
Eight gates that reported findings against correct code. Several prescribed a fix that would have introduced a real defect, and three were unclosable — no honest edit cleared them.
Closes #310, #273, #290, #293, #309, #319, #291, #297, #315. Adds the two points #304 still needed after #324.
The bar applied to every change
Two of these caught me:
findAll()is an unscoped enumeration, not a safe endpoint. That is exactly what the abuse-control bar is for.Fleet-wide finding counts
Re-measured immediately before merge, both arms in the same pass — the baselines move while you work, because other agents are fixing these apps concurrently. The DELTA is what this PR causes; the absolute numbers are a snapshot.
−398 false positives in total.
24 → 18 (−6). That was measured a few hours earlier;openregister::FederatedConfigController::typeshas since left gate-7's scope entirely because openregister#2398 removed its#[NoAdminRequired]. Five methods are cleared by Pattern 3b now, not six — the sixth was fixed in the app, not by this PR.Per gate
gate-40 (#310, #273) —
inputIdis not a prop onNcTextField/NcInputField(absent from 9.9.0 typings, zero occurrences in 8.39.0), so the prescribed:input-idwas inert.idis what reaches the<input>: 9.9.0 rendersid: __props.id; 8.39.0 isinheritAttrs:falsewithcomputedId() { return this.$attrs.id ? this.$attrs.id : this.inputName }. The Nc* branch now accepts a plainidmatching a<label for>, as the native branch already did. The only prior remedies were a no-op and anaria-labelthat overrides the visible label (WCAG 2.5.3). For #273:aria-hiddenmatched 0 of 471 fleet findings anddisplay:nonematched 8, alltype="file"— so the exemption is keyed on "not in the accessibility tree" withdisplay:nonebounded totype="file", because a style can be toggled by JS.gate-26 (#309) — the
src/views/rule is a path-shaped proxy, so rows, widgets and NC settings panels were "pages" whose only reachable remedy is a waiver. Replaced with reachability, dropping a file only on positive evidence of child-ness. Found while fixing it:type:"page"never occurs in a manifest-V2 app, so the manifest half of this gate was dead everywhere (openconnector: 35 declared pages, 0 resolved) — and a v2componentnames the registered identifier, which an import may alias to a differently-named file. All 6 of openconnector's genuine pages retained; all 7 named false positives gone.gate-7 (#297, #315) — Pattern 3b: zero parameters + no request reads + no mutation + no object access. Clears five methods (six when measured; one has since left scope), each read individually, all single delegations to a zero-argument catalogue/config/status call. The FAIL message now says the guard may live in a mapper or service two frames down, and that a 404-style tenancy refusal is a guard — the note that would have prevented two near-miss vulnerability reports. It sits on the runner message, not the finding line, because
filter_preexisting_methods.pyparsesrule=(?P<rule>.+)$.gate-29 (#293) — the prefix builder stripped a leading
!and looked the remainder up, so a negation read as an ignore rule; the only closing edit was deleting negations that rescue two real test files.git check-ignoredeliberately not used — it exits 0 when a negation matches and answers from the working tree.gate-44 (#319) — token boundaries alone were not enough:
emailis a whole token innldesign-email-footer-org-name, which collects a name. The semantic term must now fall in the last two tokens. Window is two, not one, soemailRecipient(a realtype="email"finding) survives. I tried addingorganisationto the vocabulary and reverted it — it made decidesk's site-config field a finding.gate-3 (#291) — an interface file has no
^ \}, so "the body" ran to EOF. Decided on the signature terminator instead. The< 4skip never protected these: a two-method interface extracts to exactly 4 lines. Fixed-indent terminator left alone — measured 0 tab-indented method closes across four repos.gate-55 (#304) — an unknown icon resolves to
DEFAULT_ICON = ViewDashboard, not?, so intent is lost invisibly; message corrected and now points at gate-60 for the app-level half.--app-dirmade argv[1] a log path, inspected nothing and exited 0 — malformed invocations now exit 2 with usage text stating that exit status is not the verdict.gate-6 (#290) — a routed action has no
->method(anywhere. Cleared by'<lowerCamelController>#<method>'or a#[Route]/#[ApiRoute]attribute; controllers only, since a service is not routable.Verification
All 53 discovered helper suites green with
NODE_PATHset (sotest_gate_45_to_55_acceptance.shreports no phantom gate-53 defects). Two new runner-driven suites —test_gate_29_gitignore_negation.shandtest_gate_3_bodiless_declaration.sh— are auto-discovered and add ~7s. Every helper was invoked exactly as the runner invokes it, reading the log rather than the exit code, with a 0-byte.errto prove the checker lived.