Skip to content

Commit 5b666c6

Browse files
PhysShellclaude
andcommitted
fix(S2): step 9 — the three narrow HOLD defects
1. THE HARNESS GUARD WAS TOO LOOSE. test_harness_contract.py skipped ANY top-level `if` (so `if True: sys.exit(0)` slid through) and broke traversal at the first nested FunctionDef (so a module-scope exit inside a top-level `try` was missed). The guard is rewritten as a proper recursive walk: it skips ONLY a structurally-recognised `__name__ == "__main__"` (a real ast.Compare of __name__ == "__main__"), does NOT descend into function / class / lambda scopes, and DOES descend through module-scope compound statements (if / try / with / for / while) — so an exit inside `if True:` or a top-level `try:` is caught. Added 8 guard self-tests: five that MUST be caught (bare exit, `if True:` exit, exit in a top-level try, `raise SystemExit`, exit in a for loop) and three that MUST pass (the guarded entrypoint, an exit inside a function, an exit inside a lambda). 2. NO-NEWLINE STATE WAS PER-HUNK. old_closed/new_closed reset each hunk, so a no-newline marker in the first hunk did not forbid a second hunk after it — the marker means EOF-without-newline, so nothing can follow on that side. State is now FILE-level (`saw_marker`): once any no-newline marker appears, a following hunk header is a PATCH_STRUCTURE refusal (previously it would have slipped to APPLY_CHECK — the right refusal via the wrong branch). Added a two-hunk fixture asserting PATCH_STRUCTURE. 3. CLAIM/PUBLICATION EQUALITY WAS NOT PLATFORM-AWARE, AND A POST-mkdir FAILURE LEAKED THE DIRECTORY. Both the workdir self-resolution proof and the publication parent re-proof used a raw string `realpath(x) != y`, which on case-insensitive Windows disagrees with the accepted _same_or_inside rule and could give a casing-only false refusal. A new platform-aware `_same_path` (normcase+normpath, the same rule) is now used in both. And `_claim_workdir` created the directory before the outer cleanup scope began, so a proof failure after the mkdir would strand it — the post-mkdir proofs are now wrapped so the just-created directory is removed on ANY failure before raising. Added a pure-function regression (symlink-capable platforms): a claim under a symlinked parent fails the self-resolution proof with PUBLICATION and leaves NO directory behind, plus _same_path platform cases. Nothing else in Step 9 reopened. Steps 10-12 not started. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JxKjqdGEFzq4UzZupw379G
1 parent ea47d45 commit 5b666c6

3 files changed

Lines changed: 132 additions & 29 deletions

File tree

‎ownlang/fix_gate.py‎

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,23 @@ def _canonical_bytes(obj: Any) -> bytes:
9898
return _canonical_json(obj) + b"\n"
9999

100100

