diff --git a/engine/CLAUDE.core.md b/engine/CLAUDE.core.md index 488047d..dcb7362 100644 --- a/engine/CLAUDE.core.md +++ b/engine/CLAUDE.core.md @@ -26,6 +26,7 @@ These override brevity. If proof makes a message longer, the message gets longer - If a test was skipped, timed out, or I ran a subset, say exactly which and why — never let a partial run stand in for a full one. - If the user asks "did you verify X?", answer yes or no first, then show the evidence or admit there is none. Do not re-argue the original claim. - When I catch myself about to assert something I did not observe, stop and run the check instead of writing the sentence. +- **A check that could not run is not a pass.** When a guard, gate, scan, or query meets input it cannot read — a file past a size cap, an unresolved path, a field a projection omits, a probe that errored — it says so or refuses. It never returns clean. Give such a check three outcomes (hit, clean, unchecked), not two, and pin the third with a test; whether it then fails open or closed is a per-check decision that gets written down. Saltzer and Schroeder put the burden the same way in "Basic Principles of Information Protection" (1975): base access decisions on permission rather than exclusion, so the default is lack of access and the scheme names the conditions under which access is permitted (https://web.mit.edu/Saltzer/www/publications/protection/Basic.html). - The same rule applies to claims about the conversation itself, not just about code: "I ignored/missed/forgot X" is a claim that needs evidence too. Grep the actual transcript for the instruction before saying that. If nothing turns up, say "I don't have a record of that instruction in this session" — not self-blaming language for something that was never said. - A Grep or name hit is not a check. Do not cite a file, line, or "the bug is X" until this turn's Read or command output is in the same message. If two files could match, Read both. Prefix `UNVERIFIED:` until then. Saying "my earlier check was wrong" means the claim went out before the check — that is a process failure, not a polite recovery. diff --git a/engine/hooks/scope-lock/tests/test_hooks.py b/engine/hooks/scope-lock/tests/test_hooks.py index c80d42e..5809fb2 100644 --- a/engine/hooks/scope-lock/tests/test_hooks.py +++ b/engine/hooks/scope-lock/tests/test_hooks.py @@ -64,6 +64,45 @@ def run_main(main, payload: dict) -> tuple[int, str, str]: return code, out.getvalue(), err.getvalue() +class TestUnreadableInput(unittest.TestCase): + """Pin what happens to input the detector cannot read. + + Two files feed a decision here, and they resolve in opposite directions + on purpose: a corrupt state file means no lock is in force, while an + unreadable transcript means no scope contract was recorded, so a lock + already in force is not released by a file that could not be read. + """ + + def setUp(self) -> None: + self.tmp = tempfile.TemporaryDirectory() + self.saved_state_dir = detect.STATE_DIR + detect.STATE_DIR = self.tmp.name + + def tearDown(self) -> None: + detect.STATE_DIR = self.saved_state_dir + self.tmp.cleanup() + + def test_fails_open_when_the_state_file_is_malformed(self): + payload = {"session_id": "session-corrupt"} + with open(detect.state_path(payload), "w", encoding="utf-8") as handle: + handle.write("{not json at all") + self.assertEqual(detect.load_state(payload), {}) + + def test_fails_open_when_the_state_file_holds_a_non_object(self): + payload = {"session_id": "session-list"} + with open(detect.state_path(payload), "w", encoding="utf-8") as handle: + handle.write("[1, 2, 3]") + self.assertEqual(detect.load_state(payload), {}) + + def test_unreadable_transcript_records_no_contract_so_a_lock_holds(self): + payload = {"session_id": "session-2", "transcript_path": "/nonexistent/session.jsonl"} + self.assertEqual(detect.recorded_contract(payload, 0), "") + + def test_missing_transcript_counts_zero_lines_rather_than_guessing(self): + payload = {"session_id": "session-3", "transcript_path": "/nonexistent/session.jsonl"} + self.assertEqual(detect._line_count(detect._transcript_path(payload)), 0) + + class ScopeLockCase(unittest.TestCase): def setUp(self) -> None: self.tmp = tempfile.TemporaryDirectory() diff --git a/scripts/check_hook_test_coverage.py b/scripts/check_hook_test_coverage.py index 22a58a5..ba89e6f 100755 --- a/scripts/check_hook_test_coverage.py +++ b/scripts/check_hook_test_coverage.py @@ -38,6 +38,31 @@ "clean", "empty", ) +UNCHECKED_RE = ( + "unscannable", + "unchecked", + "unreadable", + "too_large", + "over_ceiling", + "refus", + "cannot_check", + "could_not_read", + "fails_open", + "fail_open", + "missing", + "nonexistent", + "no_transcript", + "garbage", + "corrupt", + "malformed", +) +READS_INPUT_RE = ( + "open(", + "read(", + "getsize", + "st_size", + "isfile", +) POSITIVE_RE = ( "hit", "fire", @@ -79,6 +104,21 @@ def _test_names(tests_dir: str) -> list[str]: return names +def reads_external_input(detector_path: str) -> bool: + """True when a detector opens or sizes a file it did not receive inline. + + Such a detector has a third outcome besides hit and clean: input it could + not read. That outcome has to be pinned by a test, whichever way the hook + resolves it, so it cannot silently collapse into clean. + """ + try: + with open(detector_path, encoding="utf-8") as handle: + source = handle.read() + except OSError: + return False + return any(token in source for token in READS_INPUT_RE) + + def hooks_with_detector() -> list[str]: if not os.path.isdir(HOOKS_DIR): return [] @@ -109,6 +149,17 @@ def check_hook(hook_dir: str) -> list[str]: f"{name}: no negative test found (a test name matching {NEGATIVE_RE} " "that proves the detector stays silent on a clean case)" ) + detector = os.path.join(hook_dir, "detect.py") + if reads_external_input(detector) and not any( + pat in n.lower() for n in names for pat in UNCHECKED_RE + ): + problems.append( + f"{name}: detect.py reads files but no test pins its behavior on input it " + "could not read (name it for the unreadable case: unscannable, unreadable, " + "too_large, refuses, fails_open, missing, malformed). Fail open or fail " + "closed is the hook's own documented choice; leaving it untested, so an " + "unchecked file passes as clean, is not." + ) return problems diff --git a/tests/test_check_hook_test_coverage.py b/tests/test_check_hook_test_coverage.py new file mode 100644 index 0000000..13a05d9 --- /dev/null +++ b/tests/test_check_hook_test_coverage.py @@ -0,0 +1,99 @@ +#!/usr/bin/env python3 +"""Positive + negative tests for check_hook_test_coverage's unchecked-input rule. + +A detector that opens a file has a third outcome besides hit and clean: +input it could not read. This gate requires a test that pins that outcome, +whichever way the hook resolves it, so an unchecked file cannot pass as +clean by default. Fixtures here are synthetic hook directories. +""" +from __future__ import annotations + +import sys +import tempfile +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO / "scripts")) +import check_hook_test_coverage as chtc # noqa: E402 + +FIRES_AND_SILENT = ( + "def test_hit_detects_the_bad_case():\n pass\n\n" + "def test_no_hit_stays_silent_on_a_clean_case():\n pass\n" +) +UNREADABLE_TEST = "\n\ndef test_fails_open_on_unreadable_input():\n pass\n" +INLINE_DETECTOR = "import re\nPATTERN = re.compile('x')\n\ndef decide(payload):\n return None\n" +FILE_READING_DETECTOR = ( + "import os\n\n" + "def decide(payload):\n" + " path = payload['tool_input']['path']\n" + " if os.path.getsize(path) > 1024:\n" + " return None\n" + " with open(path) as handle:\n" + " return handle.read()[:1]\n" +) + + +def hook(root: Path, name: str, detector: str, tests: str) -> Path: + hook_dir = root / name + (hook_dir / "tests").mkdir(parents=True) + (hook_dir / "detect.py").write_text(detector, encoding="utf-8") + (hook_dir / "tests" / "test_hooks.py").write_text(tests, encoding="utf-8") + return hook_dir + + +class TestUncheckedInputRule(unittest.TestCase): + def test_file_reading_detector_without_an_unreadable_test_fails(self): + with tempfile.TemporaryDirectory() as tmp: + hook_dir = hook(Path(tmp), "reader", FILE_READING_DETECTOR, FIRES_AND_SILENT) + problems = chtc.check_hook(str(hook_dir)) + self.assertEqual(len(problems), 1, problems) + self.assertIn("could not read", problems[0]) + + def test_file_reading_detector_with_an_unreadable_test_passes(self): + with tempfile.TemporaryDirectory() as tmp: + hook_dir = hook( + Path(tmp), "reader", FILE_READING_DETECTOR, FIRES_AND_SILENT + UNREADABLE_TEST + ) + self.assertEqual(chtc.check_hook(str(hook_dir)), []) + + def test_inline_only_detector_is_not_asked_for_an_unreadable_test(self): + with tempfile.TemporaryDirectory() as tmp: + hook_dir = hook(Path(tmp), "inline", INLINE_DETECTOR, FIRES_AND_SILENT) + self.assertEqual(chtc.check_hook(str(hook_dir)), []) + + def test_reads_external_input_detects_a_size_check(self): + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "detect.py" + path.write_text(FILE_READING_DETECTOR, encoding="utf-8") + self.assertTrue(chtc.reads_external_input(str(path))) + + def test_reads_external_input_is_false_for_an_inline_detector(self): + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "detect.py" + path.write_text(INLINE_DETECTOR, encoding="utf-8") + self.assertFalse(chtc.reads_external_input(str(path))) + + def test_missing_detector_file_fails_open_rather_than_erroring(self): + self.assertFalse(chtc.reads_external_input("/nonexistent/detect.py")) + + +class TestExistingRulesStillHold(unittest.TestCase): + def test_no_positive_test_still_fails(self): + with tempfile.TemporaryDirectory() as tmp: + hook_dir = hook( + Path(tmp), "silent-only", INLINE_DETECTOR, + "def test_no_hit_stays_silent():\n pass\n", + ) + problems = chtc.check_hook(str(hook_dir)) + self.assertTrue(any("no positive test" in p for p in problems), problems) + + def test_the_real_repo_passes_this_gate(self): + problems = [] + for hook_dir in chtc.hooks_with_detector(): + problems.extend(chtc.check_hook(hook_dir)) + self.assertEqual(problems, []) + + +if __name__ == "__main__": + unittest.main()