ci(ai-validation): 3 CR/Copilot follow-ups — NUL guard restore + issues: write + fail-closed gate

CodeRabbit / Copilot findings on the merged PR #1959 and the still-open
PRs #1960 + #1969:

1. NUL/control-char guard regressed. PR #1959 (merged into circinus
   without this) — and likewise sagitta/rolling-followup — had the
   guard inadvertently dropped during the rolling@HEAD rebase, because
   rolling@HEAD never had the corrected form. Restoring the
   user-corrected form: `LC_ALL=C grep -zPq "[\\x01-\\x1F\\x7F]"`
   reads the NUL-delimited *.z files directly (NUL is the record
   delimiter, not a forbidden byte) and rejects every other control
   byte 0x01-0x1F + 0x7F. An earlier `tr -d "\\0\\n\\r" | grep ...`
   form stripped the very bytes it was meant to reject before the
   grep ran — see the inline comment block for the contract.

2. `gh pr comment` requires `issues: write`. The skip-notice step and
   Pass 2 summary comment both post via `POST /repos/{owner}/{repo}/
   issues/{number}/comments` — a PR conversation comment IS an issue
   comment in GitHub's data model. With only `pull-requests: write`
   the call can 403 on repos whose default GITHUB_TOKEN permission
   split routes issue-comment writes through `issues:`. Adding
   `issues: write` alongside the existing `pull-requests: write` keeps
   every comment path working without expanding the trust surface
   beyond what the original validate job needed.

3. Fail-closed gate used `[ -s changed-md.txt ]` (--diff-filter=ACMRT,
   excludes deletions) but validate is now gated on `has_md_changes`
   which is computed from `[ -s diff-md.patch ]` (unfiltered, deletion-
   aware). A deletion-only PR with a missing reference DB would reach
   validate (has_md_changes=true) but bypass the fail-closed gate
   (changed-md.txt empty), masking the DB-missing condition. Switch
   the gate to `[ -s diff-md.patch ]` so both signals agree.

A separate Copilot finding (line 170, "validate gated on has_md_changes
means RST-only PRs do not run") is pushed back on the PR thread as
intentional design — RST is legacy per the RST→MyST migration, and
the RST bookkeeping in prepare exists to surface mixed-MD-RST PRs in
the Pass 2 prompt, not to drive validation on RST-only PRs.

1 files changed, 41 insertions, 1 deletions

diff --git a/.github/workflows/ai-validation.yml b/.github/workflows/ai-validation.yml
index 425644f1..d5f1160f 100644
--- a/.github/workflows/ai-validation.yml
+++ b/.github/workflows/ai-validation.yml
@@ -80,6 +80,30 @@ jobs:
           # in changed-md.txt (which drives the bundling step).
           git diff "$BASE...HEAD" --name-only --diff-filter=ACMRT -z -- 'docs/**/*.md'  > changed-md.z
           git diff "$BASE...HEAD" --name-only --diff-filter=ACMRT -z -- 'docs/**/*.rst' > changed-rst.z
+          # Reject paths containing line-disrupting control bytes (LF, CR,
+          # other 0x01-0x1F + 0x7F) before generating the newline-delimited
+          # *.txt manifests. NUL itself can't appear in a git pathname
+          # (it's the on-disk tree-entry terminator), so it stays out of
+          # the rejection class and remains the legitimate record delimiter
+          # for `git diff -z` — `grep -z` honors that contract.
+          #
+          # POSIX filesystems generally allow LF/CR in filenames and git
+          # stores them fine; the hazard is purely in our line-delimited
+          # downstream tooling. Without this guard, `tr '\0' '\n'` on a
+          # path like `docs/foo\nbar.md` would split it into two logical
+          # lines — downstream consumers reading line-by-line would miss
+          # validation coverage on the real file (or worse, act on a
+          # synthetic path). Fail fast at this seam.
+          #
+          # An earlier `tr -d '\0\n\r' | grep [\x00-\x1F\x7F]` form
+          # stripped the very bytes it was meant to reject before the
+          # grep ran — defeating the guard.
+          for z in changed-md.z changed-rst.z; do
+            if LC_ALL=C grep -zPq '[\x01-\x1F\x7F]' "$z"; then
+              echo "::error::Refusing to bundle: path in $z contains a control character. Reject the offending file name in the PR."
+              exit 1
+            fi
+          done
           tr '\0' '\n' < changed-md.z  > changed-md.txt
           tr '\0' '\n' < changed-rst.z > changed-rst.txt
           git diff "$BASE...HEAD" -- 'docs/**/*.md' > diff-md.patch
@@ -170,7 +194,16 @@ jobs:
     runs-on: ubuntu-latest
     permissions:
       contents: read
+      # pull-requests: write is required for inline review comments via
+      # mcp__github_inline_comment__create_inline_comment (Pass 2).
+      # issues: write is required for `gh pr comment` (the skip-notice
+      # step and Pass 2's top-level summary comment) — `gh pr comment`
+      # posts via POST /repos/{owner}/{repo}/issues/{number}/comments,
+      # which the issues scope governs. Granting both keeps every
+      # comment path working on repos where the default GITHUB_TOKEN
+      # permissions split issue and PR scopes.
       pull-requests: write
+      issues: write
     steps:
       # Pass secrets via env: rather than inlining ${{ secrets.X }} into the
       # shell script. GitHub Actions template-expands ${{ ... }} BEFORE bash
@@ -309,7 +342,14 @@ jobs:
         if: steps.secrets-check.outputs.skip != 'true'
         run: |
           set -euo pipefail
-          if [ -s changed-md.txt ] && [ ! -d .reference-db/extracted ]; then
+          # Gate on diff-md.patch (unfiltered) rather than changed-md.txt
+          # (--diff-filter=ACMRT). The job-level `if: has_md_changes` already
+          # ensures we only reach here when there are MD-related changes —
+          # including deletion-only PRs (where changed-md.txt is empty by
+          # design but diff-md.patch isn't). Using changed-md.txt here would
+          # silently skip the fail-closed check on those PRs, masking a
+          # missing reference DB from the reviewer.
+          if [ -s diff-md.patch ] && [ ! -d .reference-db/extracted ]; then
             echo "::error::Reference DB missing for vyos-1x branch '${{ steps.branch.outputs.vyos1x }}'. Pass 1 cannot run. Re-trigger rebuild-reference.yml in the reviewer repo and re-run."
             exit 1
           fi