101+
def _norm(path: str) -> str:
102+
return os.path.normcase(os.path.normpath(path))
103+
104+
105+
def _same_path(a: str, b: str) -> bool:
106+
"""Platform-aware path equality — the SAME rule as _same_or_inside (normcase is
107+
identity on POSIX, case-fold on Windows), so `C:\\R\\x` and `c:\\r\\x` compare equal on
108+
Windows. A raw string `==` would give a casing-only false refusal."""
109+
return _norm(a) == _norm(b)
110+
111+
101112
def _same_or_inside(parent: str, path: str) -> bool:
102113
"""Is `path` the directory `parent` itself, or under it? Both must be physical.
103114
Case sensitivity is a PLATFORM property (normcase is identity on POSIX), so `C:\\Repo`
104115
and `c:\\repo` are one directory on Windows and two elsewhere."""
105-
p = os.path.normcase(os.path.normpath(parent))
106-
c = os.path.normcase(os.path.normpath(path))
116+
p = _norm(parent)
117+
c = _norm(path)
107118
if c == p:
108119
return True
109120
return c.startswith(p if p.endswith(os.sep) else p + os.sep)
@@ -572,10 +583,13 @@ def parse_step8_patch(patch: bytes, rel: str, preimage: bytes) -> None:
572583
i = 3
573584
prev_old_end = 0
574585
saw_hunk = False
575-
while i < len(recs):
586+
saw_marker = False # FILE-level: a no-newline marker means EOF-without-newline, so no
587+
while i < len(recs): # further hunk may follow it — the marked line is the file's last.
576588
rec = recs[i]
577589
if not (rec.startswith(b"@@ -") and rec.endswith(b" @@")):
578590
raise GateError(PATCH_STRUCTURE, f"patch: expected a hunk header, got {rec[:40]!r}")
591+
if saw_marker:
592+
raise GateError(PATCH_STRUCTURE, "patch: a hunk after a no-newline-at-EOF marker")
579593
body = rec[len(b"@@ -"):-len(b" @@")]
580594
try:
581595
old_part, new_part = body.split(b" +", 1)
@@ -614,6 +628,7 @@ def parse_step8_patch(patch: bytes, rel: str, preimage: bytes) -> None:
614628
raise GateError(PATCH_STRUCTURE, "patch: duplicate no-newline marker")
615629
old_closed = old_closed or marks_old
616630
new_closed = new_closed or marks_new
631+
saw_marker = True
617632
last_head = None
618633
i += 1
619634
continue
@@ -827,8 +842,11 @@ def _out_parent(out: str, root: str) -> tuple[str, str, str]:
827842
def _claim_workdir(parent_phys: str) -> str:
828843
"""CLAIM an unpredictable working directory: create it here and now (owner-only on
829844
POSIX, as part of the mkdir), then PROVE we own it — not a link/reparse, resolving to
830-
itself under a platform-aware comparison, and empty. A name that is merely checked and
831-
then written into is a window; this closes it."""
845+
itself under a PLATFORM-AWARE comparison, and empty. A name that is merely checked and
846+
then written into is a window; this closes it. If any proof AFTER the mkdir fails, the
847+
directory we just created is removed before raising — no leftover."""
848+
import shutil
849+
832850
for _ in range(8):
833851
path = os.path.join(parent_phys, f".owen-gate-{os.urandom(16).hex()}")
834852
if os.path.exists(path) or os.path.islink(path):
@@ -840,14 +858,19 @@ def _claim_workdir(parent_phys: str) -> str:
840858
except OSError as exc:
841859
raise GateError(PUBLICATION,
842860
f"cannot claim a work directory ({exc.strerror or exc})") from exc
843-
lst = os.lstat(path)
844-
if _is_link(lst):
845-
raise GateError(PUBLICATION, "the claimed work directory is a link")
846-
if not stat.S_ISDIR(lst.st_mode) or os.path.realpath(path) != path \
847-
or not _same_or_inside(parent_phys, os.path.realpath(path)):
848-
raise GateError(PUBLICATION, "the claimed work directory does not resolve to itself")
849-
if any(os.scandir(path)):
850-
raise GateError(PUBLICATION, "the claimed work directory is not empty")
861+
try:
862+
lst = os.lstat(path)
863+
if _is_link(lst):
864+
raise GateError(PUBLICATION, "the claimed work directory is a link")
865+
if not stat.S_ISDIR(lst.st_mode) or not _same_path(os.path.realpath(path), path) \
866+
or not _same_or_inside(parent_phys, os.path.realpath(path)):
867+
raise GateError(PUBLICATION,
868+
"the claimed work directory does not resolve to itself")
869+
if any(os.scandir(path)):
870+
raise GateError(PUBLICATION, "the claimed work directory is not empty")
871+
except BaseException:
872+
shutil.rmtree(path, ignore_errors=True)
873+
raise
851874
return path
852875
raise GateError(PUBLICATION, "could not claim a work directory")
853876

@@ -860,7 +883,7 @@ def _publish(staging: str, out_phys: str, evidence: bytes, root_phys: str) -> No
860883
parent = os.path.dirname(out_phys)
861884
if not os.path.isdir(parent):
862885
raise GateError(PUBLICATION, "the out-dir parent vanished before publication")
863-
if os.path.realpath(parent) != os.path.dirname(out_phys) \
886+
if not _same_path(os.path.realpath(parent), os.path.dirname(out_phys)) \
864887
or _same_or_inside(root_phys, os.path.realpath(parent)):
865888
raise GateError(PUBLICATION, "the out-dir parent changed to resolve inside the root")
866889
if os.path.exists(out_phys) or os.path.islink(out_phys):

‎tests/test_gate_patch.py‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313

1414
import hashlib
1515
import os
16+
import shutil
1617
import sys
18+
import tempfile
1719
from typing import Any
1820

1921
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
@@ -24,7 +26,9 @@
2426
PATCH_STRUCTURE,
2527
GateError,
2628
_bundle_sha256,
29+
_claim_workdir,
2730
_same_or_inside,
31+
_same_path,
2832
parse_step8_patch,
2933
validate_gate_authority,
3034
validate_manifest_shape,
@@ -381,10 +385,16 @@ def small(hunk: bytes) -> bytes:
381385
small(b"@@ -1,1 +1,1 @@\n-a\n\\ No newline at end of file\n"
382386
b"\\ No newline at end of file\n+z\n"), X, PRE3),
383387
PATCH_STRUCTURE, "patch: a duplicate no-newline marker")
384-
# A marker on a line that is NOT the last of its side.
388+
# A marker on a line that is NOT the last of its side (within the hunk).
385389
refuses(lambda: parse_step8_patch(
386390
small(b"@@ -1,2 +1,1 @@\n-a\n\\ No newline at end of file\n-b\n+z\n"), X, PRE3),
387391
PATCH_STRUCTURE, "patch: marker with more of the same side after it")
392+
# A marker in an EARLIER hunk: the no-newline is EOF, so no later hunk may follow it. This
393+
# is the file-level invariant (a per-hunk reset would wrongly accept it).
394+
refuses(lambda: parse_step8_patch(
395+
small(b"@@ -1,1 +1,1 @@\n-a\n\\ No newline at end of file\n+x\n"
396+
b"@@ -3,1 +3,1 @@\n-c\n+z\n"), X, PRE3),
397+
PATCH_STRUCTURE, "patch: a second hunk after a no-newline marker")
388398
# A zero-length (insertion) range whose start is past the preimage.
389399
refuses(lambda: parse_step8_patch(small(b"@@ -99,0 +4,1 @@\n+z\n"), X, PRE3),
390400
PATCH_STRUCTURE, "patch: an insertion range past the preimage")
@@ -396,6 +406,35 @@ def small(hunk: bytes) -> bytes:
396406
check(True, "patch: a pure insertion at the top parses")
397407

398408

409+
# --- claim cleanup + platform-aware path equality -----------------------------------
410+
411+
check(_same_path("/a/b", "/a/b"), "same_path: identical")
412+
check(not _same_path("/a/b", "/a/c"), "same_path: different")
413+
if os.name == "nt":
414+
check(_same_path("C:\\A\\B", "c:\\a\\b"), "same_path: Windows case-insensitive")
415+
else:
416+
check(not _same_path("/a/B", "/a/b"), "same_path: POSIX case-sensitive")
417+
418+
# A claim whose self-resolution proof fails (a symlinked parent, so realpath(path) != path)
419+
# must REMOVE the directory it just created — no leftover. Symlink-capable platforms only.
420+
_tmp = tempfile.mkdtemp()
421+
_real = os.path.join(_tmp, "real")
422+
_link = os.path.join(_tmp, "link")
423+
os.mkdir(_real)
424+
try:
425+
os.symlink(_real, _link, target_is_directory=True)
426+
_have_symlink = True
427+
except (OSError, NotImplementedError):
428+
_have_symlink = False
429+
if _have_symlink:
430+
_before = set(os.listdir(_real))
431+
refuses(lambda: _claim_workdir(_link), "PUBLICATION",
432+
"claim: a non-self-resolving parent is refused")
433+
check(set(os.listdir(_real)) == _before,
434+
"claim: a failed proof leaves no directory behind")
435+
shutil.rmtree(_tmp, ignore_errors=True)
436+
437+
399438
# --- containment platform rule -----------------------------------------------------
400439

401440
check(_same_or_inside("/repo", "/repo"), "containment: root contains itself")

‎tests/test_harness_contract.py‎

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,35 @@
1818
_HERE = os.path.dirname(os.path.abspath(__file__))
1919

2020

21-
def _module_scope_exit(tree: ast.AST) -> bool:
22-
"""True if a sys.exit(...) / raise SystemExit(...) sits at module scope (i.e. would
23-
fire on import) rather than inside a function or an `if __name__ == '__main__'` guard."""
24-
for node in ast.iter_child_nodes(tree):
25-
if isinstance(node, ast.If):
26-
continue # the `if __name__ == "__main__"` entrypoint is fine
27-
for sub in ast.walk(node):
28-
if isinstance(sub, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
29-
# do not descend into nested scopes for THIS module-scope check
30-
break
31-
if isinstance(sub, ast.Raise) and _is_systemexit(sub.exc):
32-
return True
33-
if isinstance(sub, ast.Call) and _is_sys_exit(sub.func):
34-
return True
21+
def _is_main_guard(test: ast.expr) -> bool:
22+
"""Structurally recognise EXACTLY `__name__ == "__main__"` — not any top-level `if`,
23+
so `if True: sys.exit(0)` is NOT waved through."""
24+
return (isinstance(test, ast.Compare)
25+
and isinstance(test.left, ast.Name) and test.left.id == "__name__"
26+
and len(test.ops) == 1 and isinstance(test.ops[0], ast.Eq)
27+
and len(test.comparators) == 1
28+
and isinstance(test.comparators[0], ast.Constant)
29+
and test.comparators[0].value == "__main__")
30+
31+
32+
def _module_scope_exit(node: ast.AST) -> bool:
33+
"""True if a sys.exit(...) / raise SystemExit(...) would fire at IMPORT time. Descends
34+
through module-scope compound statements (if / try / with / for / while) but NOT into
35+
a new scope (function / class / lambda) and NOT into the `if __name__ == "__main__"`
36+
entrypoint — so an exit inside `if True:` or a top-level `try:` IS caught, while the
37+
real guard and any function body are not."""
38+
for child in ast.iter_child_nodes(node):
39+
if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef,
40+
ast.Lambda, ast.ClassDef)):
41+
continue # a new scope: its body does not run on import
42+
if isinstance(child, ast.If) and _is_main_guard(child.test):
43+
continue # the sanctioned entrypoint
44+
if isinstance(child, ast.Raise) and _is_systemexit(child.exc):
45+
return True
46+
if isinstance(child, ast.Call) and _is_sys_exit(child.func):
47+
return True
48+
if _module_scope_exit(child):
49+
return True
3550
return False
3651

3752

@@ -46,8 +61,34 @@ def _is_sys_exit(func: ast.expr) -> bool:
4661
and isinstance(func.value, ast.Name) and func.value.id == "sys")
4762

4863

64+
_SELFTEST_MUST_CATCH = (
65+
("bare sys.exit at module scope", "import sys\nsys.exit(0)\n"),
66+
("exit inside `if True:`", "import sys\nif True:\n sys.exit(0)\n"),
67+
("exit inside a top-level try",
68+
"import sys\ntry:\n sys.exit(0)\nexcept Exception:\n pass\n"),
69+
("raise SystemExit at module scope", "raise SystemExit(1)\n"),
70+
("exit inside a for loop", "import sys\nfor _ in range(1):\n sys.exit(0)\n"),
71+
)
72+
_SELFTEST_MUST_PASS = (
73+
("guarded entrypoint", "import sys\nif __name__ == '__main__':\n sys.exit(0)\n"),
74+
("exit inside a function", "import sys\ndef run():\n sys.exit(0)\n"),
75+
("exit inside a lambda", "f = lambda: __import__('sys').exit(0)\n"),
76+
)
77+
78+
4979
def run() -> int:
5080
global checks
81+
# Self-test the guard first: it must catch the ways the original bug could recur, and
82+
# must NOT flag the sanctioned entrypoint or an exit that only lives inside a scope.
83+
for label, src in _SELFTEST_MUST_CATCH:
84+
checks += 1
85+
if not _module_scope_exit(ast.parse(src)):
86+
failures.append(f"guard self-test: failed to catch {label}")
87+
for label, src in _SELFTEST_MUST_PASS:
88+
checks += 1
89+
if _module_scope_exit(ast.parse(src)):
90+
failures.append(f"guard self-test: wrongly flagged {label}")
91+
5192
for fname in sorted(os.listdir(_HERE)):
5293
if not (fname.startswith("test_") and fname.endswith(".py")):
5394
continue

0 commit comments

Comments
 (0)