| Age | Commit message (Collapse) | Author |
|---|
|
(#2183)
The reviewer-v1.0.2 tag ships a branches.json that maps docs branch
'rolling' to vyos-1x branch 'current'. The vyos-networks/vyos-1x
mirror renamed 'current' to 'rolling', so the 'Checkout vyos-1x at
mapped branch' step fails on every rolling-based PR (first observed
run 30565684480, PR #2177).
reviewer-v1.0.3 carries the corrected mapping (rolling -> rolling)
plus CI/docs-only changes; no reviewer Python source changes between
the two tags.
π€ Generated by [robots](https://vyos.io)
(cherry picked from commit 3263d8760117ab85bdfc6e13bbaf2d45db3c10a8)
Co-authored-by: Yuriy Andamasov <yuriy@vyos.io>
|
|
Swap the cross-repo-checkout token from the dedicated vyos-docs-reviewer
App (VYOS_APP_ID/_PRIVATE_KEY) to the shared vyos-bot App via the fleet
get-token composite, scoped contents:read. Skip-check now gates on
APP_CLIENT_ID + APP_PRIVATE_KEY + ANTHROPIC_API_KEY (org-level vyos-bot
creds); reword the stale token comment. PR-comment posting still uses
the default GITHUB_TOKEN. Byte-identical to the paired reference copy in
VyOS-Networks/vyos-docs-opus-reviewer (scripts/ai-validation.yml).
π€ Generated by [robots](https://vyos.io)
(cherry picked from commit 459109f13adf026d012c4e803f4a86a8f84e6ce3)
|
|
The upstream `anthropics/claude-code-action` performs a write-permission
check on `github.actor` before our skip logic runs. On `pull_request_target`
the actor is the PR author; external contributors resolve to `read` and the
action exits 1 with `Actor does not have write permissions to the repository`.
Net effect: AI validation has been failing on every external-contributor PR
(LiudmylaNad, teslazonda, scottlaird in the last 4 weeks) while succeeding
on maintainer PRs. Failure reproduced on run 26541079685 (PR #2061).
Fix: set `allowed_non_write_users: '*'` on the Pass 2 step. The action
bypasses the actor check when this input is set and `github_token` is
provided (already the case). The action also auto-scrubs Anthropic / cloud
/ GHA secrets from subprocess envs when this input is set.
Safe in THIS workflow because the existing defense-in-depth bounds what
Pass 2 can do with untrusted PR content:
- `allowedTools` restricted to inline-comment + read-only surfaces
- `github_token` is the PR-scoped default (not the broader VYOS_APP_ID)
- prompt marks PR content as untrusted via `<UNTRUSTED-PR-CONTENT>`
- workspace-wipe removes `CLAUDE.md` / `.claude/` before Pass 2
- prepare bundles MD via `git show HEAD:<path>` (blob, not `cp`)
Full rationale inlined as a comment block above the new input.
π€ Generated by [robots](https://vyos.io)
(cherry picked from commit 1fa39bd7ac64898f7a21922bc0f195d115d3cdeb)
|
|
Lifts the existing Mergify-author short-circuit (today inside validate's
`secrets-check` step) to a job-level `if:` on `prepare`, so the whole
pipeline skips for backport/queue PRs.
Why now: every Mergify backport whose merge ref shares no shallow
ancestor with the (advanced) base branch fails the prepare step at
git diff "$BASE...HEAD" --name-only ...
fatal: FETCH_HEAD...HEAD: no merge base
(because base is `git fetch --no-tags --depth=1` and the merge ref is
`fetch-depth: 2`). Proximate symptom: run 25842928620 on PR #2042
(sagitta backport of #2023). AI Validation isn't a required check so
the queue isn't blocked, but every Mergify backport is left with a red
"prepare" check that adds noise to PR review.
The validate-level skip in commit 0e8a2956 was correct for the
"claude-code-action rejects bot-initiated runs" failure mode but
fires too late β prepare has already run and crashed before validate's
`if: needs.prepare.outputs.has_md_changes == 'true'` even evaluates.
Implementation: single job-level `if:` on prepare. validate's
`needs: [prepare]` cascades the skip naturally (skipped needs make
the dependent's expression-based `if:` evaluate against empty outputs).
The in-step author check in validate stays as defense-in-depth.
π€ Generated by [robots](https://vyos.io)
(cherry picked from commit 7d94d6116be1a4776b7317cb5190a83dd065e571)
|
|
The upstream anthropics/claude-code-action rejects bot-initiated
workflow runs with:
Workflow initiated by non-human actor: mergify (type: Bot).
Add bot to allowed_bots list or use '*' to allow all bots.
This leaves a failing required check on every Mergify backport PR
(#2025 today is the proximate symptom β circinus backport of #2016 is
blocked from auto-merging by this).
Per org policy, Mergify-authored PRs skip the bot-review flow entirely:
the underlying change was already reviewed on the source PR. Re-running
validation on the backport would also post duplicate findings.
Fix: extend the existing `secrets-check` step (kept the id for backwards
compat with the cascade of `if: steps.secrets-check.outputs.skip != 'true'`
gates) to also short-circuit when `github.event.pull_request.user.login`
equals `mergify[bot]`. Add a `skip_reason` output so the notify-on-PR
step can distinguish bot-skip (silent) from missing-secrets (post a
notice comment).
Implementation notes:
- PR author is bound through env: PR_AUTHOR (same pattern as secret
bindings) so the template-expansion happens before bash sees the
value as an already-quoted env variable.
- Uses `printf '...%s\n' "$PR_AUTHOR"` for the notice line so the
shell β not the YAML template engine β interpolates the author into
the workflow log output.
- Renamed step name to "Decide whether to skip validation" to reflect
the broader scope; step id stays `secrets-check`.
π€ Generated by [robots](https://vyos.io)
(cherry picked from commit d75de0c24b971a2d87e817ac3cb8e09cb816aac0)
|
|
The previous CR-driven fix (7a7a8002) re-pointed origin to the PR's HEAD
fork on the assumption that claude-code-action calls
`git fetch origin <head-branch>`. That assumption is wrong for fork PRs:
the action detects a fork PR and fetches `pull/<n>/head` instead, and
those refs only exist on the base repo. Retargeting origin to the fork
therefore breaks the fork-PR fetch path with:
PR #<n> is from a fork, fetching via refs/pull/<n>/head...
fatal: couldn't find remote ref pull/<n>/head
Observed on run 25689321499 (PR #1891 from natali-rs1985/vyos-documentation).
Leave origin pointed at vyos/vyos-documentation (the base repo) β that's
where actions/checkout left it after the merge-ref checkout, and where
`pull/<n>/head` resolves. Replace the step with an explanatory comment.
π€ Generated by [robots](https://vyos.io)
(cherry picked from commit 68f87deca228ad2b6b07c4eb64f201c0838e494f)
|
|
CR/Copilot pass on the merge-ref + retarget-origin revision raised:
1. (CR on #1991, line 278, Major) Re-resolving refs/pull/<n>/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.
|
|
CLAUDE.md/.claude (CR findings)
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 <head-ref>` resolves β but that
left validate's workspace tree at PR HEAD while prepare bundled
`diff-md.patch` + `_changed_md/` from `refs/pull/<n>/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/<n>/merge`
(workspace tree now matches the prepare bundle) and address the
action's `git fetch origin <head-ref>` requirement separately by
`git remote set-url origin <fork-url>` 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.
|
|
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.
|
|
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.
|
|
needs git workspace)
Smoke verify on PR #1977 (run 25659408343 after github_token fix
landed) reached Pass 2 with valid auth but failed at action setup:
fatal: could not open `docs/quick-start.md` for reading: No such
file or directory
fatal: not a git repository (or any of the parent directories): .git
Action failed with error: Command failed:
git fetch origin --depth=20 yuriy/docs-smoke-test-quickstart
The action expects to run inside a git checkout of the PR's repo so
it can `git fetch <head-ref>` and read changed files from the working
directory. Our split-job design checks out vyos-1x, reviewer-src, and
the reviewer-config sparse-checkout into named subdirs but leaves the
workspace root empty (we only have _changed_md/ via artifact download).
Add `actions/checkout` of `refs/pull/<n>/merge` as the FIRST step in
validate. Subsequent steps that create named subdirs (_changed_md/,
.vyos-1x/, reviewer-src/, reviewer/, .reference-db/) coexist with the
docs/scripts/.github content checked out here β no path collisions.
Trust model unchanged:
* persist-credentials: false (no token in fork-content .git/config)
* No shell step executes fork code (no pip/npm/make against the
workspace; Pass 1 runs the trusted reviewer CLI; Pass 2 only uses
allowlisted Read/Glob/Grep/mcp__github_inline_comment + Bash
restricted to `gh pr comment|diff|view`)
* The prepare job continues to provide the canonical bundled
changed-md set via the pr-input artifact; validate now ALSO has
the full PR tree available for cross-reference reads
Mirrored byte-identically across rolling/circinus/sagitta + canonical
follow-up.
|
|
Second smoke verify on PR #1977 (run 25658927427) reached Pass 2 but
failed with "Action failed with error: Invalid OIDC token". Diagnosis:
The action (anthropics/claude-code-action@v1) mints an OIDC token with
audience `claude-code-github-action` and POSTs it to
`api.anthropic.com/api/github/github-app-token-exchange` to receive
an Anthropic-managed GitHub App token for posting back to the PR.
The endpoint validated our OIDC token and rejected it ("Invalid
OIDC token") β this happens on repos where Anthropic-side OIDC
federation hasn't been provisioned for the GitHub org.
Bypass the exchange entirely by providing `github_token` to the
action: `setupGitHubToken()` in src/github/token.ts checks
`process.env.OVERRIDE_GITHUB_TOKEN` first and uses it directly if
set, skipping OIDC.
`${{ github.token }}` is auto-scoped to the current PR repo and gets
the validate job's permissions (pull-requests: write,
issues: write, contents: read) β exactly what the action needs.
`steps.app.outputs.token` (our App token) is NOT suitable here: it's
scoped to the VyOS-Networks org (vyos-1x, vyos-docs-opus-reviewer)
for the earlier reviewer-source / vyos-1x checkouts, and has no
write access on vyos/vyos-documentation PRs.
Side effect: comments now post as github-actions[bot] (the workflow's
own bot identity) rather than as claude[bot] (the Anthropic-managed
GitHub App). Acceptable trade-off, and arguably preferable β the
inline-review identity is now visibly the VyOS workflow rather than
an external bot.
Mirrored byte-identically across rolling/circinus/sagitta + canonical
PR (folded into the still-open VyOS-Networks/vyos-docs-opus-reviewer#17).
|
|
Revert of the id-token drop from #1969. Smoke verify on PR #1977
(empty-commit retrigger run 25658256103) failed at the Pass 2 step:
Action failed with error: Could not fetch an OIDC token. Did you
remember to add `id-token: write` to your workflow permissions?
The earlier Copilot finding that motivated dropping the permission
("no step uses OIDC") was incomplete. No shell step in the workflow
invokes OIDC directly, but anthropics/claude-code-action@v1 itself
calls actions/core's `getIDToken()` internally β likely for the
Claude/Anthropic auth federation path. The required scope is the
third-party action's, not the workflow body's.
Adding id-token: write back with an inline comment block citing the
specific failure mode + the run ID where it was reproduced so this
isn't re-dropped on a future review pass.
Mirrored byte-identically across rolling/circinus/sagitta + canonical.
|
|
CRITICAL BUG discovered during smoke verify of PR #1977 (a draft PR
touching docs/quick-start.md). The prepare job ran SUCCESS but
validate SKIPPED because has_md_changes was false:
$ git diff origin/rolling...HEAD --name-only -- 'docs/**/*.md'
(empty)
$ git diff origin/rolling...HEAD --name-only -- ':(glob)docs/**/*.md'
docs/quick-start.md
Root cause: git's default pathspec syntax uses fnmatch with
FNM_PATHNAME (slashes are not crossed by `*`). The `**` glob is NOT
recognized in default pathspec β it's treated as plain `**`
(two consecutive asterisks). For `docs/**/*.md` this means git
requires at least one `/` between `docs/` and `*.md`, so:
- docs/configuration/service/foo.md β MATCHES (1+ subdir)
- docs/quick-start.md β NO MATCH (0 subdirs)
- docs/index.md β NO MATCH (0 subdirs)
- docs/cli.md β NO MATCH (0 subdirs)
- docs/copyright.md β NO MATCH (0 subdirs)
- docs/coverage.md β NO MATCH (0 subdirs)
- docs/404.md β NO MATCH (0 subdirs)
- docs/documentation.md β NO MATCH (0 subdirs)
7 top-level files silently bypass validation on every PR that
touches them. Adding the `:(glob)` magic prefix opts in to true
shell-style ** semantics (also recognised by git's pathspec
parser per gitignore-pattern rules):
- docs/**/*.md β :(glob)docs/**/*.md
- docs/**/*.rst β :(glob)docs/**/*.rst
Three pathspec usages updated in the prepare job; comment references
to "docs/**/*.md" left unchanged (they read as informal shorthand).
Paired PR on VyOS-Networks/vyos-docs-opus-reviewer adjusts the
canonical scripts/ai-validation.yml identically.
|
|
Copilot finding on #1974/#1975: the inline backtick code span
`[ -s diff-md.patch ] && [ ! -d .reference-db/extracted ]` was split
across two YAML comment lines, which renders poorly in Markdown
viewers (the backtick crosses the line boundary).
Reworded to put the relevant filesystem-presence check (`-d
.reference-db/extracted`) on a single line and drop the redundant
`-s diff-md.patch` half β the latter is the job-level gate
condition, not part of the fail-closed gate logic, so omitting it
from this rationale comment removes the wrap-required spread without
losing meaning. The `-d` check on its own is what defines "the gate
is a presence check, not a schema check".
Mirrored byte-identically across rolling/circinus/sagitta.
|
|
Two small follow-ups now that the DB-pin revert wave (#1971/#1972/#1973
+ canonical #15) has settled and `reviewer-v1.0.2` is tagged:
1. Comment alignment. The fail-closed-gate rationale comment that
landed in #1971/#1972 says "the new code will refuse to load an
older-format DB at the fail-closed gate". A Copilot finding on
#1973 pointed out this is imprecise: the workflow's fail-closed
gate is a presence check (`[ ! -d .reference-db/extracted ]`); it
does not load or inspect the DB. The actual schema-mismatch
protection lives inside the pinned reviewer Python code at
`Pass 1 β deterministic checks` (loaded via
`uv pip install ./reviewer-src` from `env.REVIEWER_REF`). The fix
landed on sagitta (#1973 / b09ea673). This commit ports the same
wording to this branch so the workflow file is byte-identical
across rolling/circinus/sagitta again.
2. REVIEWER_REF bump v1.0.1 -> v1.0.2. The canonical at
`VyOS-Networks/vyos-docs-opus-reviewer/scripts/ai-validation.yml`
already defaults to v1.0.2 since the canonical-sync PR #15 merged
and the `reviewer-v1.0.2` tag was pushed. v1.0.2 is functionally
identical to v1.0.1 β the Python package and `branches.json` are
unchanged; the bump just aligns this deployed copy with the
canonical default for hygiene. No behavioral change.
|
|
Backport of #1971 to this release branch β keeps the workflow file
byte-identical across rolling/circinus/sagitta after the wave of
#1968 / #1969 / #1959 / #1960 / #1970.
Reason (full text in the rolling-side PR description):
PR #1969's `tag: ${{ env.REVIEWER_REF }}` on the reference-DB
download would 404 β the `reviewer-v1.x.x` tag has no GitHub release
with DB assets attached. Reference DBs are published to a separate
release stream (`ref-db-<timestamp>`) by the matrixed rebuild-reference
workflow. Reverting to `latest: true` with an explicit inline comment
block explaining the release-asset topology so this is not re-broken
on a future review pass.
The bug only manifests on a PR with `has_md_changes=true` (i.e. a real
`.md`-changing PR); the recent merges that landed the regression were
all infrastructure-only and never invoked the DB step.
Companion PRs: #1971 (rolling), VyOS-Networks/vyos-docs-opus-reviewer#15
(canonical sync).
|
|
write + fail-closed gate
CodeRabbit / Copilot findings on the merged PR #1959 and the still-open
PRs #1960 + #1969:
1. NUL/control-char guard regressed. PR #1959 (merged into circinus
without this) β and likewise sagitta/rolling-followup β had the
guard inadvertently dropped during the rolling@HEAD rebase, because
rolling@HEAD never had the corrected form. Restoring the
user-corrected form: `LC_ALL=C grep -zPq "[\\x01-\\x1F\\x7F]"`
reads the NUL-delimited *.z files directly (NUL is the record
delimiter, not a forbidden byte) and rejects every other control
byte 0x01-0x1F + 0x7F. An earlier `tr -d "\\0\\n\\r" | grep ...`
form stripped the very bytes it was meant to reject before the
grep ran β see the inline comment block for the contract.
2. `gh pr comment` requires `issues: write`. The skip-notice step and
Pass 2 summary comment both post via `POST /repos/{owner}/{repo}/
issues/{number}/comments` β a PR conversation comment IS an issue
comment in GitHub's data model. With only `pull-requests: write`
the call can 403 on repos whose default GITHUB_TOKEN permission
split routes issue-comment writes through `issues:`. Adding
`issues: write` alongside the existing `pull-requests: write` keeps
every comment path working without expanding the trust surface
beyond what the original validate job needed.
3. Fail-closed gate used `[ -s changed-md.txt ]` (--diff-filter=ACMRT,
excludes deletions) but validate is now gated on `has_md_changes`
which is computed from `[ -s diff-md.patch ]` (unfiltered, deletion-
aware). A deletion-only PR with a missing reference DB would reach
validate (has_md_changes=true) but bypass the fail-closed gate
(changed-md.txt empty), masking the DB-missing condition. Switch
the gate to `[ -s diff-md.patch ]` so both signals agree.
A separate Copilot finding (line 170, "validate gated on has_md_changes
means RST-only PRs do not run") is pushed back on the PR thread as
intentional design β RST is legacy per the RSTβMyST migration, and
the RST bookkeeping in prepare exists to surface mixed-MD-RST PRs in
the Pass 2 prompt, not to drive validation on RST-only PRs.
|
|
guard)
Copilot finding on PR #1959 (line 114): the path-traversal hardening
block used `continue` (skip-with-warning) on detection of absolute /
traversal paths, but the non-regular-tree-entry check below uses
`exit 1`. The asymmetry meant a fork PR that smuggles in a path like
`docs/../../outside.md` would silently bypass Pass 1 (no file copied
into _changed_md/) and Pass 2 (LLM Read/Glob/Grep tools see no
content) β reducing validation coverage on exactly the inputs that
warrant the closest look.
Change `continue` to `exit 1` so both unsafe-input checks have
consistent visible-failure semantics. Maintainers must explicitly
address an offending path rather than have it skipped.
Mirrored byte-identically across all three open workflow PRs.
|
|
pin/no-id-token
PR #1968 merged on rolling while this PR was in CR review. It adds:
* has_md_changes output on prepare + job-level if-gate on validate
(fixes the empty-_changed_md/ crash on infrastructure-only PRs)
* fetch base by refs/heads/<ref> + use FETCH_HEAD instead of
origin/<ref> (fixes the tag-vs-branch ambiguity on vyos/vyos-
documentation where 'rolling' exists as both a branch and a tag)
* skip validate on deletion-only Markdown PRs
This commit pulls in rolling@HEAD's ai-validation.yml verbatim,
then re-applies the 4 still-needed fixes raised by CR/Copilot:
1. SHA-pin actions/checkout@v6 (x4) + actions/upload-artifact@v4 +
actions/download-artifact@v4 + anthropics/claude-code-action@v1
2. Pin reference-DB download to tag: ${{ env.REVIEWER_REF }}
(was latest: true) β aligns DB version with pinned reviewer code
3. Drop id-token: write from validate job permissions (no OIDC use)
Items already present in rolling@HEAD via PR #1968 (no further action
here):
* NUL/control-char rejection in changed-*.z (the corrected
grep -zPq '[\x01-\x1F\x7F]' form, plus the explanatory
comment block about why NUL is excluded from the rejection class)
* Job-level if-gate on validate so infrastructure-only PRs skip the
entire expensive validate chain
* mkdir _changed_md defensive step in validate
Result: 3 PRs (this one + the circinus/sagitta companions) now share a
single byte-identical workflow file that is also a strict superset of
rolling@HEAD's current file (3 fixes layered on top).
|
|
Two factual issues in prior wording (flagged by Copilot on #1969):
- "NUL inside the name" implied embedded NUL is something to reject;
NUL cannot appear in a git pathname (it is the tree-entry terminator).
- "Filesystems β¦ typically reject these" was wrong for LF/CR β POSIX
filesystems allow them and git stores them fine. The actual hazard
is in our line-delimited downstream tooling.
Consolidated the two adjacent comment blocks into one accurate
explanation. No behavior change.
π€ Generated by [robots](https://vyos.io)
|
|
Previous form `tr -d '\0\n\r' | grep -Pq [\x00-\x1F\x7F]` stripped
the very chars (LF, CR) it was meant to catch before the grep ran,
so a path containing newlines or carriage returns slipped through.
`grep -z` keeps NUL as the record delimiter (legitimate separator from
git diff -z) and the pattern excludes 0x00 while catching every other
control byte 0x01-0x1F + 0x7F. LF/CR inside any path now correctly
fail the guard.
Surfaced by Copilot on #1969; applied to all three branch copies
(rolling/circinus/sagitta) so the workflow stays in sync.
π€ Generated by [robots](https://vyos.io)
|
|
The token is used only for read-only repo operations (sparse-checkout
of reviewer branches.json, full checkout of vyos-1x, download of the
reference-DB release asset). Without an explicit permission-* input
the token inherits all installation permissions. Scope it down so a
compromise cannot mutate either repo.
Surfaced by CodeRabbit on #1960; applied to all three branch copies
(rolling via #1969 follow-up + circinus #1959 + sagitta #1960) so the
workflow stays in sync across the version-train branches.
π€ Generated by [robots](https://vyos.io)
|
|
drop id-token
Addresses the four CodeRabbit `Major / Quick win` findings raised
on this PR (after flip to ready):
1. SHA-pin all action invocations (was movable tags `@v6`, `@v4`, `@v1`
on 4 actions). pull_request_target workflows have repo write access
and access to secrets; GitHub security guidance recommends full
commit SHAs as the only immutable release form. Other steps already
were SHA-pinned, so this just resolves the inconsistency:
* actions/checkout@v6 -> @de0fac2e # v6.0.2 (x4)
* actions/upload-artifact@v4 -> @ea165f8d # v4.6.2
* actions/download-artifact@v4 -> @d3f86a10 # v4.3.0
* anthropics/claude-code-action@v1 -> @476e359e # v1.0.119
2. Reject paths containing control characters (NUL/CR/newline/etc) in
changed-md.z and changed-rst.z before `tr '\0' '\n'` converts them
to newline-delimited manifests. A fork PR could in principle commit
`docs/foo<LF>bar.md` and split it into two logical lines, masking
the real file from line-based consumers. The filesystem and
porcelain git typically reject these but the path could survive in
the tree object; fail-fast in the workflow.
3. Pin reference-DB download to `tag: ${{ env.REVIEWER_REF }}` (was
`latest: true`). REVIEWER_REF already fixes the reviewer code to
`reviewer-v1.0.1` on every use; the DB download was the one place
that resolved from the latest release tag, breaking reproducibility
and creating a silent compat-window where a future v1.x.x release
with a DB-schema change could be picked up while pinned reviewer
code still expects the old schema.
4. Drop `id-token: write` from the validate job's permissions. No step
in the workflow uses OIDC; the permission was a copy-paste leftover.
Least-privilege.
Files: same change copied byte-identically to all three workflow PRs
once they land (this PR for circinus, paired PR for sagitta, follow-up
on rolling since #1957 already merged).
|
|
Mirrors bf50e7d1 from PR #1957 β two stale-comment fixes flagged by
Copilot on this PR (review of f02cc04): the 'cp loop' wording and the
'setup-uv on Debian 12' rationale.
Documentation-only; keeps this branch's workflow byte-identical with
rolling's #1957 head.
|
|
Adds .github/workflows/ai-validation.yml on the circinus release branch.
Byte-identical to the version that lands on rolling via #1957.
Background
----------
For pull_request_target events, GitHub Actions evaluates the workflow
file from the repo's DEFAULT branch (rolling), so the rolling workflow
already fires on PRs targeting circinus β that's why phantom queue
entries for "AI Validation" appeared on Mergify backport PRs to
circinus even when this file didn't yet exist on the branch.
Adding the file here is therefore not a behavioral fix; it's
governance/clarity:
* makes the workflow visible to maintainers reading the circinus
branch in isolation,
* preserves the workflow if rolling's copy is ever removed or
renamed,
* allows independent edits per release branch in the future (e.g.
different REVIEWER_REF pin if a release branch needs a frozen
reviewer version).
Runner pool
-----------
`runs-on: ubuntu-latest` on both prepare and validate β see #1957 for
the full rationale (the vyos org has no self-hosted runners labeled
`web`; ubuntu-latest is the only available pool for this repo).
Branches-map and reference DB
-----------------------------
The reviewer's branches.json already maps `circinus β circinus`
(see VyOS-Networks/vyos-docs-opus-reviewer/branches.json on
reviewer-v1.0.1), and the matrixed rebuild-reference workflow already
publishes `reference-db-circinus.tar.gz` as part of each rebuild. No
reviewer-side change needed.
|
