From 08b6c6c75ebf3c4bd014665b989e4a07a4ed20e4 Mon Sep 17 00:00:00 2001 From: Yuriy Andamasov Date: Mon, 11 May 2026 12:28:58 +0300 Subject: ci(ai-validation, sagitta): follow-up — uniform rm -rf + wipe .venv + drop stray quote escape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sagitta merge of #1992 landed BEFORE the 3 follow-up Copilot/CR findings on the same wave were addressed. This PR brings sagitta back to byte-identity with rolling (PR #1990) and circinus (PR #1991) — same 3 fixes that those PRs got via their second-round commit: 1. (CR on #1991) Switch wipe step to use `rm -rf` uniformly for all reserved paths. `rm -f` silently no-ops on directories; a fork could commit `pass1-findings.json/` AS a directory and bypass the wipe. 2. (Copilot on #1991) Add `.venv` to the wipe list. astral-sh/setup-uv with activate-environment:true creates ${{ github.workspace }}/.venv and prepends its bin/ to PATH; a fork-pre-populated .venv/bin/ could poison PATH for subsequent steps. 3. (Copilot on #1992) Replace literal `PR"\'"\'"s` (a stray shell heredoc escape) with plain `PR's` in the inline comment block. No behavioural change beyond the three fixes; sagitta now matches the post-follow-up state of rolling/circinus. --- .github/workflows/ai-validation.yml | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ai-validation.yml b/.github/workflows/ai-validation.yml index 4ea2897d..ccb6fe1c 100644 --- a/.github/workflows/ai-validation.yml +++ b/.github/workflows/ai-validation.yml @@ -295,21 +295,25 @@ jobs: persist-credentials: false fetch-depth: 2 - # Defense-in-depth: actions/checkout above brings the PR'"'"'s tree + # 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. + # reference-DB extract, reviewer install, Pass 1 output, the + # uv-managed .venv/). Wiping the reserved paths guarantees the + # subsequent producer steps start from a clean slate and that + # PATH (which setup-uv prepends with ${{ github.workspace }}/ + # .venv/bin) is not poisoned by fork-controlled binaries. + # Uses `rm -rf` uniformly so that a fork-controlled DIRECTORY + # at a path normally holding a regular file (e.g. a malicious + # `pass1-findings.json/` directory) is also removed — `rm -f` + # silently no-ops on directories. - 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 + rm -rf _changed_md .reference-db .vyos-1x reviewer reviewer-src .venv \ + changed-md.txt changed-rst.txt diff-md.patch pass1-findings.json - name: Download PR input if: steps.secrets-check.outputs.skip != 'true' -- cgit v1.2.3 From 4f8ebb8475872c8cabf599257c7f788a2e8871c1 Mon Sep 17 00:00:00 2001 From: Yuriy Andamasov Date: Mon, 11 May 2026 12:32:37 +0300 Subject: ci(ai-validation): merge-ref checkout + retarget origin to fork + wipe CLAUDE.md/.claude (CR findings) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two new CR findings on PR #1990 (both raised on the same review cycle): 1. (CR, line 296, Major, Heavy lift) Tree drift between Pass 1 and Pass 2. The prior fix in this PR used `head.sha` (origin=fork) so the action's `git fetch origin ` resolves — but that left validate's workspace tree at PR HEAD while prepare bundled `diff-md.patch` + `_changed_md/` from `refs/pull//merge`. For PRs where base also contributes content to a touched file, Pass 2 workspace-Read could disagree with Pass 1's view. Fix: switch the validate checkout back to `refs/pull//merge` (workspace tree now matches the prepare bundle) and address the action's `git fetch origin ` requirement separately by `git remote set-url origin ` after the checkout. The workspace tree is unchanged by the remote retarget; only the fetch destination is updated. For public forks (the only kind targeting public vyos/vyos-documentation) the unauthenticated fetch succeeds. 2. (CR, line 316, Major) `anthropics/claude-code-action@v1` invokes the Claude Code CLI, which auto-discovers and loads `CLAUDE.md` and `.claude/**` (memory, skills, hooks, MCP, plugins) from the workspace at session startup. A malicious fork could pre-place a `CLAUDE.md` with prompt-injection content, or a `.claude/skill.md` marked as auto-load. Documented at: - code.claude.com/docs/en/claude-directory.md - code.claude.com/docs/en/memory Fix: add `CLAUDE.md` and `.claude` to the wipe list so Claude's auto-discovery starts from a clean slate (workflow-controlled state only). `--bare` would also disable this but would lose the `mcp__github_inline_comment` MCP server the action provides, so wiping reserved paths is the better fit. Mirrored across rolling (#1990), circinus (#1991), and the sagitta follow-up (#1993). Canonical sync once these merge. --- .github/workflows/ai-validation.yml | 99 +++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 43 deletions(-) diff --git a/.github/workflows/ai-validation.yml b/.github/workflows/ai-validation.yml index ccb6fe1c..f69844fb 100644 --- a/.github/workflows/ai-validation.yml +++ b/.github/workflows/ai-validation.yml @@ -256,63 +256,76 @@ 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 ` and reads files like - # `docs/.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 `` (e.g. - # `contributor/branch`) does not exist and the action's fetch - # fails. + # Check out the PR's MERGE REF into the workspace root. Required by + # anthropics/claude-code-action@v1: it runs `git fetch origin + # ` and reads files from the working dir during setup. + # The merge ref is GitHub's auto-computed merge of base + head; the + # working tree matches the bundled diff-md.patch + _changed_md/ + # produced by `prepare`, so Pass 1 (which reads the bundle) and + # Pass 2 (which can also Read/Glob/Grep the workspace) operate on + # the same tree. # - # 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) + # The token is used to download the tree (not an unauthenticated + # fetch); persist-credentials:false suppresses writing it into the + # resulting .git/config so fork-controlled file content the Pass 2 + # LLM may read cannot exfiltrate it. + - name: Checkout PR merge ref (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 }} + ref: refs/pull/${{ github.event.number }}/merge 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 + # claude-code-action subsequently runs `git fetch origin ` + # internally during its setup. `` is the branch name in + # the PR'"'"'s HEAD repo — for fork PRs that branch does NOT exist on + # the base repo (which is what `origin` points at after the merge- + # ref checkout above), so the fetch fails with + # fatal: couldn't find remote ref refs/heads/ + # + # Re-point origin to the HEAD repo'"'"'s URL so the action'"'"'s fetch + # resolves. The workspace tree is unchanged — it remains the merge + # tree from above. For public forks (the only kind that target + # vyos/vyos-documentation, since the repo is public) the + # unauthenticated fetch from the fork succeeds without any credential + # (persist-credentials:false stripped the extraheader at the end of + # the previous step). + - name: Re-point origin to fork URL for claude-code-action'"'"'s git fetch + if: steps.secrets-check.outputs.skip != 'true' + env: + FORK: ${{ github.event.pull_request.head.repo.full_name }} + run: | + set -euo pipefail + git remote set-url origin "https://github.com/$FORK" + + # Defense-in-depth: actions/checkout above brings the PR's merge + # tree into the workspace root, which means a malicious fork could + # pre-create files/dirs at workflow-reserved paths that producer # steps below populate (artifact download, vyos-1x checkout, # reference-DB extract, reviewer install, Pass 1 output, the - # uv-managed .venv/). Wiping the reserved paths guarantees the - # subsequent producer steps start from a clean slate and that - # PATH (which setup-uv prepends with ${{ github.workspace }}/ - # .venv/bin) is not poisoned by fork-controlled binaries. - # Uses `rm -rf` uniformly so that a fork-controlled DIRECTORY - # at a path normally holding a regular file (e.g. a malicious - # `pass1-findings.json/` directory) is also removed — `rm -f` - # silently no-ops on directories. + # uv-managed .venv/, plus CLAUDE.md / .claude/ which the Claude + # Code CLI auto-loads as session instructions on startup — + # documented at code.claude.com/docs/en/claude-directory.md and + # code.claude.com/docs/en/memory). + # + # Wiping the reserved paths guarantees: + # * subsequent producer steps start from a clean slate + # * PATH (which setup-uv prepends with ${{ github.workspace }}/ + # .venv/bin) is not poisoned by fork-controlled binaries + # * Claude Code's auto-discovery of CLAUDE.md / .claude/ does + # not pull fork-controlled prompt-injection instructions into + # the Pass 2 session + # + # Uses `rm -rf` uniformly so a fork-controlled DIRECTORY at a path + # normally holding a regular file (e.g. `pass1-findings.json/`) is + # also removed — `rm -f` silently no-ops on directories. - 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 .venv \ + CLAUDE.md .claude \ changed-md.txt changed-rst.txt diff-md.patch pass1-findings.json - name: Download PR input -- cgit v1.2.3 From e9f0672232db0804b8995014b595a4d1acb1206c Mon Sep 17 00:00:00 2001 From: Yuriy Andamasov Date: Mon, 11 May 2026 12:51:13 +0300 Subject: ci(ai-validation): pin validate to prepare merge_sha + fix 2 stray escapes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CR/Copilot pass on the merge-ref + retarget-origin revision raised: 1. (CR on #1991, line 278, Major) Re-resolving refs/pull//merge in validate is racy. GitHub may advance the merge ref between prepare and validate (rapid pushes), so Pass 1 (artifact from prepare) and Pass 2 (validate workspace) can see different revisions. concurrency.cancel-in-progress narrows the window but does not make the ref immutable. Fix: prepare outputs its post-checkout `git rev-parse HEAD` as `merge_sha`, validate checks out THAT exact sha. Both jobs now operate on the same revision regardless of subsequent pushes. 2. (Copilot on #1991, two findings on line 294) The merge-ref + retarget commit introduced two NEW stray heredoc escapes: `PR"'sclaude-code-action'"s`. Same class of error as the previous-fix-cycle: shell heredoc escape sequences survived verbatim into YAML. Fix: replace with plain `PRs` / `claude-code-actions` in the inline comment block and step name. 3. (Copilot on #1990/#1991, "PR title/description says HEAD repo + head.sha") Metadata cleanup tracked separately — PR titles + bodies updated to describe the merge-ref + retarget-origin approach (no workflow file change for that part). Mirrored byte-identically across the 3 open wave-5 PRs (#1990 rolling, #1991 circinus, #1993 sagitta follow-up). Canonical sync once these merge. --- .github/workflows/ai-validation.yml | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ai-validation.yml b/.github/workflows/ai-validation.yml index f69844fb..993d1876 100644 --- a/.github/workflows/ai-validation.yml +++ b/.github/workflows/ai-validation.yml @@ -57,6 +57,14 @@ jobs: # validate's working-directory: _changed_md would otherwise fail # before any in-step short-circuit can run. has_md_changes: ${{ steps.changes.outputs.has_md_changes }} + # The exact SHA of the merge commit that prepare bundled. validate + # below checks out THIS sha (rather than re-resolving + # `refs/pull//merge`, which GitHub may update between prepare + # and validate on rapid pushes — concurrency.cancel-in-progress + # narrows the window but does not make the ref immutable). Pass 1 + # (artifact) and Pass 2 (validate workspace) now operate on the + # same revision. + merge_sha: ${{ steps.changes.outputs.merge_sha }} steps: - name: Checkout PR merge ref (NO credentials) uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -174,6 +182,11 @@ jobs: echo "has_md_changes=false" >> "$GITHUB_OUTPUT" fi + # Pin the merge SHA that prepare bundled, so validate below can + # check out the exact same revision and avoid drift if GitHub + # advances refs/pull//merge between jobs. + echo "merge_sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" + - name: Upload PR input artifact uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: @@ -273,25 +286,28 @@ jobs: if: steps.secrets-check.outputs.skip != 'true' uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: - ref: refs/pull/${{ github.event.number }}/merge + # Pin to the exact merge SHA prepare bundled so validate's + # workspace tree matches Pass 1's artifact even on a PR that + # has had additional pushes between prepare and validate. + ref: ${{ needs.prepare.outputs.merge_sha }} persist-credentials: false fetch-depth: 2 # claude-code-action subsequently runs `git fetch origin ` # internally during its setup. `` is the branch name in - # the PR'"'"'s HEAD repo — for fork PRs that branch does NOT exist on + # the PR's HEAD repo — for fork PRs that branch does NOT exist on # the base repo (which is what `origin` points at after the merge- # ref checkout above), so the fetch fails with # fatal: couldn't find remote ref refs/heads/ # - # Re-point origin to the HEAD repo'"'"'s URL so the action'"'"'s fetch + # Re-point origin to the HEAD repo's URL so the action's fetch # resolves. The workspace tree is unchanged — it remains the merge # tree from above. For public forks (the only kind that target # vyos/vyos-documentation, since the repo is public) the # unauthenticated fetch from the fork succeeds without any credential # (persist-credentials:false stripped the extraheader at the end of # the previous step). - - name: Re-point origin to fork URL for claude-code-action'"'"'s git fetch + - name: Re-point origin to fork URL for claude-code-action's git fetch if: steps.secrets-check.outputs.skip != 'true' env: FORK: ${{ github.event.pull_request.head.repo.full_name }} -- cgit v1.2.3