fix(gates 45-55): eleven gates passed over an unopened scope, eight over a dead interpreter, and three could not see the defect they exist for

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())

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

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):

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",
+                   "<?php\nclass SetupController {\n"
+                   "    public function status() { return 1; }\n}\n")
+        base = repo.commit("baseline")
+        repo.write("lib/Controller/SetupController.php",
+                   "<?php\nclass SetupController {\n"
+                   "    #[\\OCP\\AppFramework\\Http\\Attribute\\NoAdminRequired]\n"
+                   "    public function status() { return 1; }\n}\n")
+        repo.commit("open the endpoint to non-admins")
+        security, has_test = repo.scan(base)
+        self.assertEqual(security, ["lib/Controller/SetupController.php"])
+        self.assertFalse(has_test)
+
+    def test_end_to_end_a_comment_only_diff_is_not_reported(self):
+        repo = _Repo()
+        self.addCleanup(repo.close)
+        repo.write("lib/Controller/CharactersController.php",
+                   "<?php\nclass C {\n"
+                   "    /**\n"
+                   "     * becomes `@NoAdminRequired` again, with an ownership check.\n"
+                   "     */\n"
+                   "    public function report() { return 1; }\n}\n")
+        base = repo.commit("baseline")
+        repo.write("lib/Controller/CharactersController.php",
+                   "<?php\nclass C {\n"
+                   "    /**\n"
+                   "     * becomes admin-optional again, with an ownership check.\n"
+                   "     */\n"
+                   "    public function report() { return 1; }\n}\n")
+        repo.commit("reword one docblock sentence")
+        security, has_test = repo.scan(base)
+        self.assertEqual(security, [])
+
+
 if __name__ == "__main__":
     unittest.main(verbosity=2)