diff --git a/hydra-gates/README.md b/hydra-gates/README.md index 77a0147..d0a4268 100644 --- a/hydra-gates/README.md +++ b/hydra-gates/README.md @@ -399,6 +399,37 @@ protection. So every run states how many waivers it honoured: A green earned by passing is then distinguishable from one earned by waiving. +### Declaring an admin-only endpoint (gate 5) + +`@auth admin-only ` is a **declaration**, not a waiver, and it exists +because gate 5 was otherwise unsatisfiable for a whole class of correct code. + +In Nextcloud, **admin is the default and is expressed by the ABSENCE of an +attribute**: `#[NoAdminRequired]` *widens* access, so an admin-only controller +method has no attribute available to declare itself with. Gate 5's finding is +precisely "no attribute", so before 2026-08-08 such a method could only be +reported — or silently exempted by the bug in +[#196](https://github.com/ConductionNL/.github/issues/196), where a docblock +*mentioning* `#[NoAdminRequired]` satisfied the grep. Two methods with +identical auth posture got opposite verdicts depending on their prose. + +Closing that false negative without adding a declaration would have converted a +silent false negative into a **permanent** false positive on correct code — the +no-honest-end-state shape. So the posture is stated instead: + +```php +/** + * @auth admin-only writes billing state for every tenant; admin posture is the absence of NoAdminRequired + */ +public function update(string $id): JSONResponse +``` + +The reason must be at least 20 characters and sit at docblock-tag position. +**Making mere absence sufficient was considered and rejected**: absence is the +only thing gate 5 reports, so accepting it would empty the gate completely. +gate 9 (semantic-auth) still owns whether the declared posture matches the +method body. + --- ## Testing the package diff --git a/hydra-gates/scripts/lib/check_csrf_removal.py b/hydra-gates/scripts/lib/check_csrf_removal.py new file mode 100644 index 0000000..2c852c7 --- /dev/null +++ b/hydra-gates/scripts/lib/check_csrf_removal.py @@ -0,0 +1,92 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: EUPL-1.2 +"""Gate-48 csrf-cochange — which removed lines actually DROPPED CSRF protection? + +WHY THIS EXISTS (#191) +---------------------- +The gate found removed attributes like this:: + + git diff -U0 "${BASE_REF}...HEAD" -- 'lib/Controller/*.php' \\ + | grep -E '^-.*(@NoCSRFRequired|#\\[NoCSRFRequired\\])' + +`^-.*` puts no constraint on where in the line the token sits, so a comment +that NAMES the attribute is indistinguishable from an attribute that was +deleted. Measured on nldesign (`development` vs `origin/beta`): the gate was +red, nothing about CSRF had changed, and the matched line was one removed +sentence from a class docblock — + + - * (#[PublicPage] + #[NoCSRFRequired]) and the response contract are owned by + +— replaced by another sentence saying the same thing. + +WHY THIS ONE IS ESPECIALLY BAD TO LEAVE +--------------------------------------- +The cheapest way to clear the finding is to REWORD A COMMENT. That changes +nothing about CSRF and teaches exactly the habit the gate exists to prevent, +so a gate satisfiable by prose is worse than no gate: it manufactures the +appearance of a security review. Same family as #184, where gate-64 grepped a +quoted string literal and so matched every comment and missed every constant. + +THE RULE +-------- +A removed line counts only when the token is in a CODE POSITION: + + attribute form after the `-`, optional whitespace, the content STARTS with + `#[`, and that attribute list contains `NoCSRFRequired`. + `#[NoAdminRequired, NoCSRFRequired]` counts; a sentence with + `#[NoCSRFRequired]` in the middle of it does not. + + docblock form after the `-`, optional whitespace, an optional leading `*` + and optional whitespace, the content STARTS with + `@NoCSRFRequired`. That is the only position PHP's own + docblock parsers accept a tag in, and it is not a position + prose reaches. + +The nldesign line fails both — its `#[NoCSRFRequired]` sits mid-sentence after +`(` — while a genuine deletion of either form still matches. + +Usage:: + + check_csrf_removal.py < unified.diff + +Prints the removed lines that are real CSRF removals, one per line. Exits 0 +always; the OUTPUT is the answer (#209). +""" +from __future__ import annotations + +import re +import sys + +# `-` then optional whitespace then `#[`, with NoCSRFRequired inside the +# attribute group. `[^]]*` is bounded by the closing bracket so a `#[Foo]` +# followed later on the line by the word NoCSRFRequired in prose cannot match. +ATTRIBUTE_REMOVED = re.compile(r'^-\s*#\[[^]]*\bNoCSRFRequired\b') +# `-` then optional whitespace, an optional docblock star, optional +# whitespace, then the tag AT THE START of the content. +DOCBLOCK_TAG_REMOVED = re.compile(r'^-\s*(?:\*\s*)?@NoCSRFRequired\b') + +# A diff header line is `---` / `---` shaped; it is not a removed line of code. +DIFF_HEADER = re.compile(r'^---(\s|$)') + + +def removals(diff: str) -> list[str]: + out = [] + for line in diff.splitlines(): + if not line.startswith('-') or DIFF_HEADER.match(line): + continue + if ATTRIBUTE_REMOVED.match(line) or DOCBLOCK_TAG_REMOVED.match(line): + out.append(line) + return out + + +def main(argv: list[str]) -> int: + if len(argv) > 1: + print("usage: check_csrf_removal.py < unified.diff", file=sys.stderr) + return 2 + for line in removals(sys.stdin.read()): + print(line) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/hydra-gates/scripts/lib/check_js_call_sites.py b/hydra-gates/scripts/lib/check_js_call_sites.py new file mode 100644 index 0000000..69583dc --- /dev/null +++ b/hydra-gates/scripts/lib/check_js_call_sites.py @@ -0,0 +1,215 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: EUPL-1.2 +"""Gate-34 (window-confirm) and gate-58 (e2e-networkidle), over CODE only. + +Both gates grepped raw file text for a fixed spelling, and both failed in the +two directions #184 named: + +GATE-34 (#224), measured in four arms on doriath +------------------------------------------------ + arm 1 a comment saying the component "deliberately avoids + window.confirm() and uses NcDialog" -> FAIL, false RED + arm 2 the same file with that comment deleted -> PASS (the control + proving arm 1's finding IS the comment) + arm 3 `if (!window['confirm']('Delete everything?'))` -> PASS, false GREEN + arm 4 the same code written `window.confirm(...)` -> FAIL (the control + proving the probe can fire) + +Arm 1 punishes the code that did the right thing and teaches people not to +write down why. Arm 3 is the serious one: the native call it hid on doriath +guarded a CASCADING DELETE. `window['confirm']` is not a contrived bypass — +it is what several minifiers and some lint autofixes emit, and +`const { confirm } = window` is ordinary style. + +GATE-58 (#230), measured on larpingapp +-------------------------------------- +The gate reported one finding. The line was:: + + // live `waitForLoadState('networkidle')` in the suite; every other mention + +i.e. a comment explaining that the LAST live call had been removed. The repo +has zero live calls; all ten occurrences under tests/ are comments. This is +doubly perverse — the more carefully a repo documents the ADR-074 rule 4 +removal, the more findings it accrues — and unfixable in the leaf, because +the only remedies are deleting the explanation or putting an `exclude` marker +on a comment, which is comment-satisfaction. + +WHY A MASK AND NOT A LINE FILTER +-------------------------------- +#230 suggested dropping lines that START with `//`. That would still count:: + + const wait = 1 // waitForLoadState('networkidle') is banned + +and would still fire on a block comment's interior lines that begin with a +letter. The mask blanks the comment REGIONS, whatever column they start in, +and leaves a real call with a trailing comment intact — asserted both ways. + +STRING LITERALS ARE KEPT. Gate-19's mask blanks string CONTENTS, because for +gate-19 "is argument 1 a string" is the discriminator. Here the string IS the +evidence: `'networkidle'` and `window['confirm']` are both string literals. +Using the wrong mask would have turned a fixed gate into a dead one — the +failure mode this package has already shipped eleven times. + +OFFSETS SURVIVE, WHICH IS WHAT MAKES THE SUPPRESSION MARKER WORK. The +`e2e-networkidle exclude ` marker lives IN a comment — exactly what +the mask blanks — so the match is found in the mask and the marker is read +out of the ORIGINAL text at the same line number. + +Usage:: + + check_js_call_sites.py --rule native-dialog|networkidle [...] + +One finding per line, `path:line: `; exits 0 always (#209). +""" +from __future__ import annotations + +import os +import re +import sys + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +from source_scope import js_code_mask, js_exec_mask, read_text # noqa: E402 + +_DIALOGS = "confirm|alert|prompt" + +# ANCHOR, THEN READ THE ORIGINAL. +# +# The anchor patterns run against a mask whose STRING CONTENTS are blanked, +# so a sentence of documentation is not a call site:: +# +# const doc = 'do not use window.confirm here' +# +# But the evidence for the two defects being fixed IS a string literal — +# `window['confirm']`, `'networkidle'`. Both cannot be true of one mask, so +# they are two questions asked of two texts at ONE coordinate: the anchor says +# "this offset is code", and the full pattern is then matched against the +# ORIGINAL text at that same offset. Every mask in source_scope preserves +# offsets precisely so this is available. +# ⚠️ The `=` alternative is a LOOKAHEAD, so it consumes only the `=` itself. +# Written as `=\s*window\s*[.\[]` it swallowed the `window` that follows, and +# `finditer` (which returns NON-OVERLAPPING matches) then never offered the +# `window` anchor — so `const r = window.confirm('x')` matched the alias rule, +# failed it because a `(` follows, and reported NOTHING. A real call, silently +# dropped by an anchor that was one character too greedy. +DIALOG_ANCHOR = re.compile( + r"(? list[int]: + """Line numbers where *anchor* sits at a code position in *masked* AND + *full* matches the ORIGINAL text starting there. + + Overlapping anchors on one construct (`window[` matches once) cannot + double-count, because the offsets are de-duplicated by line and start. + """ + seen: set[int] = set() + out: list[int] = [] + for m in anchor.finditer(masked): + start = m.start() + if not full.match(src, start): + continue + if start in seen: + continue + seen.add(start) + out.append(src.count("\n", 0, start) + 1) + return out + + +def _native_dialog(path: str, src: str) -> list[str]: + masked = js_exec_mask(src, path) + original = src.splitlines() + out = [] + for line_no in _hits(src, masked, DIALOG_ANCHOR, NATIVE_DIALOG): + text = original[line_no - 1].strip() if line_no <= len(original) else "" + out.append(f"{path}:{line_no}:{text}") + return out + + +def _networkidle(path: str, src: str) -> list[str]: + masked = js_code_mask(src) + original = src.splitlines() + out = [] + for line_no in _hits(src, masked, NETWORKIDLE_ANCHOR, NETWORKIDLE): + text = original[line_no - 1] if line_no <= len(original) else "" + # The suppression marker is written in a comment, which the mask + # blanked — so it is read from the ORIGINAL line. + if NETWORKIDLE_EXCLUDE in text: + continue + out.append(f"{path}:{line_no}:{text.strip()}") + return out + + +RULES = { + "native-dialog": _native_dialog, + "networkidle": _networkidle, +} + + +def scan_source(rule: str, path: str, src: str) -> list[str]: + return RULES[rule](path, src) + + +def main(argv: list[str]) -> int: + if len(argv) < 4 or argv[1] != "--rule" or argv[2] not in RULES: + print("usage: check_js_call_sites.py --rule native-dialog|networkidle ...", + file=sys.stderr) + return 2 + rule = argv[2] + for path in argv[3:]: + try: + src = read_text(path) + except OSError: + continue + for line in scan_source(rule, path, src): + print(line) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/hydra-gates/scripts/lib/check_markup_a11y.py b/hydra-gates/scripts/lib/check_markup_a11y.py new file mode 100644 index 0000000..262433e --- /dev/null +++ b/hydra-gates/scripts/lib/check_markup_a11y.py @@ -0,0 +1,140 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: EUPL-1.2 +"""Gate-31 (img-alt) and gate-32 (semantic-controls), over MARKUP only. + +WHY THIS MOVED OUT OF THE BASH GATES +------------------------------------ +Both gates read a file like this:: + + _flat=$(tr '\\n' ' ' < "${vue}") + echo "${_flat}" | grep -oE ']*>' + +Two defects in three lines, and both were measured live: + +1. THE WHOLE FILE IS FLATTENED, so `` +# or ``, and an HTML parser ends the element at both — so the +# regex would run the "script body" on past the real close and blank markup +# that ships. Caught by CodeQL (py/bad-tag-filter, high) on this very change: +# a mask that over-blanks is a gate that reports nothing, which is the failure +# mode this whole change exists to remove. +_SCRIPT_BLOCK = re.compile(r']*)?>(.*?)]*)?>', re.DOTALL | re.IGNORECASE) +_STYLE_BLOCK = re.compile(r']*)?>(.*?)]*)?>', re.DOTALL | re.IGNORECASE) + + +def _blank_span(buf: list[str], a: int, b: int) -> None: + for k in range(max(a, 0), min(b, len(buf))): + if buf[k] != "\n": + buf[k] = " " + + +def _top_level_template_spans(text: str) -> list[tuple[int, int]]: + """(start, end) of each TOP-LEVEL `