From d0a11eafd2af54b6a3567fc4c9b512c356cafea0 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 24 Aug 2026 15:58:56 -0700 Subject: [PATCH 1/2] Resolve Symlinks Before Accepting a Canonical File CodeRabbit finding on the develop -> main promotion PR (#978), against PR #977's earlier is_file() checks: a tracked in-repo symlink whose target escapes ROOT would pass both escapes_repo_root() (a lexical string check) and a bare Path.is_file() (which follows the link) the same way a real file would. The audit engine would then read that external target's live filesystem content while dating it from the symlink's own git history, a mismatch between what was verified and what was read. Add canonical_file_in_root(), which resolves the candidate strictly and requires the result to both exist as a file and stay under ROOT, and use it in place of the two is_file() calls PR #977 added. Verified by hand: symlinked a path inside this checkout to a file outside it, confirmed the new check rejects it while still accepting a real hub file and still rejecting a missing path and a directory, removed the symlink after. --- spec/validate.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/spec/validate.py b/spec/validate.py index 4f6c20e2..ecf688f4 100755 --- a/spec/validate.py +++ b/spec/validate.py @@ -71,6 +71,20 @@ def escapes_repo_root(value): ) +def canonical_file_in_root(rel_path): + """Whether `ROOT / rel_path` resolves, symlinks followed, to an existing file that stays + under ROOT. `escapes_repo_root()` only reads the lexical string, so a tracked symlink whose + target escapes ROOT would otherwise pass it and then `Path.is_file()` too, since both follow + the link. The audit engine would then read that external target's live content while dating + it from the symlink's own git history. + """ + try: + resolved = (ROOT / rel_path).resolve(strict=True) + except OSError: + return False + return resolved.is_file() and resolved.is_relative_to(ROOT) + + def description_errors_for_repo(repo, name): """The per-repo optional-field guard: an explicit `"description": null` is declared-but-invalid, not absent. @@ -816,7 +830,7 @@ def check_selector(where, applies_to): elif isinstance(ref, str): if escapes_repo_root(ref): errors.append(f"files.json: {path} reference '{ref}' must be a repo-relative path") - elif fid == "intent" and not (ROOT / ref).is_file(): + elif fid == "intent" and not canonical_file_in_root(ref): # This field outranks intentRef in the audit engine's canonical resolution. # An intent unit's reference needs the same existing-file check intentRef gets below. errors.append( @@ -840,7 +854,7 @@ def check_selector(where, applies_to): errors.append( f"files.json: {path} intentRef '{intent_ref}' must be a repo-relative path" ) - elif not (ROOT / intent_path).is_file(): + elif not canonical_file_in_root(intent_path): # A directory such as "." exists but is not a file. # The staleness check would then read the whole repo's most recent commit as this one file's, false-flagging every intent unit as stale. errors.append( From eecc19e8fa3297d4baa11e7acfc3e9eeea97591c Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 24 Aug 2026 16:07:57 -0700 Subject: [PATCH 2/2] Reformat canonical_file_in_root()'s Docstring to One Sentence Per Line qodo finding on PR #979: the new docstring wrapped multi-sentence prose across lines, the exact comment-structure violation caught and fixed once already on PR #977's own intent_canonical_rel(). Tightened to a one-line summary, blank line, then a single-sentence rationale, each on its own line. --- spec/validate.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/spec/validate.py b/spec/validate.py index ecf688f4..f2580f0d 100755 --- a/spec/validate.py +++ b/spec/validate.py @@ -72,11 +72,9 @@ def escapes_repo_root(value): def canonical_file_in_root(rel_path): - """Whether `ROOT / rel_path` resolves, symlinks followed, to an existing file that stays - under ROOT. `escapes_repo_root()` only reads the lexical string, so a tracked symlink whose - target escapes ROOT would otherwise pass it and then `Path.is_file()` too, since both follow - the link. The audit engine would then read that external target's live content while dating - it from the symlink's own git history. + """Whether `ROOT / rel_path` resolves, symlinks followed, to an existing file under ROOT. + + A tracked symlink whose target escapes ROOT passes a bare `Path.is_file()` the same way a real file would, since both follow the link. """ try: resolved = (ROOT / rel_path).resolve(strict=True)