diff options
| author | Yuriy Andamasov <yuriy@vyos.io> | 2026-05-11 12:14:16 +0300 |
|---|---|---|
| committer | Yuriy Andamasov <yuriy@vyos.io> | 2026-05-11 12:14:16 +0300 |
| commit | 036339beacaafc885faf91456c0d7ee0f68011ae (patch) | |
| tree | 9fe94dbe29d309c5e043ec2a892b756bd00105ed /.github | |
| parent | c1d3149b53a7805d9d3c912ccba66a30688f82ad (diff) | |
| download | vyos-documentation-036339beacaafc885faf91456c0d7ee0f68011ae.tar.gz vyos-documentation-036339beacaafc885faf91456c0d7ee0f68011ae.zip | |
ci(ai-validation): address 4 Copilot findings on validate-checkout step
CR pass on #1990/#1991/#1992 raised 4 findings on the new validate-
side actions/checkout step. All valid; folding into one bundle:
1. Fork PR head-ref not found in base repo (#1990, #1991, #1992 each
raised this). The previous form checked out refs/pull/<n>/merge
from origin (vyos/vyos-documentation), leaving origin pointed at
the base repo. claude-code-action then runs
`git fetch origin <head-ref>` where <head-ref> is e.g.
`contributor/branch` — which does NOT exist in the base repo for
fork PRs, so the fetch fails the same way "no .git" failed before.
Fix: check out the PR HEAD repo (the fork for fork PRs) at the
head sha. origin now resolves to the head repo where the head
ref exists, so the action's fetch succeeds. github.token is the
default GITHUB_TOKEN which can read public forks (the only kind
of fork that can target vyos/vyos-documentation, since that repo
is public).
2. Untrusted PR content at workspace root can pre-create reserved
paths (#1991). The PRs tree at workspace root could in
principle contain `.reference-db/extracted/...`,
`pass1-findings.json`, `_changed_md/poisoned.md`, etc., which
subsequent steps would treat as workflow-generated. The fail-
closed gate only checks `[ ! -d .reference-db/extracted ]` so a
PR-placed empty `.reference-db/extracted/` would bypass it.
Fix: add a "Wipe reserved workspace paths" step right after the
checkout that `rm -rf`s the workflow-owned paths
(.reference-db, .vyos-1x, reviewer, reviewer-src, _changed_md)
and `rm -f`s the workflow-owned files (pass1-findings.json,
diff-md.patch, changed-*.txt) — every one of these is recreated
by the trusted producer step immediately following.
3. Checkout runs unconditionally even on skip=true paths (#1991).
The checkout only needs to run when validate will actually use
it (skip != true → Pass 2 runs).
Fix: gate the checkout (and the wipe step) on
`if: steps.secrets-check.outputs.skip != true`. Move them
AFTER the secrets-check step in step order so the conditional
is meaningful.
4. "NO credentials" wording misleading (#1992). actions/checkout
uses the GITHUB_TOKEN to download the tree — it is not an
unauthenticated fetch. `persist-credentials: false` only
suppresses writing the token into the resulting .git/config.
Fix: rename the step to "Checkout PR HEAD (no persisted
credentials)" and update the inline comment block to be explicit
that the token is used for the download but not persisted.
Mirrored byte-identically across rolling/circinus/sagitta + canonical
follow-up after this wave merges.
Diffstat (limited to '.github')
| -rw-r--r-- | .github/workflows/ai-validation.yml | 83 |
1 files changed, 55 insertions, 28 deletions
diff --git a/.github/workflows/ai-validation.yml b/.github/workflows/ai-validation.yml index fb57543b..4ea2897d 100644 --- a/.github/workflows/ai-validation.yml +++ b/.github/workflows/ai-validation.yml @@ -216,34 +216,6 @@ jobs: # by run 25658256103 on PR #1977. id-token: write steps: - # Check out the PR's merge ref into the workspace root. Required by - # anthropics/claude-code-action@v1: the action runs - # `git fetch origin <head-ref>` and reads files like - # `docs/<changed>.md` directly from the working dir during its - # setup. Without this checkout it fails with `fatal: not a git - # repository` + `could not open <file>`. - # - # Trust boundary preserved: - # - persist-credentials: false → no token in fork-content - # .git/config (so fork-controlled file contents that the - # LLM tool calls might read can't exfiltrate a token) - # - No shell step in validate executes fork code (no - # `pip install` / `npm install` / `make` against the - # workspace; Pass 1 runs the trusted reviewer CLI from - # reviewer-src/; Pass 2's allowlisted tools are Read / - # Glob / Grep / mcp__github_inline_comment + Bash - # restricted to `gh pr comment|diff|view`). - # - Subsequent steps create their own subdirs (_changed_md/ - # via artifact download, .vyos-1x/, reviewer-src/, reviewer/, - # .reference-db/) which coexist with the docs/scripts/.github - # content checked out here. - - name: Checkout PR merge ref into workspace (NO credentials) - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - ref: refs/pull/${{ github.event.number }}/merge - persist-credentials: false - fetch-depth: 2 - # Pass secrets via env: rather than inlining ${{ secrets.X }} into the # shell script. GitHub Actions template-expands ${{ ... }} BEFORE bash # parses the script, so a secret containing a single quote, backtick, @@ -284,6 +256,61 @@ jobs: --repo "${{ github.repository }}" \ --body "AI Validation skipped — required secrets are not configured on this repo (\`ANTHROPIC_API_KEY\`, \`VYOS_APP_ID\`, \`VYOS_APP_PRIVATE_KEY\`). Maintainers: see the workflow run for details." + # Check out the PR HEAD into the workspace root. Required by + # anthropics/claude-code-action@v1: it runs + # `git fetch origin <head-ref>` and reads files like + # `docs/<changed>.md` directly from the working dir during its + # setup. We check out the PR HEAD repo (which is the fork for + # fork PRs) so that the head-ref name resolves locally: doing a + # merge-ref checkout from `vyos/vyos-documentation` would leave + # origin pointed at the base repo, where `<head-ref>` (e.g. + # `contributor/branch`) does not exist and the action's fetch + # fails. + # + # The token is still used to download the tree (this is not an + # unauthenticated fetch); `persist-credentials: false` prevents + # it from being written into the resulting .git/config — so any + # fork-controlled file content that the Pass 2 LLM tools may + # read cannot exfiltrate the token. + # + # Trust boundary preserved: + # - persist-credentials: false (no token in .git/config) + # - No shell step in validate executes fork code (no + # `pip install` / `npm install` / `make` against the + # workspace; Pass 1 runs the trusted reviewer CLI from + # reviewer-src/; Pass 2's allowlisted tools are Read / + # Glob / Grep / mcp__github_inline_comment + Bash + # restricted to `gh pr comment|diff|view`). + # - The "Wipe reserved workspace paths" step below strips any + # fork-controlled placeholders at workflow-reserved paths + # (.reference-db, .vyos-1x, reviewer, reviewer-src, + # _changed_md, pass1-findings.json, diff-md.patch, changed-* + # .txt) before the trusted producer steps re-create them. + - name: Checkout PR HEAD (no persisted credentials) + if: steps.secrets-check.outputs.skip != 'true' + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + repository: ${{ github.event.pull_request.head.repo.full_name }} + ref: ${{ github.event.pull_request.head.sha }} + persist-credentials: false + fetch-depth: 2 + + # Defense-in-depth: actions/checkout above brings the PR'"'"'s tree + # into the workspace root, which means a malicious fork could + # pre-create files/dirs at the same paths that workflow-producer + # steps below populate (artifact download, vyos-1x checkout, + # reference-DB extract, reviewer install, Pass 1 output). Wiping + # the reserved paths guarantees subsequent producer steps start + # from a clean slate and the fail-closed gate that checks for + # `.reference-db/extracted` reflects workflow state, not PR + # content. + - name: Wipe reserved workspace paths (defense-in-depth vs fork-controlled placeholders) + if: steps.secrets-check.outputs.skip != 'true' + run: | + set -euo pipefail + rm -rf _changed_md .reference-db .vyos-1x reviewer reviewer-src + rm -f changed-md.txt changed-rst.txt diff-md.patch pass1-findings.json + - name: Download PR input if: steps.secrets-check.outputs.skip != 'true' uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 |
