diff --git a/hydra-gates/scripts/lib/check_custom_widget_ratchet.py b/hydra-gates/scripts/lib/check_custom_widget_ratchet.py index 0b322847..f46b6bb9 100644 --- a/hydra-gates/scripts/lib/check_custom_widget_ratchet.py +++ b/hydra-gates/scripts/lib/check_custom_widget_ratchet.py @@ -47,8 +47,31 @@ Output: one location-prefixed finding block per violation plus one ``[custom-widget-ratchet] base=N head=M delta=±K`` report line whenever the -ratchet is computed. Exit code is the number of findings (capped at 99); -the calling gate uses it as the failure count. +ratchet is computed, and — ALWAYS, on every successful run — one + + [custom-widget-ratchet] findings=N + +line. THAT line is the answer, not the exit status. + +WHY THE COUNT IS ON STDOUT AND NOT IN THE EXIT CODE (#209) +---------------------------------------------------------- +This helper used to return its finding count as its exit status and the gate +read it as the failure count. An exit status is ONE BYTE, and it is also how +the interpreter reports that the helper never finished. Both meanings arrived +on the same channel, so they were not distinguishable: + + * a Python traceback exits 1, and the gate reported it as + ``FAIL — 1 custom-widget finding(s)``. Measured 2026-08-08 by injecting a + ``raise`` into ``main()``: the gate produced a plausible, actionable, + entirely fictional finding, and a reader chasing it would have gone + looking for a widget that does not exist. + * the count was clamped to 99 to stay inside the byte, so any run with 100+ + findings under-reported — the same lossy-channel shape that made gate-19 + report 266 findings as 10. + +Exit status is now a plain boolean (0 = clean, 1 = findings present). A gate +that sees a non-zero exit WITHOUT a ``findings=`` line knows the helper died +and must report WIRING, never a finding. """ import os @@ -355,12 +378,20 @@ def _is_candidate_shape(path): # -------------------------------------------------------------------------- # Main. # -------------------------------------------------------------------------- +FINDINGS_LINE = "[custom-widget-ratchet] findings={n}" + + def _emit(findings, counts_line=None): if counts_line is not None: print(counts_line) for finding in findings: print(finding) - return min(len(findings), 99) + # The count, on stdout, unclamped — see the module docstring (#209). + # Printed LAST so it cannot be confused with a finding block's own text, + # and printed on EVERY successful run including the clean one, so its + # absence means "the helper did not finish". + print(FINDINGS_LINE.format(n=len(findings))) + return 1 if findings else 0 def _format_delta(delta): @@ -392,7 +423,9 @@ def main(argv): # ratchet needs a base to compare against and is not computed. head_count = sum(len(es) for es in head_customs.values()) if head_count == 0: - return 0 + # Still emit `findings=0` — the caller uses the PRESENCE of that + # line to tell a clean run from a helper that died (#209). + return _emit([]) findings = [ f"{path}:{e.line}: " + JUSTIFICATION_MSG.format(key=e.key) for path, es in sorted(head_customs.items()) @@ -433,7 +466,7 @@ def base_customs(path): for path in sorted(changed_head | base_only) ) if not active: - return 0 + return _emit([]) # Per-app counts: unchanged files contribute identically to both sides. head_count = sum(len(es) for es in head_customs.values()) diff --git a/hydra-gates/scripts/lib/check_security_cochange.py b/hydra-gates/scripts/lib/check_security_cochange.py index 98e11487..4aca8fe4 100644 --- a/hydra-gates/scripts/lib/check_security_cochange.py +++ b/hydra-gates/scripts/lib/check_security_cochange.py @@ -68,16 +68,57 @@ re.compile(r"^lib/.*/(Auth|Session|Csrf|Rbac|Permission|Authorization)/.*$"), ) -# Tokens that are a security DECLARATION wherever they appear — including in a -# docblock, because in Nextcloud the docblock form IS the declaration. +# Tokens that are a security DECLARATION — but ONLY IN A CODE POSITION. +# +# WHY THIS IS POSITION-ANCHORED, AND WHY IT WAS BOTH WRONG DIRECTIONS AT ONCE +# --------------------------------------------------------------------------- +# This used to be an unanchored alternation: the literal `#[NoAdminRequired]` +# or `@NoAdminRequired` matched anywhere on a changed line. Measured on +# larpingapp 2026-08-08, it was wrong in both directions from the same regex — +# the pairing #269 found in gate-48 and did not carry across to its sibling. +# +# FALSE POSITIVE. `CharactersController.php` carries a docblock paragraph +# explaining why a method is deliberately admin-only: +# +# * becomes `@NoAdminRequired` again, paired with a real ownership check. +# +# Rewording that ONE SENTENCE — a change with no code in it at all — made +# gate-47 demand a test co-change. That is #191's shape one level up: the +# cheapest way to clear the finding is to reword the prose again, so the +# gate is satisfiable by prose and manufactures the appearance of a security +# review. The gate's own module docstring already committed to not doing +# this ("Prose that merely mentions IUserSession is not — it is a sentence"); +# the annotation arm simply never implemented it. +# +# FALSE NEGATIVE. `#\[NoAdminRequired\]` is a LITERAL, so the equally valid +# fully-qualified form is invisible: +# +# #[\OCP\AppFramework\Http\Attribute\NoAdminRequired] +# +# A commit adding exactly that line to a controller — opening an +# admin-only endpoint to every authenticated user — with no test in the diff +# was measured to report `[gate-47] security-change-has-tests: PASS`. Same +# class as #184: a checker that greps a string literal misses every +# qualified form and matches every comment, and so fails both ways at once. +# +# THE RULE (identical to check_csrf_removal.py, deliberately) +# attribute form the line's content STARTS with `#[`, and the attribute +# group contains the token. `[^]]*` is bounded by the closing +# bracket, so `#[NoAdminRequired, NoCSRFRequired]` and the +# fully-qualified form both count, while a sentence with +# `#[NoAdminRequired]` in the middle of it does not. +# docblock form an optional comment lead-in (`*`, `//`, `#`), then the tag +# AT THE START of the content. That is the only position +# PHP's docblock parsers accept a tag in, and it is not a +# position prose reaches. The lead-in is permissive on +# purpose — `// @PublicPage` is still a declaration-shaped +# line, and narrowing to `*` only would trade this false +# positive for a false negative, which is the trap #269 +# named. What is excluded is the tag appearing PART-WAY +# THROUGH a sentence, which is the only shape prose takes. _ANNOTATION_RE = re.compile( - r"#\[NoAdminRequired\]" - r"|#\[AuthorizedAdminSetting\(" - r"|#\[PublicPage\]" - r"|#\[NoCSRFRequired\]" - r"|@NoAdminRequired\b" - r"|@NoCSRFRequired\b" - r"|@PublicPage\b" + r"^\s*#\[[^]]*\b(?:NoAdminRequired|AuthorizedAdminSetting|PublicPage|NoCSRFRequired)\b" + r"|^\s*(?:\*+|//+|\#(?!\[)|/\*+)?\s*@(?:NoAdminRequired|NoCSRFRequired|PublicPage)\b" ) # Tokens that are security-relevant only as CODE. In prose they are the name @@ -112,11 +153,17 @@ def is_security_path(path: str) -> bool: def line_is_security_relevant(line: str) -> bool: """Is this ONE changed line a security change? - A comment line qualifies only via ``_ANNOTATION_RE``. `#[` is excluded - from the `#` comment shape so a PHP 8 attribute is never read as a shell - comment. + A comment line qualifies only via ``_ANNOTATION_RE``, and only when the + annotation is at DOCBLOCK-TAG POSITION — a docblock whose tag changed is a + changed auth declaration; a sentence that names the tag is not. `#[` is + excluded from the `#` comment shape so a PHP 8 attribute is never read as + a shell comment. + + ``match`` rather than ``search``: ``_ANNOTATION_RE`` is anchored with + ``^`` on both branches, so the two are equivalent here, but ``match`` + states the intent — position is the whole point of this regex. """ - if _ANNOTATION_RE.search(line): + if _ANNOTATION_RE.match(line): return True if _COMMENT_LINE_RE.match(line): return False diff --git a/hydra-gates/scripts/lib/test_check_custom_widget_ratchet.py b/hydra-gates/scripts/lib/test_check_custom_widget_ratchet.py index b06509e9..0f7419d6 100644 --- a/hydra-gates/scripts/lib/test_check_custom_widget_ratchet.py +++ b/hydra-gates/scripts/lib/test_check_custom_widget_ratchet.py @@ -192,7 +192,19 @@ def _commit(self, entries, msg="head"): def test_new_widget_without_note_fails_justification_and_ratchet(self): self._commit([NOTED, BUILTIN, PAGE_ENTRY, NOTELESS]) rc, out = self._run() - self.assertEqual(rc, 2, out) + # THE COUNT COMES OFF STDOUT, NOT THE EXIT BYTE (#209). + # + # This asserted `rc == 2` — the helper returned its finding count as + # its exit status. That is the same channel the interpreter uses to + # report that the helper never finished, so a traceback (exit 1) was + # indistinguishable from one finding, and the gate duly printed + # "FAIL — 1 custom-widget finding(s)" over a crash. The count was also + # clamped to 99 to fit in a byte. + # + # Exit status is now boolean and the count is a line. Both are checked: + # asserting only the boolean would be weaker than what this test had. + self.assertEqual(rc, 1, out) + self.assertIn("[custom-widget-ratchet] findings=2", out) self.assertIn( 'registry["dealHeatmap"] is kind:"widget" without a _note', out, @@ -244,7 +256,17 @@ def test_untouched_registry_is_noop(self): cwd=self.repo, env=env, capture_output=True, text=True, ) self.assertEqual(proc.returncode, 0, proc.stdout) - self.assertEqual(proc.stdout, "", + # The no-op must not COMPUTE OR REPORT THE RATCHET — that is the claim. + # It was written as `stdout == ""`, which also forbade the helper from + # saying it had finished. Since #209 the `findings=` line is how a + # caller tells a clean run from a helper that died, so a truly silent + # success is now indistinguishable from a crash. The assertion is on + # the ratchet and the findings, which is what the sentence meant. + self.assertNotIn("base=", proc.stdout) + self.assertNotIn("delta=", proc.stdout) + self.assertNotIn("ADR-049", proc.stdout) + self.assertEqual(proc.stdout.strip(), + "[custom-widget-ratchet] findings=0", "no-op pass must not compute/report the ratchet") def test_legacy_noteless_entry_untouched_not_flagged(self): diff --git a/hydra-gates/scripts/lib/test_check_security_cochange.py b/hydra-gates/scripts/lib/test_check_security_cochange.py index 59728262..b5ff546b 100644 --- a/hydra-gates/scripts/lib/test_check_security_cochange.py +++ b/hydra-gates/scripts/lib/test_check_security_cochange.py @@ -21,6 +21,7 @@ from __future__ import annotations import os +import re import shutil import subprocess import sys @@ -276,5 +277,138 @@ def test_ordinary_code_is_not_security_relevant(self): self.assertFalse(csc.line_is_security_relevant(line)) +class AnnotationMustBeInACodePosition(unittest.TestCase): + """The annotation arm was wrong in BOTH directions from one regex. + + Measured on larpingapp 2026-08-08 against the pre-fix helper, which read:: + + _ANNOTATION_RE = re.compile( + r"#\\[NoAdminRequired\\]" + ... + r"|@NoAdminRequired\\b" + ... + ) + + unanchored, so position was never constrained and the attribute forms were + bare literals. Every case below was RUN against that regex first: the + false-positive cases matched it (they must not now) and the + fully-qualified cases did not (they must now). A test that only ever saw + the fixed code proves nothing about what the fix changed. + """ + + # The exact pre-fix pattern, kept verbatim so these assertions are a + # comparison and not an assertion about the current implementation. + _PRE_FIX = re.compile( + r"#\[NoAdminRequired\]" + r"|#\[AuthorizedAdminSetting\(" + r"|#\[PublicPage\]" + r"|#\[NoCSRFRequired\]" + r"|@NoAdminRequired\b" + r"|@NoCSRFRequired\b" + r"|@PublicPage\b" + ) + + # Prose that NAMES an annotation. Verbatim from larpingapp's + # CharactersController.php docblock, which explains why the method is + # deliberately admin-only; rewording that sentence made gate-47 demand a + # test co-change on a diff containing no code at all. + PROSE = ( + " * becomes `@NoAdminRequired` again, paired with a real ownership check.", + " * Deliberately NOT `@NoAdminRequired`. The body requires an administrator", + " * (#[PublicPage] + #[NoCSRFRequired]) and the response contract are owned by", + " * see the #[NoCSRFRequired] note above before changing this", + ) + + # The fully-qualified attribute forms. Valid PHP, in daily use, and + # invisible to a literal `#[NoAdminRequired]` match. + QUALIFIED = ( + " #[\\OCP\\AppFramework\\Http\\Attribute\\NoAdminRequired]", + " #[\\OCP\\AppFramework\\Http\\Attribute\\NoCSRFRequired]", + " #[\\OCP\\AppFramework\\Http\\Attribute\\PublicPage]", + " #[NoAdminRequired, NoCSRFRequired]", + ) + + def test_the_pre_fix_regex_really_did_fail_both_ways(self): + """Positive control: show the mutant CAN fail before trusting the fix. + + Without this, a green suite would be equally consistent with "the bug + was never there". + """ + for line in self.PROSE: + with self.subTest(direction="false positive", line=line): + self.assertIsNotNone( + self._PRE_FIX.search(line), + "pre-fix regex was supposed to match this prose", + ) + for line in self.QUALIFIED[:3]: + with self.subTest(direction="false negative", line=line): + self.assertIsNone( + self._PRE_FIX.search(line), + "pre-fix regex was supposed to MISS the qualified form", + ) + + def test_prose_naming_an_annotation_is_not_a_security_change(self): + for line in self.PROSE: + with self.subTest(line=line): + self.assertFalse(csc.line_is_security_relevant(line)) + + def test_a_fully_qualified_attribute_is_a_security_change(self): + for line in self.QUALIFIED: + with self.subTest(line=line): + self.assertTrue(csc.line_is_security_relevant(line)) + + def test_a_docblock_tag_at_tag_position_still_counts(self): + for line in (" * @NoAdminRequired", + " * @NoCSRFRequired", + " * @PublicPage", + "// @PublicPage", + " #[NoAdminRequired]", + " #[AuthorizedAdminSetting(Application::APP_ID)]"): + with self.subTest(line=line): + self.assertTrue(csc.line_is_security_relevant(line)) + + def test_end_to_end_a_qualified_attribute_with_no_test_is_reported(self): + """The whole-repo shape, not just the line classifier. + + Reproduces the measured miss: a commit that opens an admin-only + endpoint to every authenticated user via the qualified attribute, with + no test in the diff, reported PASS. + """ + repo = _Repo() + self.addCleanup(repo.close) + repo.write("lib/Controller/SetupController.php", + "appConfig` are PHP variables that must reach the fixture VERBATIM. +# Letting the shell expand them would write ` = ->appConfig->...` into the +# file and silently turn every arm into a test of an empty fixture, which is +# the exact "green over nothing" failure this suite exists to catch. +# shellcheck disable=SC2016 + +set -u + +_here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +_scripts="$(cd "${_here}/.." && pwd)" +_runner="${HYDRA_GATES_RUNNER_UNDER_TEST:-${_scripts}/run-hydra-gates.sh}" + +_failures=0 +_ok() { echo " ok — $1"; } +_bad() { echo " FAIL — $1"; _failures=$((_failures + 1)); } + +echo "test_gate_45_to_55_acceptance.sh" + +_tmp="$(mktemp -d "${TMPDIR:-/tmp}/hydra-g4555.XXXXXX")" +trap 'rm -rf "${_tmp}"' EXIT + +_run() { # _run [runner args...] + local app="$1" out="$2"; shift 2 + local logs="${_tmp}/logs.$$.${RANDOM}" + mkdir -p "${logs}" + ( + cd "${app}" || exit 1 + HYDRA_GATE_LOG_DIR="${logs}" bash "${_runner}" "$@" . > "${out}" 2>&1 + ) + return $? +} + +# The verdict word is not always one token — "NOT APPLICABLE" is two. +_verdict() { grep -oE "^\[gate-$2\] [^:]+: [A-Z]+( [A-Z]+)*( \([a-z]+\))?" "$1" | head -1 | sed 's/^[^:]*: //'; } + +_expect() { # _expect + local got; got="$(_verdict "$1" "$2")" + if [ "${got}" = "$3" ]; then + _ok "gate-$2 $4 → $3" + else + _bad "gate-$2 $4 → expected '$3', got '${got:-}'" + fi +} + +_commit() { git -C "$1" add -A >/dev/null 2>&1; git -C "$1" -c user.email=t@t -c user.name=t commit -qm "$2" >/dev/null 2>&1; } + +# =========================================================================== +# FAMILY A — an unopened scope must render NOT APPLICABLE, never PASS. +# =========================================================================== +_appA="${_tmp}/appA" +mkdir -p "${_appA}/src" "${_appA}/lib/Controller" "${_appA}/lib/Settings" +cat > "${_appA}/src/manifest.json" <<'JSON' +{ "$schema": "https://codeberg.org/Conduction/nextcloud-vue/raw/branch/main/src/schemas/app-manifest-v2.schema.json", "version": "0.1.0", "menu": [], "pages": [] } +JSON +printf 'export default {}\n' > "${_appA}/src/registry.js" +cat > "${_appA}/lib/Controller/ThingController.php" <<'PHP' + "${_appA}/lib/Settings/fx_register.json" +git -C "${_appA}" init -q . +_commit "${_appA}" init +printf 'docs only\n' > "${_appA}/README.md" +_commit "${_appA}" docs + +_base="$(git -C "${_appA}" rev-parse HEAD~1)" +_outA="${_tmp}/a.txt" +_run "${_appA}" "${_outA}" --scope-to-diff --base "${_base}" + +# The README-only diff opens nothing for any of these. Every one of them used +# to print PASS. +for _g in 45 46 49 50 51 53 54 55; do + _expect "${_outA}" "${_g}" "NOT APPLICABLE" "on a README-only diff (nothing inspected)" +done + +# Gates 47/48 legitimately RAN here — they classified the diff and found no +# security change — so PASS is the correct verdict and the arm below is the +# anti-widening pair for family A: the fix must not turn every gate into a +# permanent skip. +_expect "${_outA}" 47 "PASS" "classified a real (non-security) diff" +_expect "${_outA}" 48 "PASS" "examined a real (no-removal) diff" + +# ...and on a run with NO diff at all, 47/48 cannot form a verdict. +_outAfull="${_tmp}/a-full.txt" +_run "${_appA}" "${_outAfull}" +_expect "${_outAfull}" 47 "NOT APPLICABLE" "on a whole-repository run (no change set)" +_expect "${_outAfull}" 48 "NOT APPLICABLE" "on a whole-repository run (no change set)" + +# =========================================================================== +# FAMILY B — gate-45 on an app that renders from PHP templates and has no src/. +# =========================================================================== +_appB="${_tmp}/appB" +mkdir -p "${_appB}/templates/settings" +cat > "${_appB}/templates/settings/admin.php" <<'PHP' +
+

Settings

+
+ +PHP +git -C "${_appB}" init -q . +_commit "${_appB}" init + +_outB="${_tmp}/b.txt" +_run "${_appB}" "${_outB}" +_expect "${_outB}" 45 "FAIL" "sees a +PHP +_commit "${_appB}" "add the reduced-motion fallback" +_outB2="${_tmp}/b2.txt" +_run "${_appB}" "${_outB2}" +_expect "${_outB2}" 45 "PASS" "accepts a template that ships the fallback" + +# And a repo that truly renders no markup at all is still NOT APPLICABLE — the +# category must not become unreachable. +_appB3="${_tmp}/appB3" +mkdir -p "${_appB3}/lib" +printf ' "${_appB3}/lib/Nothing.php" +git -C "${_appB3}" init -q . +_commit "${_appB3}" init +_outB3="${_tmp}/b3.txt" +_run "${_appB3}" "${_outB3}" +_expect "${_outB3}" 45 "NOT APPLICABLE" "on a repo with no src/, templates/ or appinfo/templates/" + +# =========================================================================== +# FAMILY C — gate-50, both directions. +# =========================================================================== +_appC="${_tmp}/appC" +mkdir -p "${_appC}/lib/Service" + +_write_service() { # _write_service + cat > "${_appC}/lib/Service/ListingService.php" <appConfig->getValueString(Application::APP_ID, '"'"'listing_register'"'"', '"'"''"'"'); + return $reg; + }' +git -C "${_appC}" init -q . 2>/dev/null +_commit "${_appC}" init +_outC1="${_tmp}/c1.txt" +_run "${_appC}" "${_outC1}" +_expect "${_outC1}" 50 "FAIL" "sees an unguarded read whose app id is a class constant" + +# C2 — the same leak with a quoted app id. Must still fail (no regression). +_write_service ' public function scope(): string + { + $reg = $this->appConfig->getValueString('"'"'fx'"'"', '"'"'listing_register'"'"', '"'"''"'"'); + return $reg; + }' +_commit "${_appC}" "quoted app id" +_outC2="${_tmp}/c2.txt" +_run "${_appC}" "${_outC2}" +_expect "${_outC2}" 50 "FAIL" "still sees an unguarded read whose app id is a literal" + +# C3 — ANTI-WIDENING. A correct COMPOUND guard. The pre-fix regex required a +# closing paren immediately after the empty string, so this shipped as two +# findings and zero defects — and the "fix" it suggested (split into two +# single-key ifs) changes nothing about the code. +_write_service ' public function scope(): array + { + $reg = $this->appConfig->getValueString('"'"'fx'"'"', '"'"'listing_register'"'"', '"'"''"'"'); + $sch = $this->appConfig->getValueString('"'"'fx'"'"', '"'"'listing_schema'"'"', '"'"''"'"'); + if ($reg === '"'"''"'"' || $sch === '"'"''"'"') { + return []; + } + return [$reg, $sch]; + }' +_commit "${_appC}" "compound guard" +_outC3="${_tmp}/c3.txt" +_run "${_appC}" "${_outC3}" +_expect "${_outC3}" 50 "PASS" "accepts a correct compound empty-check guard" + +# C4 — ANTI-WIDENING. A guard that is not an `if` at all. Verbatim shape from +# larpingapp's SetupController::isProvisioned(); it fails closed. +_write_service ' public function isProvisioned(): bool + { + $registerId = $this->appConfig->getValueString(Application::APP_ID, '"'"'register'"'"', '"'"''"'"'); + $schemaMarker = $this->appConfig->getValueString(Application::APP_ID, '"'"'schema_marker'"'"', '"'"''"'"'); + + return $registerId !== '"'"''"'"' && $schemaMarker !== '"'"''"'"'; + }' +_commit "${_appC}" "boolean-return guard" +_outC4="${_tmp}/c4.txt" +_run "${_appC}" "${_outC4}" +_expect "${_outC4}" 50 "PASS" "accepts a boolean-return empty-check guard" + +# C5 — the opencatalogi#86 shape: one read guarded, the next one two lines +# later NOT. The gate must report the second and only the second. +_write_service ' public function scope(): array + { + $reg = $this->appConfig->getValueString('"'"'fx'"'"', '"'"'listing_register'"'"', '"'"''"'"'); + if ($reg === '"'"''"'"') { + return []; + } + $sch = $this->appConfig->getValueString('"'"'fx'"'"', '"'"'listing_schema'"'"', '"'"''"'"'); + return [$reg, $sch]; + }' +_commit "${_appC}" "guarded read + leak" +_outC5="${_tmp}/c5.txt" +_run "${_appC}" "${_outC5}" +if grep -qE "^\[gate-50\][^:]*: FAIL — 1 unsafe" "${_outC5}"; then + _ok "gate-50 reports exactly the unguarded read of the pair, not both" +else + _bad "gate-50 on a guarded+unguarded pair → $(grep -E '^\[gate-50\]' "${_outC5}" | head -1)" +fi + +# C6 — ANTI-WIDENING. A PHPCS-FORMATTED MULTI-LINE READ. +# +# Verbatim shape from procest lib/Service/AiService.php:580 and :967. Two +# five-line calls plus a blank line put the guard on the ELEVENTH line, one +# outside a window that counted from where the call BEGAN. The constant-app-id +# fix (C1) is what made these reads visible at all, so the window bug arrived +# with it: 3 findings on procest, all three textbook `empty()` guards. +_write_service ' public function writeAudit(): void + { + $registerId = $this->appConfig->getValueString( + Application::APP_ID, + '"'"'register'"'"', + '"'"''"'"' + ); + $schemaId = $this->appConfig->getValueString( + Application::APP_ID, + '"'"'ai_audit_entry_schema'"'"', + '"'"''"'"' + ); + + if (empty($registerId) === true || empty($schemaId) === true) { + $this->logger->warning('"'"'AI audit: register or schema ID not configured'"'"'); + return; + } + }' +_commit "${_appC}" "multi-line reads with a guard below them" +_outC6="${_tmp}/c6.txt" +_run "${_appC}" "${_outC6}" +_expect "${_outC6}" 50 "PASS" "accepts a PHPCS-formatted multi-line read whose guard follows it" + +# C7 — ANTI-WIDENING. The guard on the SAME LINE as the read (procest:710). +_write_service ' public function settings(): array + { + return [ + '"'"'ai_api_key_set'"'"' => $this->appConfig->getValueString(Application::APP_ID, '"'"'ai_api_key'"'"', '"'"''"'"') !== '"'"''"'"', + ]; + }' +_commit "${_appC}" "same-line emptiness check" +_outC7="${_tmp}/c7.txt" +_run "${_appC}" "${_outC7}" +_expect "${_outC7}" 50 "PASS" "accepts an emptiness check written on the read's own line" + +# C8 — the reverse control for C6/C7: the SAME multi-line shape with the guard +# DELETED must still fail. Without this, C6 and C7 could be satisfied by a +# window so wide the gate can no longer find anything. +_write_service ' public function writeAudit(): void + { + $registerId = $this->appConfig->getValueString( + Application::APP_ID, + '"'"'register'"'"', + '"'"''"'"' + ); + + $this->objectService->saveObject($registerId, []); + }' +_commit "${_appC}" "multi-line read with no guard at all" +_outC8="${_tmp}/c8.txt" +_run "${_appC}" "${_outC8}" +_expect "${_outC8}" 50 "FAIL" "still fails a multi-line read with no guard anywhere" + +# =========================================================================== +# FAMILY D — gate-53 must block the PR that CREATES larpingapp#286. +# +# `EventRoster` was registered in src/registry.js, resolvable, and named by no +# manifest position, so the event check-in surface had no entry point. When +# that defect was reintroduced exactly, gate-53 printed PASS. Direction 1 of +# the registry cross-reference stays advisory for LEGACY orphans — the gate +# cannot tell "wire it" from "delete it" — but when the DIFF ITSELF removed +# the last reference, it can, and that is the case worth blocking. +# =========================================================================== +_appD="${_tmp}/appD" +mkdir -p "${_appD}/src" +cat > "${_appD}/src/manifest.json" <<'JSON' +{ + "$schema": "https://codeberg.org/Conduction/nextcloud-vue/raw/branch/main/src/schemas/app-manifest-v2.schema.json", + "version": "0.1.0", + "menu": [{ "id": "EventDetail", "label": "Events", "icon": "Calendar", "route": "EventDetail", "order": 10 }], + "pages": [ + { + "id": "EventDetail", + "type": "detail", + "route": "/events/:id", + "title": "Event", + "config": { "sidebar": { "tabs": [ + { "id": "checkin", "label": "Check-in", "icon": "AccountCheck", "component": "EventRoster" } + ] } } + } + ] +} +JSON +cat > "${_appD}/src/registry.js" <<'JS' +import EventRoster from './views/EventRoster.vue' + +export default { + EventRoster: { kind: 'section', component: EventRoster }, +} +JS +mkdir -p "${_appD}/src/views" +printf '\n' > "${_appD}/src/views/EventRoster.vue" +git -C "${_appD}" init -q . +_commit "${_appD}" init +_baseD="$(git -C "${_appD}" rev-parse HEAD)" + +# D1 — remove the ONLY reference, keep the registry entry. This is #286. +python3 - "${_appD}/src/manifest.json" <<'PY' +import json, sys +p = sys.argv[1] +raw = open(p).read() +old = ' { "id": "checkin", "label": "Check-in", "icon": "AccountCheck", "component": "EventRoster" }\n' +assert old in raw, "PLANT ANCHOR MISSING — the fixture changed, fix the test not the anchor" +open(p, 'w').write(raw.replace(old, '', 1)) +json.load(open(p)) +PY +_commit "${_appD}" "drop the check-in tab" +_outD1="${_tmp}/d1.txt" +_run "${_appD}" "${_outD1}" --scope-to-diff --base "${_baseD}" +_expect "${_outD1}" 53 "FAIL" "blocks the PR that removes the last reference to a registered component" +if grep -q "EventRoster" "${_outD1}"; then + _ok "gate-53 NAMES the orphaned component" +else + _bad "gate-53 failed without naming EventRoster" +fi + +# D2 — ANTI-WIDENING. Removing BOTH sides is a legitimate retirement. +git -C "${_appD}" checkout -q -B d2 "${_baseD}" +python3 - "${_appD}/src/manifest.json" "${_appD}/src/registry.js" <<'PY' +import json, sys +m, r = sys.argv[1], sys.argv[2] +raw = open(m).read() +old = ' { "id": "checkin", "label": "Check-in", "icon": "AccountCheck", "component": "EventRoster" }\n' +assert old in raw, "PLANT ANCHOR MISSING (manifest)" +open(m, 'w').write(raw.replace(old, '', 1)) +json.load(open(m)) +js = open(r).read() +oldj = "\tEventRoster: { kind: 'section', component: EventRoster },\n" +assert oldj in js, "PLANT ANCHOR MISSING (registry)" +open(r, 'w').write(js.replace(oldj, '', 1)) +PY +_commit "${_appD}" "retire the check-in surface entirely" +_outD2="${_tmp}/d2.txt" +_run "${_appD}" "${_outD2}" --scope-to-diff --base "${_baseD}" +_expect "${_outD2}" 53 "PASS" "accepts removing the component and its registry entry together" + +# D3 — ANTI-WIDENING. A pre-existing orphan is still advisory, not blocking. +# Without this arm the fix would be indistinguishable from promoting +# direction 1 wholesale, which would light up every app carrying legacy debt. +_appD3="${_tmp}/appD3" +mkdir -p "${_appD3}/src/views" +cat > "${_appD3}/src/manifest.json" <<'JSON' +{ "$schema": "https://codeberg.org/Conduction/nextcloud-vue/raw/branch/main/src/schemas/app-manifest-v2.schema.json", "version": "0.1.0", "menu": [], "pages": [] } +JSON +cat > "${_appD3}/src/registry.js" <<'JS' +import Orphan from './views/Orphan.vue' + +export default { + Orphan: { kind: 'section', component: Orphan }, +} +JS +printf '\n' > "${_appD3}/src/views/Orphan.vue" +git -C "${_appD3}" init -q . +_commit "${_appD3}" init +_outD3="${_tmp}/d3.txt" +_run "${_appD3}" "${_outD3}" +_expect "${_outD3}" 53 "PASS" "leaves a PRE-EXISTING orphan advisory (WARN), not blocking" +if grep -qE '^\[gate-53\].*WARN finding' "${_outD3}"; then + _ok "gate-53 still SURFACES the pre-existing orphan as a WARN" +else + _bad "gate-53 swallowed the pre-existing orphan entirely" +fi + +# =========================================================================== +# FAMILY E — gate-52: a crashed helper is WIRING, never a finding. +# +# The runner read `_cwr_fail=$?` straight off the helper. An exit status is one +# byte and it is also how Python reports a traceback, so a dead checker +# reported `FAIL — 1 custom-widget finding(s)` — an actionable-looking finding +# with nothing behind it, pointing at a widget that does not exist. Same +# lossy-channel shape as #209, where 266 findings were reported as 10. +# +# Driven by copying the package and injecting a `raise` into the helper's +# main(), then pointing the runner-under-test at the copy. The copy is why +# this arm can exist at all: the shipped helper must not be edited to test it. +# =========================================================================== +_pkg="${_tmp}/pkg" +mkdir -p "${_pkg}" +cp -R "${_scripts}" "${_pkg}/scripts" +_broken="${_pkg}/scripts/lib/check_custom_widget_ratchet.py" +python3 - "${_broken}" <<'PY' +import sys +p = sys.argv[1] +s = open(p).read() +old = "def main(argv):" +assert old in s, "MUTATION ANCHOR MISSING — check_custom_widget_ratchet.py no longer defines main(argv)" +s = s.replace(old, 'def main(argv):\n raise RuntimeError("simulated helper crash")\n\n\ndef _unreachable_main(argv):', 1) +open(p, "w").write(s) +PY + +_appE="${_tmp}/appE" +mkdir -p "${_appE}/src" +printf "export default {\n\tThing: { kind: 'widget', component: 1 },\n}\n" > "${_appE}/src/registry.js" +git -C "${_appE}" init -q . +_commit "${_appE}" init + +_outE="${_tmp}/e.txt" +_saved_runner="${_runner}" +_runner="${_pkg}/scripts/run-hydra-gates.sh" +_run "${_appE}" "${_outE}" +_runner="${_saved_runner}" + +_expect "${_outE}" 52 "SKIPPED (wiring)" "reports a crashed helper as wiring, not as a finding" +if grep -qE '^\[gate-52\][^:]*: FAIL' "${_outE}"; then + _bad "gate-52 turned a helper crash into a FAIL with a fabricated finding count" +else + _ok "gate-52 invents no finding count when the helper did not finish" +fi + +# ANTI-WIDENING for family E: the SAME fixture with the real helper must still +# catch its planted true positive (a kind:"widget" entry with no _note). +_outE2="${_tmp}/e2.txt" +_run "${_appE}" "${_outE2}" +_expect "${_outE2}" 52 "FAIL" "still catches a kind:\"widget\" entry with no _note" + +# =========================================================================== +# FAMILY F — A CRASHED INTERPRETER MUST NOT PRODUCE A VERDICT. +# +# A planted true positive only fires WHEN THE GATE RUNS, so no arm above can +# see this. Measured 2026-08-08 on a tree carrying real findings, with a +# `python3` on PATH that exits 1 on every call: EIGHT of these eleven gates +# printed PASS. The worst was gate-46, which reported PASS over the 277 +# unresolved @spec findings — 104 distinct targets — it had reported one run +# earlier on the same files. Gates 45/49/50 discarded the status with +# `2>/dev/null`; gates 47/51/54/55 with `|| true`; gate-52 read the count off +# the exit byte, so a traceback became `FAIL — 1 custom-widget finding(s)`. +# +# `_a` is a fake `python3` earlier on PATH. Both directions are asserted: the +# same fixture with a working interpreter must produce real verdicts, or this +# family would be satisfied by a runner that skipped everything always. +# =========================================================================== +_appF="${_tmp}/appF" +mkdir -p "${_appF}/src/manifest.d" "${_appF}/lib/Controller" "${_appF}/lib/Service" \ + "${_appF}/lib/Settings" "${_appF}/templates" +cat > "${_appF}/src/manifest.json" <<'JSON' +{ "$schema": "https://codeberg.org/Conduction/nextcloud-vue/raw/branch/main/src/schemas/app-manifest-v2.schema.json", + "version": "0.1.0", "menu": [], "pages": [] } +JSON +printf "export default {}\n" > "${_appF}/src/registry.js" +printf '\n\n' \ + > "${_appF}/src/Thing.vue" +cat > "${_appF}/lib/Controller/ThingController.php" <<'PHP' +objectService->deleteObject($id); + return 1; + } +} +PHP +cat > "${_appF}/lib/Service/ListingService.php" <<'PHP' +appConfig->getValueString('fx', 'listing_register', ''); + } +} +PHP +cat > "${_appF}/lib/Settings/fx_register.json" <<'JSON' +{ "components": { "schemas": { "thing": { "properties": { + "bare": { "type": "string" }, + "rel": { "type": "string", "format": "uuid", "title": "Rel", "description": "Reference to the related thing object" } +} } } } } +JSON +git -C "${_appF}" init -q . +_commit "${_appF}" init + +# Working interpreter: these gates must produce REAL verdicts on this tree. +_outF="${_tmp}/f.txt" +_run "${_appF}" "${_outF}" +_real=0 +for _g in 45 46 49 50 51 54; do + grep -qE "^\[gate-${_g}\][^:]*: FAIL" "${_outF}" && _real=$((_real + 1)) +done +if [ "${_real}" -ge 5 ]; then + _ok "with a working interpreter the fixture yields ${_real} real FAIL verdicts (the thing a crash must not erase)" +else + _bad "fixture produced only ${_real} FAIL verdicts — family F would prove nothing" +fi + +# Broken interpreter: every one of them must say WIRING, and none may PASS. +_fakebin="${_tmp}/fakebin" +mkdir -p "${_fakebin}" +printf '#!/bin/sh\necho "python3: simulated interpreter failure" >&2\nexit 1\n' > "${_fakebin}/python3" +chmod +x "${_fakebin}/python3" +_outF2="${_tmp}/f2.txt" +( + cd "${_appF}" || exit 1 + _l="${_tmp}/logs.crash"; mkdir -p "${_l}" + PATH="${_fakebin}:${PATH}" HYDRA_GATE_LOG_DIR="${_l}" bash "${_runner}" . > "${_outF2}" 2>&1 +) +_green_over_crash="" +for _g in 45 46 47 48 49 50 51 52 54 55; do + if grep -qE "^\[gate-${_g}\][^:]*: (PASS|FAIL)" "${_outF2}"; then + _green_over_crash="${_green_over_crash} ${_g}" + fi +done +if [ -z "${_green_over_crash}" ]; then + _ok "with python3 exiting 1, no gate in the band produced a verdict — all reported wiring or na" +else + _bad "gate(s)${_green_over_crash} produced a PASS/FAIL verdict although their checker never ran" +fi +if grep -qE "^\[gate-46\][^:]*: SKIPPED \(wiring\)" "${_outF2}"; then + _ok "gate-46 says SKIPPED (wiring) rather than PASS over findings it cannot see" +else + _bad "gate-46 on a dead interpreter → $(grep -E '^\[gate-46\]' "${_outF2}" | head -1)" +fi + +echo "" +if [ "${_failures}" -eq 0 ]; then + echo "test_gate_45_to_55_acceptance.sh: ALL GREEN" + exit 0 +fi +echo "test_gate_45_to_55_acceptance.sh: ${_failures} failure(s)" +exit 1 diff --git a/hydra-gates/scripts/lib/test_gate_a11y_markup_scope.sh b/hydra-gates/scripts/lib/test_gate_a11y_markup_scope.sh index 46cbc825..cbe52cc1 100755 --- a/hydra-gates/scripts/lib/test_gate_a11y_markup_scope.sh +++ b/hydra-gates/scripts/lib/test_gate_a11y_markup_scope.sh @@ -230,7 +230,7 @@ _run "${_nosrc_app}" "${_nosrc_out}" _excused="" _still="" -for _g in 31 32 34 35 36 37 39 40 42 43 44; do +for _g in 31 32 34 35 36 37 39 40 42 43 44 45; do if grep -qE "^\[gate-${_g}\][^:]*: NOT APPLICABLE" "${_nosrc_out}"; then _excused="${_excused} ${_g}" elif ! grep -qE "^\[gate-${_g}\]" "${_nosrc_out}"; then @@ -251,9 +251,20 @@ _nosrc_caught=0 _nosrc_total=0 while IFS=: read -r _g _what; do [ -z "${_g}" ] && continue - # gate-45 (prefers-reduced-motion) is outside the 34-44 band this arm - # repairs and still guards on `[ -d src ]`; gate-38 is not in _expect. - case "${_g}" in 38|45) continue ;; esac + # gate-38 is not in _expect (it is a whole-document rule, not a per-element + # one) so it has nothing to be counted against here. + # + # gate-45 USED TO BE EXCLUDED HERE TOO, AND THE EXCLUSION WAS THE BUG + # (.github#274). #272 migrated 35/40/42/44 off `[ -d src ]` and left the + # twelfth member of the family behind — still `[ -d src ]`-guarded, still + # listed under `[ -d src ]` in the applicability table — so on this exact + # templates-only fixture gate-45 reported NOT APPLICABLE ("this repo ships + # no frontend") over a `', src, re.IGNORECASE | re.DOT continue print(f'{fname}: