feat(ci): fail on line citations past the end of the cited file

[kyle-sexton]

No related issue: measured in the LlgiOCmFG_w re-vet follow-up (stale line citations, 10 mechanical findings on 11 PRs in one week)

Summary

Across the 391 PRs merged 2026-10-03..10, review bots flagged stale path:N / path:N-M line citations 14 times on 11 PRs. 10 of those were mechanical: the cited line was past the end of the file (for example #6064, where ci.yml:2639 was cited but the file had 2561 lines). Nothing checked for this before review.

Fix

• scripts/check-line-citations.py scans every tracked *.md for PATH:N, PATH:N-M and comma lists of these (PATH:269-271,410-432). It resolves PATH against the repository root and the citing file's directory, against tracked files only, and fails when the highest cited line is past the end of every file it resolves to. It does not judge whether the line still says what the citation claims.
• These stay quiet by design: a path that resolves to no tracked file (bare basenames, times like 10:30, host:port), URLs, owner/repo:path citations, path:N:C line-and-column, fenced code blocks (sample output and illustrative rows), and point-in-time records (docs/adr/, docs/upstream/, CHANGELOG.md, .changes/), which cite the tree as it was when they were written.
• It is wired as an ungated continue-on-error step in lint-repo (id: line_citations) and feeds line-citations= into the hygiene aggregator. It scans the whole repository because a citation usually goes stale when the cited file changes, and that change rarely touches the citing doc. It takes about 1 s.

Backlog: with fences and records excluded, the repository has 0 violations, so the gate covers the whole repository from the start. The raw scan found 3 violations before those exclusions. None had a clear fix, so none were edited:

• an ADR decision row (docs/adr/0004…, CLAUDE.md:13-30), which describes CLAUDE.md as it was then;
• a discharged finding in docs/upstream/mattpocock-skills-v12-map.md that already says the cited lines no longer exist;
• an example table inside a text fence in rename-references/context/audit-modes.md.

Verification

• bash scripts/check-line-citations.test.sh: 14 tests pass. Firing cases: a line past the end, a range end past the end, a later item of a comma list, a path relative to the citing file, a citation ending a sentence. Quiet cases: valid line and range, a nonexistent path or bare basename, a URL with a port, an external repository citation, times, line:column, a fenced example, an ADR and a CHANGELOG. One more case checks that a citation after a closed fence still fires.
• python3 scripts/check-line-citations.py on this tree exits 0.
• scripts/run-ruff.sh check and format --check pass. shellcheck and actionlint are clean. These suites pass: aggregate-hygiene-results.sh --self-test, check-docs-only-gate.sh --check, check-fixture-git-isolation.sh --check, plan-test-lanes.test.sh, workflow-self-paths.test.sh, check-docs-only-gate.test.sh.
• selection-audit.test.sh fails one case locally because strace is missing. It fails the same way on origin/main.

Related

• Example findings: build(shared-lib): migrate hook-utils.sh to the regen flow and retire the sync machinery #6064, docs(plugin-philosophy): run local-scope test installs outside a worktree #6409.
• Pattern followed: scripts/check-code-metrics-config-reference.py with its .test.sh wrapper and lint-repo step.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-10T23:10:52.993351Z	9f26a6a	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 2m 16s —— View job

---

Security review in progress

• Gather context (PR diff, changed files, CLAUDE.md/AGENTS.md)
• Skip-gate check (open PR, security-relevant surface present, no prior review on this head)
• Read changed files: scripts/check-line-citations.py, scripts/check-line-citations.test.sh, scripts/test_check_line_citations.py, .github/workflows/pr-require-checks.yml
• Check instruction-surface deletions — N/A, this PR only adds a check; no rule, guardrail, or allowlist is removed, narrowed, or softened
• Hunt for injection / path-traversal / unsafe file handling in the new script
• Review workflow file change for Actions-hardening issues outside zizmor's lane
• Adversarial verification of candidate findings
• Post final findings

Findings

No security issues found.

Notes from the review (none rose to a reportable finding):

• scripts/check-line-citations.py resolves cited paths with Path.resolve() and then requires target.relative_to(REPO_ROOT) to succeed and the relative path to be in the git ls-files tracked set (scripts/check-line-citations.py#L83-L93). This closes the obvious path-traversal and symlink-escape angles: a citation like ../../../etc/passwd:1 or a tracked symlink pointing outside the repo resolves outside REPO_ROOT and is discarded rather than read.
• The only subprocess.run call uses a fixed argument list (git -C <root> ls-files -z), no shell=True, no interpolated untrusted input (scripts/check-line-citations.py#L132-L136) — no command-injection surface.
• The new workflow step (.github/workflows/pr-require-checks.yml#L646-L655) is continue-on-error: true, runs a local script with no secrets, no new permissions, and the workflow's trigger is the existing hardened pull_request (not pull_request_target/workflow_run) — out of scope for this lane per the zizmor deferral, and nothing here needs logic/trust-boundary reasoning beyond what zizmor covers.
• The test harness (scripts/test_check_line_citations.py) runs git init/git add in a tempfile.TemporaryDirectory() and explicitly scrubs leaked GIT_* env vars before doing so (scripts/test_check_line_citations.py#L19-L33), consistent with the repo's fixture-isolation convention — no risk of the test fixture touching the real repo.
• No deleted/narrowed/softened standing instructions in this diff — the instruction-surface-deletion lens doesn't apply.

No related issue to check per the PR body's No related issue: line (follow-up measured from a re-vet, not an issue-tracked request), and no label action needed.

[claude]

Claude finished @kyle-sexton's task in 5m 12s —— View job

---

Code review complete

• Gather context (PR diff, changed files, CLAUDE.md/AGENTS.md, REVIEW.md)
• Skip-gate check (open, ready for review, non-trivial change, no prior code-review-lane pass on this head)
• Read changed files: scripts/check-line-citations.py, scripts/check-line-citations.test.sh, scripts/test_check_line_citations.py, .github/workflows/pr-require-checks.yml
• Review correctness, error handling, test coverage, maintainability
• Adversarial verification of candidate findings
• Post final findings

No blocking findings.

Traced the detector against each of the 14 test cases by hand (regex lookbehind/lookahead, fence-state machine, two-base path resolution, longest-candidate selection) and didn't find a case the tests miss:

• The fence state machine (check-line-citations.py#L96-L127) correctly handles nested/mismatched fence lengths and char types (backtick vs. tilde), and only quiets fenced blocks, not indented code — which is fine here because MD046: fenced already bans indented code blocks repo-wide (.markdownlint-cli2.jsonc:26), so that gap can't produce a false positive in this tree.
• candidates() correctly excludes paths that resolve outside the repo root (e.g. ../../x) via the relative_to ValueError, and the latest commit (9f26a6a) already fixed the "names the wrong candidate" bug with a regression test for the two-candidate case.
• The new line_citations step and CHECK_RESULTS entry follow the exact same continue-on-error + raw-outcome-to-aggregator wiring as the sibling ungated whole-repo scanners (code_metrics_skill_prose, docs_naming, etc.) — "ungated" here means not conditionally skipped on doc-only diffs, not non-blocking; a real violation does fail the job via the aggregator, consistent with its siblings.
• The test fixture's leaked-git-var scrub list matches scripts/test-git-helpers.sh exactly (7 vars), satisfying check-fixture-git-isolation.sh.
• The .test.sh wrapper follows the established python-probe.sh contract identically to check-code-metrics-config-reference.test.sh.

Could not independently verify: this sandbox's permission mode denies python3/bash <script> execution (gh/git/ls are allowed), so I could not myself run scripts/check-line-citations.test.sh, python3 scripts/check-line-citations.py, or scripts/run-ruff.sh. The PR body's claims about test results, the 0-violation scan, and ruff/shellcheck/actionlint cleanliness are author-reported and unverified by this review.

[github-actions]

claude-security-review has reviewed this pull request through 03ed392; a later push is reviewed from there.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9f26a6ad22

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 1m 39s —— View job

---

Security review (/review:security-review)

• Gather context (PR diff, changed files)
• Check skip gate (not applicable — PR is open, has security-relevant surface: new script reads tracked files by attacker-influenceable path strings)
• Review scripts/check-line-citations.py for injection / path-traversal / unsafe handling
• Review .github/workflows/pr-require-checks.yml change (deferred Actions-hardening findings to zizmor per lane scope)
• Review test files
• Check for instruction-surface deletions (none — diff only adds a check/step, no CLAUDE.md/AGENTS.md/rules/skill-or-agent-body/allowlist narrowing)
• Post findings

No security issues found.

Notes from the review (informational, not findings):

• Path resolution in candidates() builds a target from attacker-influenceable markdown content (base / cited), but it is guarded correctly: .resolve() followed by target.relative_to(REPO_ROOT) rejects anything that escapes the repository root (catching ValueError), and the result must additionally be in the git ls-files tracked set before it's read. A crafted citation like ../../../etc/passwd:1 or a symlink pointing outside the repo cannot cause a read outside the checkout.
• The git ls-files -z subprocess call uses an argument list (no shell=True), so there's no command-injection surface.
• The workflow addition (.github/workflows/pr-require-checks.yml) only adds a continue-on-error: true step running a fixed command with no interpolation of PR-controlled data into run: — no expression-injection surface for this lane to flag; broader Actions hardening is zizmor's lane.
• The test suite's fixture isolation (clearing GIT_DIR/GIT_WORK_TREE/etc. before git init/git add in a temp dir) correctly prevents the test's throwaway repo operations from leaking into the real checkout.

[github-actions]

claude-review has reviewed this pull request through 03ed392; a later push is reviewed from there.

[claude]

Claude finished @kyle-sexton's task in 1m 38s —— View job

---

Security review (/review:security-review)

• Gather context (PR diff, changed files, CLAUDE.md/AGENTS.md)
• Skip-gate check (PR open; head advanced to 03ed392 since the last security review on 9f26a6a, so re-review was required; security-relevant surface present — script reads tracked files by attacker-influenceable path strings)
• Diff 9f26a6a...03ed392 (the only commit since the last review): fence-recognition fix for block quotes/lists in scripts/check-line-citations.py
• Re-check scripts/check-line-citations.py end-to-end for injection / path-traversal / unsafe handling
• Check for instruction-surface deletions
• Adversarial verification of candidate findings
• Post final findings

No security issues found.

The only change since the prior security review (9f26a6a) is this commit, which tightens FENCE from ^\s*({3,}|~{3,}) to recognize a fence opener inside a block quote (> ) or a list item marker (- , 1. ), and fixes the closer check to measure trailing content from the *matching* fence's own match end (line[opener.end():]) rather than from line.strip()[len(run):]`:

• scripts/check-line-citations.py#L65-L66 — the new FENCE regex. This only changes which lines the scanner treats as "inside a fence" (and therefore skips); it does not touch the path-resolution or subprocess code already reviewed clean in the prior three passes. No new attacker-controlled input reaches a filesystem or process API here — fence/opener state only gates whether a line's citations are matched at all.
• The added (?:\s*>)* and (?:(?:[-*+]|\d+[.)])\s+)? groups are not susceptible to catastrophic backtracking: (?:\s*>)* can only repeat by consuming a literal > each iteration (no ambiguity in how \s* partitions against it), and the rest is a single bounded alternation — still linear in line length, so no ReDoS surface from markdown content in this repo (including content a PR author controls).
• scripts/check-line-citations.py#L110 — line[opener.end():].strip() indexes into the current line using that same line's own match object, so this can't read past the line or from stale offsets carried over from the opening fence's line.
• No CLAUDE.md/AGENTS.md/rules-file/skill-or-agent-body/allowlist changes in this diff, so the instruction-surface-deletion lens doesn't apply.
• Confirmed the rest of the PR (path resolution via .resolve() + relative_to(REPO_ROOT) + tracked-set membership, and the fixed-argument-list git ls-files -z subprocess call) is unchanged from the versions already reviewed clean in the three prior passes on this PR.

No related issue to check per the PR body's No related issue: line, and no label action needed.

[claude]

Claude finished @kyle-sexton's task in 3m 57s —— View job

---

Code review (/review:code-review) complete

• Gather context (PR diff, changed files, CLAUDE.md/AGENTS.md, REVIEW.md)
• Skip-gate check (open, ready for review, meaningful surface; prior passes on this lane covered 9f26a6a, the diff below is scoped to what changed since)
• Review changed files for correctness, error handling, test coverage, maintainability
• Adversarial verification of candidate findings
• Post findings

No blocking findings.

This lane's prior pass covered the PR through 9f26a6a. The only substantive change since then is 03ed392 ("recognize fences inside block quotes and list items"), which fixes the P2 false-positive the Codex review flagged (a fenced block nested in a > block quote or a list item wasn't recognized as a fence, so a path-shaped example inside one could trip the gate). I traced the new FENCE regex and the revised close-match check by hand against each of the repo's three real block-quoted fences (plugins/songwriting/context/pat-pattison/research/song-forms-examples.md:591-594,599-605,736-740 and five-compositional-elements.md:362-366) and against the two new regression tests (scripts/test_check_line_citations.py:120-122) — open/close pairing, nested > levels, and list-marker indentation all resolve correctly, and I didn't find an input in this tree that the new prefix-matching would mis-close or fail to close. check-line-citations.py:66, check-line-citations.py:97-113.

The rest of the file (CITATION regex, candidates() two-base resolution, "fires only when past the end of every candidate" logic, the workflow wiring into lint-repo and the hygiene aggregator) is unchanged since the prior claude-review and claude-security-review passes on 9f26a6a, which found nothing blocking; I re-checked that logic myself and agree.

Could not independently verify: this sandbox's permission mode denies python3/bash <script> execution, so I could not run scripts/check-line-citations.test.sh or python3 scripts/check-line-citations.py myself. The PR body's test-suite and ruff/shellcheck/actionlint results are author-reported and unverified by this review (same limitation the prior code-review pass on this PR noted).