From c7702cea43ad070537c531bdb1894bf80ef4e62f Mon Sep 17 00:00:00 2001
From: Yuriy Andamasov <yuriy@vyos.io>
Date: Mon, 11 May 2026 12:22:45 +0300
Subject: ci(ai-validation): address 3 follow-up CR/Copilot findings on
 validate-checkout step
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After the first fix bundle landed on the three wave-5 PRs, three more
findings surfaced:

1. (CR on #1991, line 312) `rm -f` silently no-ops on directories. A
   fork could commit `changed-md.txt/`, `diff-md.patch/`, or
   `pass1-findings.json/` AS DIRECTORIES (not files), and the wipe
   step would skip them — the subsequent artifact-download writes
   into the real-file path would then collide with the fork-placed
   directory, producing unpredictable behaviour.

   Fix: use `rm -rf` for ALL reserved paths uniformly. The
   distinction was cosmetic at best; making it uniform closes the
   gap.

2. (Copilot on #1991, line 311) `.venv/` is also fork-attackable.
   `astral-sh/setup-uv` with `activate-environment: true` creates
   `${{ github.workspace }}/.venv` and prepends its `bin/` to PATH.
   If a fork pre-populates `.venv/bin/`, those binaries land in
   PATH for subsequent steps (Pass 1 runs `vyos-doc-review`, Pass 2
   runs `claude-code-action` whose internals may shell out).

   Fix: add `.venv` to the wipe list.

3. (Copilot on #1992, line 298) The inline comment block contains
   a literal `PR's` — a stray shell heredoc escape sequence
   that survived the commit verbatim. Confusing in YAML.

   Fix: replace with plain `PRs`.

Mirrored byte-identically across rolling/circinus/sagitta + canonical
follow-up after this wave merges.
---
 .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