diff options
| -rw-r--r-- | .github/workflows/ai-validation.yml | 73 |
1 files changed, 61 insertions, 12 deletions
diff --git a/.github/workflows/ai-validation.yml b/.github/workflows/ai-validation.yml index f75081dc..d5f1160f 100644 --- a/.github/workflows/ai-validation.yml +++ b/.github/workflows/ai-validation.yml @@ -59,7 +59,7 @@ jobs: has_md_changes: ${{ steps.changes.outputs.has_md_changes }} steps: - name: Checkout PR merge ref (NO credentials) - uses: actions/checkout@v6 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: ref: refs/pull/${{ github.event.number }}/merge persist-credentials: false @@ -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 @@ -110,8 +134,14 @@ jobs: || "$path" == "../"* \ || "$path" == *"/.." \ || "$path" == ".." ]]; then - echo "::warning::Skipping unsafe path with traversal/absolute prefix: $path" - continue + # Fail-fast (don't `continue`) so an unsafe path can't silently + # bypass Pass 1 (no file copied into _changed_md/) and Pass 2 + # (LLM tools see no content) — same reasoning as the non-regular + # tree-entry check below: maintainers must explicitly decide to + # land such a path. Visible failure > silent skip on inputs that + # warrant the closest look. + echo "::error::Refusing to bundle: path has traversal/absolute prefix: $path" + exit 1 fi # Refuse to bundle non-regular tree entries (symlinks mode 120000, # submodules 160000, etc). Skipping silently would let a PR that @@ -145,7 +175,7 @@ jobs: fi - name: Upload PR input artifact - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: pr-input path: | @@ -164,8 +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 - id-token: write + issues: write steps: # Pass secrets via env: rather than inlining ${{ secrets.X }} into the # shell script. GitHub Actions template-expands ${{ ... }} BEFORE bash @@ -209,7 +247,7 @@ jobs: - name: Download PR input if: steps.secrets-check.outputs.skip != 'true' - uses: actions/download-artifact@v4 + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 with: name: pr-input @@ -235,7 +273,7 @@ jobs: - name: Sparse-checkout branches.json from reviewer if: steps.secrets-check.outputs.skip != 'true' - uses: actions/checkout@v6 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: repository: VyOS-Networks/vyos-docs-opus-reviewer ref: ${{ env.REVIEWER_REF }} @@ -266,7 +304,7 @@ jobs: - name: Checkout vyos-1x at mapped branch if: steps.secrets-check.outputs.skip != 'true' - uses: actions/checkout@v6 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: repository: vyos-networks/vyos-1x ref: ${{ steps.branch.outputs.vyos1x }} @@ -285,7 +323,11 @@ jobs: uses: robinraju/release-downloader@28fc21f50d76778e7023361aa1f863e717d3d56f # v1.13 with: repository: VyOS-Networks/vyos-docs-opus-reviewer - latest: true + # Pin the DB to the same release tag as REVIEWER_REF so a future + # reviewer-v1.x.x release with a schema change cannot be silently + # picked up while the pinned reviewer code still expects the old + # schema. Reproducibility > recency for this artifact. + tag: ${{ env.REVIEWER_REF }} fileName: reference-db-${{ steps.branch.outputs.vyos1x }}.tar.gz out-file-path: .reference-db token: ${{ steps.app.outputs.token }} @@ -300,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 @@ -330,7 +379,7 @@ jobs: # reviewer-src/.git/config where the Pass 2 LLM step could read it. - name: Checkout reviewer package source (pinned to REVIEWER_REF) if: steps.secrets-check.outputs.skip != 'true' - uses: actions/checkout@v6 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: repository: VyOS-Networks/vyos-docs-opus-reviewer ref: ${{ env.REVIEWER_REF }} @@ -359,7 +408,7 @@ jobs: - name: Pass 2 — Claude review if: steps.secrets-check.outputs.skip != 'true' - uses: anthropics/claude-code-action@v1 + uses: anthropics/claude-code-action@476e359e6203e73dad705c8b322e333fabbd7416 # v1.0.119 with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} # claude-code-action@v1 removed the top-level `model` input; CLI |