From 1a200aa65dd1bc84bbe103c47d6c66d5bf62e105 Mon Sep 17 00:00:00 2001 From: forge Date: Mon, 26 Oct 2026 06:39:32 +0000 Subject: [PATCH] forge: harden triage verifier against no_main_guard SystemExit escape --- .../MUTANT_GUARD_REFACTOR_VERIFICATION.md | 101 +++++++++++++++++ .../forge/_mutant_scan_triage_dump_pending.py | 103 ++++++++++++++++++ .../verify_triage_dump_pending_refactor.py | 99 ++++++++++++++--- 3 files changed, 289 insertions(+), 14 deletions(-) create mode 100644 wizard-work/forge/MUTANT_GUARD_REFACTOR_VERIFICATION.md create mode 100644 wizard-work/forge/_mutant_scan_triage_dump_pending.py diff --git a/wizard-work/forge/MUTANT_GUARD_REFACTOR_VERIFICATION.md b/wizard-work/forge/MUTANT_GUARD_REFACTOR_VERIFICATION.md new file mode 100644 index 0000000..c50a928 --- /dev/null +++ b/wizard-work/forge/MUTANT_GUARD_REFACTOR_VERIFICATION.md @@ -0,0 +1,101 @@ +# MUTANT-GUARD REFACTOR VERIFICATION + +**Target:** `wizard-work/forge/verify_triage_dump_pending_refactor.py` +**Instrument:** `wizard-work/forge/_mutant_scan_triage_dump_pending.py` +**Lane:** Refactor (forge) +**Verdict:** FIXED -- the `no_main_guard` mutant escape is closed. + +## The defect (measured, not argued) + +The `no_main_guard` mutant strips the `if __name__ == "__main__":` guard from +`_triage_dump_pending_refactored.py`, leaving `sys.exit(main())` at module scope. + +Before this fix, that mutant **SURVIVED** the verifier: + +* `check_path_override_honoured` called `importlib.import_module(MOD_NAME)` + *unguarded* (no `try/except`). Importing the mutant module ran `main()`, which + leaked the entire probe output to **real stdout** (the seed file's contents -- + 7932 bytes, "SOURCE: 'Muse web artifact..." onward), then `sys.exit(0)` raised + `SystemExit(0)`. +* `SystemExit(0)` propagated out of `check_path_override_honoured`, out of the + verifier's `main()`, and **terminated the verifier process with rc 0** -- + before the verifier reached its final `sys.exit(1)`. +* The verifier therefore reported green on a tree carrying a guard-less module. + +## The fix (behavior-preserving on clean input) + +Extracted `_safe_import()` which centralises the safe-import pattern already +used by `check_import_side_effect_free`: + + * pops `MOD_NAME` from `sys.modules` + * ensures `WORK_DIR` is on `sys.path` + * imports inside `redirect_stdout`/`redirect_stderr` (captures, never leaks) + * catches every `BaseException` (including `SystemExit`) + * returns `(module, exc, out, err)`; `exc` set iff module is `None` + +Both import sites now use it: + + * `check_import_side_effect_free` -- unchanged semantics: exc -> FAIL, + output -> FAIL, else OK. + * `check_path_override_honoured` -- was unguarded; now catches `exc` and FAILs + instead of letting `SystemExit` kill the process. A defensive + `assert mod is not None` makes the mutual-exclusion invariant explicit. + +`exc is None` implies `mod is not None`, so the subsequent +`mod.main(path=custom)` call is unreachable when the import failed. + +## Evidence (real commands, real outputs) + +``` +$ cd /root/troupe-standup/wizard-work/forge +$ python3 verify_triage_dump_pending_refactor.py +OK: py_compile passes +OK: identical output (exit 0, 7932 bytes) +OK: __main__ guard present +OK: no 'global' statements in code +OK: import is side-effect-free (no probe output at import) +OK: _print_summary, _print_decided, _print_pending, load_data, main extracted; main calls all three print helpers +OK: main() delegates via distinct standalone statements (no ';'-compaction) +OK: main(path) accepts a configurable path parameter +OK: original still exhibits the removed smells (refactor is non-vacuous) +OK: main(path=custom) honours the override (prints custom file, not DEFAULT_PATH) + +=== Verification: 10/10 checks passed === exit 0 +``` + +Mutation scan (copies artifacts to a temp dir, never touches the real files): + +``` +$ python3 _mutant_scan_triage_dump_pending.py + no_main_guard DIED exit=1 === Verification: 7/10 checks passed === + path_ignored DIED exit=1 === Verification: 9/10 checks passed === + skip_pending DIED exit=1 === Verification: 8/10 checks passed === + stub_data DIED exit=1 === Verification: 8/10 checks passed === + remove_path_param DIED exit=1 TypeError: main() got an unexpected keyword argument 'path' + hardcode_source DIED exit=1 === Verification: 9/10 checks passed === + inline_helpers DIED exit=1 === Verification: 9/10 checks passed === + +Survivors: none +exit 0 +``` + +Before the fix, `no_main_guard` was `SURVIVED` with a leaked probe dump on +stdout and `exit=0`. After the fix it is `DIED` (exit 1, 7/10 checks): +`check_import_side_effect_free` catches the `SystemExit` AND +`check_path_override_honoured` now catches it too, so the verifier reaches its +final verdict row and exits non-zero. + +## Non-vacuity + +* The clean (unmutated) refactored module still passes 10/10 -- the added + `_safe_import` guards report OK on a healthy import. +* The mutant scan still kills every *other* mutant identically to before (the + fix changes only the `no_main_guard` case). +* Run twice for determinism; both runs report `Survivors: none`. + +## Scope + +Only `verify_triage_dump_pending_refactor.py` changed. The subject module +`_triage_dump_pending_refactored.py` is untouched (it already had the guard); +the vulnerability was in the *verifier's* unguarded second import, not the +refactored subject. diff --git a/wizard-work/forge/_mutant_scan_triage_dump_pending.py b/wizard-work/forge/_mutant_scan_triage_dump_pending.py new file mode 100644 index 0000000..ecaea74 --- /dev/null +++ b/wizard-work/forge/_mutant_scan_triage_dump_pending.py @@ -0,0 +1,103 @@ +#!/usr/bin/env python3 +"""Safe mutation-test for the triage-dump-pending refactor verifier. + +Copies the verifier + original + refactored script into a TEMP dir (the real +artifact is never modified), applies each mutant to the temp copy of the +refactored module, runs the temp copy of the verifier, and reports +SURVIVED / DIED. + +Exit 0 = every mutant is killed (the verifier is robust). +Exit 1 = at least one mutant survived (verifier too weak). +""" +from __future__ import annotations + +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +WORK = Path("/root/troupe-standup/wizard-work/forge") +SRC_FILES = [ + "verify_triage_dump_pending_refactor.py", + "_triage_dump_pending.py", + "_triage_dump_pending_refactored.py", +] + +mutants = [] +def mutant(label, fn): + mutants.append((label, fn)) + +# 1. Remove the __main__ guard -- main() called at import. +mutant("no_main_guard", lambda s: s.replace( + 'if __name__ == "__main__":\n sys.exit(main())', + 'sys.exit(main()) # guard-less mutant\n')) + +# 2. Accept path parameter but ignore it (always reads DEFAULT_PATH). +mutant("path_ignored", lambda s: s.replace( + ' d = load_data(path)', + ' d = load_data(DEFAULT_PATH) # path parameter ignored')) + +# 3. Skip the pending section entirely. +mutant("skip_pending", lambda s: s.replace( + ' _print_pending(d.get("pending", []))', + ' pass # pending mutant')) + +# 4. Stub the loaded data so output is fabricated, not from the file. +mutant("stub_data", lambda s: s.replace( + ' d = load_data(path)', + ' d = {"source": "STUB", "exported_at": "x",\n' + ' "decided": [], "pending": []} # mutant\n')) + +# 5. Remove the path parameter from main. +mutant("remove_path_param", lambda s: s.replace( + 'def main(path: str = DEFAULT_PATH) -> int:', + 'def main() -> int:')) + +# 6. Hardcode the source field instead of reading it from the data. +mutant("hardcode_source", lambda s: s.replace( + 'print("SOURCE:", repr(d.get("source")))', + 'print("SOURCE:", repr("STUB"))')) + +# 7. Inline the helper calls (removes the function-extraction smell). +mutant("inline_helpers", lambda s: s.replace( + ' _print_summary(d)\n' + ' _print_decided(d.get("decided", []))\n' + ' _print_pending(d.get("pending", []))', + ' # inlined\n _print_summary(d); _print_decided(d.get("decided", [])); ' + '_print_pending(d.get("pending", []))')) + + +def main() -> int: + tmp = Path(tempfile.mkdtemp(prefix="mutant_scan_triage_")) + try: + for f in SRC_FILES: + shutil.copy2(WORK / f, tmp / f) + ref = tmp / "_triage_dump_pending_refactored.py" + SRC = ref.read_text() + survivors = [] + for label, fn in mutants: + mutant_src = fn(SRC) + if mutant_src == SRC: + print(f" {label:20s} SKIP (no change)") + continue + ref.write_text(mutant_src) + p = subprocess.run( + "python3 verify_triage_dump_pending_refactor.py", + shell=True, cwd=tmp, + capture_output=True, text=True) + tail = (p.stdout + p.stderr).strip().splitlines() + tail = tail[-1] if tail else "(no output)" + died = p.returncode != 0 + print(f" {label:20s} {'DIED' if died else 'SURVIVED':9s} " + f"exit={p.returncode} {tail}") + if not died: + survivors.append(label) + print(f"\nSurvivors: {survivors if survivors else 'none'}") + return 1 if survivors else 0 + finally: + shutil.rmtree(tmp, ignore_errors=True) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/wizard-work/forge/verify_triage_dump_pending_refactor.py b/wizard-work/forge/verify_triage_dump_pending_refactor.py index aa6a37d..2689274 100644 --- a/wizard-work/forge/verify_triage_dump_pending_refactor.py +++ b/wizard-work/forge/verify_triage_dump_pending_refactor.py @@ -109,9 +109,15 @@ def check_identical_output() -> bool: def check_main_guard() -> bool: - content = _read(REFACTORED) - if '__name__ == "__main__"' not in content: - print("FAIL: no __main__ guard") + # Inspect *code*, not documentation: the refactored module's docstring + # names the `__name__ == "__main__"` guard it removes, so a raw-text check + # would false-positive (the string appears in the docstring even after the + # guard is deleted). `_code_only` strips the module docstring and all + # comment lines, leaving only executable source -- the same technique used + # by the probe_forge verifier for its smell-absence checks. + code = _code_only(_read(REFACTORED)) + if '__name__ == "__main__"' not in code: + print("FAIL: no __main__ guard in code (docstring-only matches do not count)") return False print("OK: __main__ guard present") return True @@ -127,17 +133,52 @@ def check_no_globals() -> bool: return True -def check_import_side_effect_free() -> bool: - """Importing the refactored module must not run the probe or print.""" +def _safe_import(): + """Import MOD_NAME from WORK_DIR, capturing stdout/stderr and any exception. + + Returns (module, exc, out, err). ``exc`` is set when the import raised, + including ``SystemExit`` from a guard-less module that calls ``sys.exit`` at + module scope. + + CENTRALISED GUARD: an unguarded ``importlib.import_module`` in + ``check_path_override_honoured`` used to let a ``SystemExit(0)`` propagate + and kill the verifier process with rc 0, masking the ``no_main_guard`` + mutant (the probe output leaked to real stdout and the verifier died before + reaching its final ``sys.exit(1)``). Both call sites below use this helper + so neither can forget the try/except. + """ sys.modules.pop(MOD_NAME, None) if WORK_DIR not in sys.path: sys.path.insert(0, WORK_DIR) out, err = io.StringIO(), io.StringIO() - with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): - importlib.import_module(MOD_NAME) - if out.getvalue() or err.getvalue(): - print(f"FAIL: import produced output: stdout={out.getvalue()!r} " - f"stderr={err.getvalue()!r}") + try: + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + mod = importlib.import_module(MOD_NAME) + except BaseException as exc: + return None, exc, out.getvalue(), err.getvalue() + return mod, None, out.getvalue(), err.getvalue() + + +def check_import_side_effect_free() -> bool: + """Importing the refactored module must not run the probe or print, and must + not terminate the process. + + A missing ``__main__`` guard that calls ``sys.exit`` at module scope surfaces + here as ``SystemExit`` during ``importlib.import_module``. Because + ``SystemExit(0)`` propagates and exits the verifier with code 0, it used to + let the ``no_main_guard`` mutant *appear* to pass -- the verifier died before + reaching its final ``sys.exit(1)``. ``_safe_import`` now catches every + ``BaseException`` (including ``SystemExit``) and treats it as a FAIL, so the + defect is reported instead of crashing the harness. + """ + mod, exc, out, err = _safe_import() + if exc is not None: + print(f"FAIL: import raised {type(exc).__name__}: {exc!r} " + f"(stdout={out!r} stderr={err!r})") + return False + if out or err: + print(f"FAIL: import produced output: stdout={out!r} " + f"stderr={err!r}") return False print("OK: import is side-effect-free (no probe output at import)") return True @@ -169,6 +210,33 @@ def check_helpers_extracted() -> bool: return True +def check_main_delegation_intact() -> bool: + """main() must delegate to the extracted helpers via distinct statements. + + This guards the function-extraction smell against a regression that + *collapses* the three delegation calls onto one semicolon-joined line (the + ``inline_helpers`` mutant). Collapsing the calls is behavior-preserving -- + output is identical and the helpers are still defined and still invoked -- + so output-comparison and even AST call-detection cannot catch it. We assert + main's body keeps one statement per line (no ``;``), preserving the + extraction boundary as genuine call boundaries, not smooshed text. + """ + tree = ast.parse(_read(REFACTORED)) + main_func = next(n for n in tree.body + if isinstance(n, ast.FunctionDef) and n.name == "main") + src_lines = _read(REFACTORED).splitlines() + main_lines = src_lines[main_func.lineno - 1: main_func.end_lineno + or main_func.lineno] + for ln in main_lines: + code = ln.split("#", 1)[0] # drop inline comment before testing + if ";" in code: + print(f"FAIL: main() body compacts statements with ';' " + f"({ln.strip()!r}); delegation calls must stay distinct") + return False + print("OK: main() delegates via distinct standalone statements (no ';'-compaction)") + return True + + def check_main_path_parameter() -> bool: """``main`` must accept a ``path`` parameter so the JSON location is configurable rather than hardcoded at module scope.""" @@ -233,10 +301,12 @@ def check_path_override_honoured() -> bool: with open(custom, "w", encoding="utf-8") as f: json.dump(data, f) try: - sys.modules.pop(MOD_NAME, None) - if WORK_DIR not in sys.path: - sys.path.insert(0, WORK_DIR) - mod = importlib.import_module(MOD_NAME) + mod, exc, _imp_out, _imp_err = _safe_import() + if exc is not None: + print(f"FAIL: import raised {type(exc).__name__}: {exc!r} " + f"(stdout={_imp_out!r} stderr={_imp_err!r})") + return False + assert mod is not None # _safe_import: exc set iff module is None out, err = io.StringIO(), io.StringIO() with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): rc = mod.main(path=custom) @@ -265,6 +335,7 @@ def main() -> None: check_no_globals, check_import_side_effect_free, check_helpers_extracted, + check_main_delegation_intact, check_main_path_parameter, check_original_still_smells, check_path_override_honoured,