From 363dab253350d4f7a74e008039c486726e54d694 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 24 Aug 2026 14:22:06 -0700 Subject: [PATCH 1/6] Fix Intent-Staleness Check to Read the Manifest's intentRef check_intent_staleness compared six intent-fidelity entries (.editorconfig, .editorconfig-checker.json, .gitattributes, version.json, AUDIT.md, spec/secrets.json) against the hub file sharing their own path, because it only read the manifest's `reference` field, which none of the six set. Their entries declare `intentRef` instead, naming a different canonical, so the comparison ran against the wrong hub file in both directions: a false positive (AUDIT.md's fleet-wide procedure changing flagged a downstream repo's unrelated self-audit doc) and a silent false negative (docs/repo-config.md changing raised nothing, since no entry declared it as anyone's canonical). Add intent_canonical_rel(item, path): reference, then intentRef, then path, stripping an intentRef's #anchor since it names a section for a reader rather than a narrower file to diff against. Covered by four cases in _selftest(). Fixes #726 --- spec/audit.py | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/spec/audit.py b/spec/audit.py index 6101d71a..1f0045c1 100755 --- a/spec/audit.py +++ b/spec/audit.py @@ -1675,6 +1675,16 @@ def hub_last_change(rel_path): return date, sha +def intent_canonical_rel(item, path): + """The hub path an intent unit's copy is judged against for staleness: `reference` if the + manifest sets one, else the intent unit's own canonical, `intentRef`, else `path` itself. + `reference` never carries an anchor, but `intentRef` routes a reader to one section of a + larger doc, so an anchor there names a place to read rather than a narrower file to diff + against, and is stripped - the whole canonical is a wrong-but-safe over-approximation, where + leaving it in would make the path unreadable and silently fall back to `path` (#726).""" + return (item.get("reference") or item.get("intentRef") or path).split("#", 1)[0] + + def check_intent_staleness(slug, ground, path, canonical_rel, down_text): """The intent-staleness advisory: a last-modified comparison, since intent has no content check. @@ -2109,7 +2119,7 @@ def audit_repo(entry, spec, branch=None): # The hub's own copies are the canonicals, so the hub itself has nothing to trail. elif item is not None and fid == "intent" and entry.get("name") != HUB_NAME: findings.extend( - check_intent_staleness(slug, ground, path, item.get("reference") or path, text) + check_intent_staleness(slug, ground, path, intent_canonical_rel(item, path), text) ) # Heading-based presence is only meaningful for Markdown. # A "section" named on a non-md file, a tasks.json task group being one, is an intent marker judged per AUDIT.md rather than a heading grep. @@ -4667,6 +4677,41 @@ def _selftest(): finally: globals()["owner_repos"] = real_owner_repos + # intent_canonical_rel: an intentRef with an anchor resolves to the whole hub file, not the + # anchor-qualified name git cannot look up, and reference still wins where the manifest sets both (#726). + canonical_cases = [ + ( + "no reference or intentRef falls back to the file's own path", + {}, + "AGENTS.md", + "AGENTS.md", + ), + ( + "an intentRef equal to the path is itself the canonical", + {"intentRef": "GOVERNANCE.md"}, + "GOVERNANCE.md", + "GOVERNANCE.md", + ), + ( + "an anchored intentRef strips the anchor and keeps the whole file", + {"intentRef": "GOVERNANCE.md#line-endings"}, + ".editorconfig", + "GOVERNANCE.md", + ), + ( + "reference wins over intentRef when the manifest sets both", + {"reference": "catalog/snippets/configs/codecov.yml", "intentRef": "WORKFLOW.md"}, + "codecov.yml", + "catalog/snippets/configs/codecov.yml", + ), + ] + for label, item, path, want in canonical_cases: + got = intent_canonical_rel(item, path) + good = got == want + if not good: + ok = False + print(f" {'ok ' if good else 'FAIL'} intent_canonical_rel: {label} -> {got!r}") + print("SELFTEST PASS" if ok else "SELFTEST FAIL") return 0 if ok else 1 From 25a7ef25de03926167c2f6d0a9d3131291707304 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 24 Aug 2026 14:41:12 -0700 Subject: [PATCH 2/6] Restrict Anchor Stripping to intentRef and Type-Check It Review findings on PR #977: - intent_canonical_rel() stripped a `#anchor` from whatever value it picked, so a `reference` or bare `path` legitimately containing a literal `#` would be truncated too, though only `intentRef` is ever meant to carry one. Restructure the resolution as reference, then intentRef, then path, and strip only in the intentRef branch. - A non-string `intentRef` (a malformed spec/files.json entry) would crash `.split()` and abort the whole audit run; `reference` gets the same string-type check in spec/validate.py already, `intentRef` did not. Add the matching check there, so the audit engine can trust the type the same way it already trusts `reference`. Verified by hand: an injected non-string intentRef is caught, reverted after. - Reformatted the new docstring to one sentence per line (it was wrapped mid-sentence) and dropped a spaced-hyphen dash and two current-task references, none of which belong in carried prose. Two new _selftest() cases cover the literal-'#' fix. Declined, with evidence in-thread: full type annotations on this function (spec/ is the lint-only Scripts profile, not the strict src/ layout the cited rule targets, and 70 of this file's other 71 functions carry none) and the PR title's casing (its "to" is an explicitly listed lowercase bind word, and "intentRef" is a manifest field name kept in its own casing, matching existing PR-title precedent for identifiers). --- spec/audit.py | 37 +++++++++++++++++++++++++++++-------- spec/validate.py | 5 +++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/spec/audit.py b/spec/audit.py index 1f0045c1..ab8aa1b0 100755 --- a/spec/audit.py +++ b/spec/audit.py @@ -1676,13 +1676,22 @@ def hub_last_change(rel_path): def intent_canonical_rel(item, path): - """The hub path an intent unit's copy is judged against for staleness: `reference` if the - manifest sets one, else the intent unit's own canonical, `intentRef`, else `path` itself. - `reference` never carries an anchor, but `intentRef` routes a reader to one section of a - larger doc, so an anchor there names a place to read rather than a narrower file to diff - against, and is stripped - the whole canonical is a wrong-but-safe over-approximation, where - leaving it in would make the path unreadable and silently fall back to `path` (#726).""" - return (item.get("reference") or item.get("intentRef") or path).split("#", 1)[0] + """The hub path an intent unit's copy is judged against for staleness. + + `reference` wins where the manifest sets one. + Otherwise the intent unit's own canonical, `intentRef`, wins. + Otherwise the unit compares against its own `path`. + Only `intentRef` ever carries a `#anchor`, routing a reader to one section of a larger doc. + The anchor names a place to read, not a narrower file to diff against, so it is stripped + there and nowhere else, comparing the whole canonical file instead. + """ + ref = item.get("reference") + if ref: + return ref + intent = item.get("intentRef") + if intent: + return intent.split("#", 1)[0] + return path def check_intent_staleness(slug, ground, path, canonical_rel, down_text): @@ -4678,7 +4687,7 @@ def _selftest(): globals()["owner_repos"] = real_owner_repos # intent_canonical_rel: an intentRef with an anchor resolves to the whole hub file, not the - # anchor-qualified name git cannot look up, and reference still wins where the manifest sets both (#726). + # anchor-qualified name git cannot look up, and reference still wins where the manifest sets both. canonical_cases = [ ( "no reference or intentRef falls back to the file's own path", @@ -4704,6 +4713,18 @@ def _selftest(): "codecov.yml", "catalog/snippets/configs/codecov.yml", ), + ( + "a literal '#' in reference is not mistaken for an anchor and stripped", + {"reference": "docs/notes#1.md"}, + "notes.md", + "docs/notes#1.md", + ), + ( + "a literal '#' in path is not mistaken for an anchor and stripped", + {}, + "docs/notes#1.md", + "docs/notes#1.md", + ), ] for label, item, path, want in canonical_cases: got = intent_canonical_rel(item, path) diff --git a/spec/validate.py b/spec/validate.py index 83185e7b..b21655ca 100755 --- a/spec/validate.py +++ b/spec/validate.py @@ -811,6 +811,11 @@ def check_selector(where, applies_to): f"files.json: {path} fidelity 'verbatim' but its canonical source {src} is missing" ) + # The audit engine's intent_canonical_rel() trusts this is a string once validated, the same way it trusts reference above. + intent_ref = item.get("intentRef") + if intent_ref is not None and not isinstance(intent_ref, str): + errors.append(f"files.json: {path} intentRef must be a string") + sections = item.get("sections", []) if not isinstance(sections, list): errors.append(f"files.json: {path} sections must be an array") From 260328101b1479d7bcdaee31c5e61d8cd9397959 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 24 Aug 2026 14:52:09 -0700 Subject: [PATCH 3/6] Constrain intentRef to a Repo-Relative Path, Fix a Test Label CodeRabbit findings on PR #977's fix commit: - The new intentRef string check accepted any string, including `../../outside` or `/etc/passwd`, which intent_canonical_rel() then joins with ROOT unconfined, the same escape `reference` is already guarded against a few lines above. Add the matching repo-relative check, on the fragment-stripped path since that is the part the audit engine actually reads. Verified by hand: an injected `../../etc/passwd` intentRef is now caught, reverted after. - Two new _selftest() case labels said a literal '#' is "stripped" when the case actually asserts the opposite, that it survives untouched. Reworded to say what the case checks. --- spec/audit.py | 4 ++-- spec/validate.py | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/spec/audit.py b/spec/audit.py index ab8aa1b0..63d64648 100755 --- a/spec/audit.py +++ b/spec/audit.py @@ -4714,13 +4714,13 @@ def _selftest(): "catalog/snippets/configs/codecov.yml", ), ( - "a literal '#' in reference is not mistaken for an anchor and stripped", + "a literal '#' in reference is preserved rather than treated as an anchor", {"reference": "docs/notes#1.md"}, "notes.md", "docs/notes#1.md", ), ( - "a literal '#' in path is not mistaken for an anchor and stripped", + "a literal '#' in path is preserved rather than treated as an anchor", {}, "docs/notes#1.md", "docs/notes#1.md", diff --git a/spec/validate.py b/spec/validate.py index b21655ca..dc4c93d8 100755 --- a/spec/validate.py +++ b/spec/validate.py @@ -815,6 +815,17 @@ def check_selector(where, applies_to): intent_ref = item.get("intentRef") if intent_ref is not None and not isinstance(intent_ref, str): errors.append(f"files.json: {path} intentRef must be a string") + elif isinstance(intent_ref, str): + # The audit engine strips a trailing #anchor before ever joining this with ROOT, so validate the same part it will actually read. + intent_path = intent_ref.split("#", 1)[0] + if ( + not intent_path + or intent_path.startswith("/") + or ".." in pathlib.PurePosixPath(intent_path).parts + ): + errors.append( + f"files.json: {path} intentRef '{intent_ref}' must be a repo-relative path" + ) sections = item.get("sections", []) if not isinstance(sections, list): From aefed17c2a24182edb01c8f6ff39ef8ebf4f64a9 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 24 Aug 2026 15:00:58 -0700 Subject: [PATCH 4/6] Reject an intentRef That Does Not Resolve to a Real File CodeRabbit finding on PR #977's fix commit: a shape-valid but unresolved intentRef, a missing file or a directory such as ".", passed every check added so far and reached check_intent_staleness(). A missing canonical produces no finding at all: git log on a never-tracked path returns nothing, so hub_last_change() reads it as untracked and the check silently no-ops. A directory produces the opposite failure: `git log -- .` matches every commit in the repo, so hub_last_change(".") returns the single most recent commit anywhere, which reads as newer than any real file's own history, false-flagging every intent unit on that entry as stale. Verified both by hand: `git log -1 -- .` on this checkout returned this branch's own latest commit, and `git log -1 -- missing.md` returned nothing. Require the fragment-stripped intentRef to name an existing file. Verified by hand: injected intentRef values of "missing.md" and "." into files.json, confirmed validate.py now rejects both, reverted. --- spec/validate.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/validate.py b/spec/validate.py index dc4c93d8..b29043b0 100755 --- a/spec/validate.py +++ b/spec/validate.py @@ -826,6 +826,12 @@ 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(): + # 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( + f"files.json: {path} intentRef '{intent_ref}' canonical {intent_path} is not a file in this checkout" + ) sections = item.get("sections", []) if not isinstance(sections, list): From ba9a071caee4a0a8c1641fd0b56a05118ee7ff49 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 24 Aug 2026 15:15:02 -0700 Subject: [PATCH 5/6] Reject Windows-Style Path Escapes in reference and intentRef CodeRabbit finding on PR #977's fix commit: PurePosixPath reads a backslash as an ordinary filename character, so a value such as `..\outside.md` passes the ".." check that assumes POSIX separators. On a host where ROOT is a WindowsPath, `ROOT / value` parses that same string with native separators and can resolve outside ROOT. A bare Windows drive-absolute path (`C:/outside.md`, no backslash at all) escapes the leading-`/` check the same way. Extract escapes_repo_root(), covering POSIX `..` and a leading `/` (what the reference check already had), plus a backslash and a drive letter (what it was missing), and use it for both `reference` and the identical, adjacent gap this PR's own new `intentRef` check has. Verified: unit-checked the new function against nine escape and non-escape strings directly (OS-independent, since it is a string check rather than an actual path resolution), then end-to-end against files.json with an injected `..\..\outside.md` intentRef, confirmed validate.py rejects it, reverted. --- spec/validate.py | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/spec/validate.py b/spec/validate.py index b29043b0..7826fd7f 100755 --- a/spec/validate.py +++ b/spec/validate.py @@ -56,6 +56,21 @@ def is_str_list(v): return isinstance(v, list) and all(isinstance(x, str) for x in v) +def escapes_repo_root(value): + """Whether `ROOT / value` could resolve outside ROOT on some host `PurePosixPath` alone + misses: a POSIX `..` segment, a leading `/`, a backslash (Windows treats it as a separator + even though POSIX reads the whole thing as one filename), or a Windows drive letter such as + `C:`. + """ + return ( + not value + or value.startswith("/") + or "\\" in value + or re.match(r"^[A-Za-z]:", value) is not None + or ".." in pathlib.PurePosixPath(value).parts + ) + + def description_errors_for_repo(repo, name): """The per-repo optional-field guard: an explicit `"description": null` is declared-but-invalid, not absent. @@ -798,12 +813,8 @@ def check_selector(where, applies_to): if ref is not None and not isinstance(ref, str): errors.append(f"files.json: {path} reference must be a string") ref = None - elif isinstance(ref, str) and ( - ref.startswith("/") or ".." in pathlib.PurePosixPath(ref).parts - ): - errors.append( - f"files.json: {path} reference '{ref}' must be a repo-relative path (no leading / or ..)" - ) + elif isinstance(ref, str) and escapes_repo_root(ref): + errors.append(f"files.json: {path} reference '{ref}' must be a repo-relative path") if fid == "verbatim": src = ref if isinstance(ref, str) else path if isinstance(src, str) and not (ROOT / src).exists(): @@ -818,11 +829,7 @@ def check_selector(where, applies_to): elif isinstance(intent_ref, str): # The audit engine strips a trailing #anchor before ever joining this with ROOT, so validate the same part it will actually read. intent_path = intent_ref.split("#", 1)[0] - if ( - not intent_path - or intent_path.startswith("/") - or ".." in pathlib.PurePosixPath(intent_path).parts - ): + if escapes_repo_root(intent_path): errors.append( f"files.json: {path} intentRef '{intent_ref}' must be a repo-relative path" ) From f3e67d7ae4622fb6ea25cc3f5cd8059adac383c9 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 24 Aug 2026 15:21:17 -0700 Subject: [PATCH 6/6] Require an Intent Unit's reference to Exist Too CodeRabbit finding on PR #977's fix commit: reference outranks intentRef in intent_canonical_rel(), so an intent-fidelity item that sets reference (codecov.yml does today) skipped the existing-file check just added for intentRef entirely, the existing-file check below it only ever runs for the losing field. A missing or directory-shaped reference on such an item reaches check_intent_staleness() exactly as unverified as an unchecked intentRef did. Add the same is_file() check, gated on fid == "intent" so a non-intent unit's reference (a verbatim or interface unit's, checked separately by its own existing rule) is unaffected. Verified: the real files.json still validates clean (codecov.yml's reference resolves), then injected a missing reference on it, confirmed validate.py now rejects it, reverted. --- spec/validate.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/spec/validate.py b/spec/validate.py index 7826fd7f..4f6c20e2 100755 --- a/spec/validate.py +++ b/spec/validate.py @@ -813,8 +813,15 @@ def check_selector(where, applies_to): if ref is not None and not isinstance(ref, str): errors.append(f"files.json: {path} reference must be a string") ref = None - elif isinstance(ref, str) and escapes_repo_root(ref): - errors.append(f"files.json: {path} reference '{ref}' must be a repo-relative path") + 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(): + # 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( + f"files.json: {path} reference '{ref}' is not a file in this checkout" + ) if fid == "verbatim": src = ref if isinstance(ref, str) else path if isinstance(src, str) and not (ROOT / src).exists():