diff options
| author | Yuriy Andamasov <yuriy@vyos.io> | 2026-05-11 12:22:45 +0300 |
|---|---|---|
| committer | Yuriy Andamasov <yuriy@vyos.io> | 2026-05-11 12:22:45 +0300 |
| commit | c7702cea43ad070537c531bdb1894bf80ef4e62f (patch) | |
| tree | e6bdb6557a6e7465879573d2cd57244b3b1960df /.github/workflows/ai-validation.yml | |
| parent | 036339beacaafc885faf91456c0d7ee0f68011ae (diff) | |
| download | vyos-documentation-c7702cea43ad070537c531bdb1894bf80ef4e62f.tar.gz vyos-documentation-c7702cea43ad070537c531bdb1894bf80ef4e62f.zip | |
ci(ai-validation): address 3 follow-up CR/Copilot findings on validate-checkout step
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.
Diffstat (limited to '.github/workflows/ai-validation.yml')
| -rw-r--r-- | .github/workflows/ai-validation.yml | 20 |
1 files 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' |
