| Age | Commit message (Collapse) | Author |
|---|
|
|
|
pipeline (#2209)
* fix: IS-572: correct verified defects in the Cloudflare Workers docs pipeline
Fixes raised against the LTS port PR but landing on rolling first, so the
backport runs in the right direction, plus one defect found while verifying
the nightly canary sweep.
- apex PDF: a 206 is now only answered to a request that actually carried a
Range header, and every R2Range shape is normalized to concrete bounds.
R2 reports a whole-object range on un-ranged gets, so keying the 206 off
obj.range alone made every plain GET of the 1.3 PDF a 206.
- apex PDF: a failed If-Match now maps to 412, not 304.
- apex sitemap index: entries derive their origin from the request, so the
canary index no longer points at production.
- UA policy: drop the dead Google-Extended entry (a robots.txt token, never
a UA) and log Applebot-Extended while still allowing Applebot, via a
most-specific-match precedence between the allow and log lists.
- docs-build: replace the check_head guard's exit 78 (neutral only in the
retired Actions v1 runtime) with an output gating every deploy/promote
step, so a superseded run ends green-and-skipped instead of red.
- smoke/parity: read CF Access service-token credentials from the
environment instead of argv, and pass them via env: in both workflows.
- smoke: stop asserting X-Docs-Build on the PDF probe — apex's R2 fallback
path legitimately never sets it, making the probe unpassable for 1.3.
- gates/smoke: strip critical-pages lines before testing for comments.
- parity: fetch each sitemap once, honouring the _SCHEME override.
Advances: IS-572
* fix: IS-572: harden check_head guard against a failed ls-remote
Phase-0 CodeRabbit findings on the new guard, both valid:
- set -euo pipefail + an explicit emptiness check, so a failed git ls-remote
cannot leave $remote empty and be misread as 'branch moved' (cut exits 0
on empty input) — that would silently skip a deploy that should have run.
- pass github.ref_name and github.sha through env vars rather than
interpolating them into the shell command.
Advances: IS-572
* fix: IS-572: address adversarial + CodeRabbit review on the CF docs gates
Answers the seven adversarial-review findings on vyos/vyos-documentation#2209,
five of which GitHub CodeRabbit independently reproduced on review 4946100889.
Range handling (apex Worker) — the reported defect was real but mis-diagnosed.
Probing a real R2 binding under vitest-pool-workers shows R2 never returns the
inverted/zero-length range the review predicted: for the Headers form this
Worker uses, R2 answers every unsatisfiable, malformed, multi-range and
unknown-unit Range by IGNORING it and returning the complete object with
range={offset:0,length:size} — indistinguishable from a satisfied whole-object
range, and it does not throw. (Only the object-literal form throws, code 10039,
which this Worker cannot reach.) Trusting obj.range therefore answered
`Range: bytes=99-` with 206 + `Content-Range: bytes 0-8/9` and the whole body,
so a client resuming mid-download would silently corrupt its file. Range intent
is now re-derived from the client's own header per RFC 9110 §14.1.2:
unsatisfiable → 416 + `Content-Range: bytes */size`, ignored → 200 with the
complete body, satisfiable → 206 as before. resolveRange() additionally clamps
its length at zero so its documented contract holds for every R2Range input.
Preconditions: reorder to RFC 9110 §13.2.2's MUST-ordered precedence
(If-Match, If-Unmodified-Since, If-None-Match, If-Modified-Since) so
`If-Match: "old"` + `If-None-Match: "new"` yields 412 rather than 304, and gate
304 on GET/HEAD per §13.1.2 so a conditional POST cannot be answered 304.
UA gate: a log match now wins a contest against an allow match unless the allow
entry strictly contains the log entry, so `GPTBot/1.0 DuckDuckBot` emits its
ua-log event instead of being swallowed by the longer allow needle. Applebot /
Applebot-Extended verdicts are unchanged and now pinned by tests.
parity.py: restore the exact-200 sitemap check (_OPENER returns for any 2xx, so
a 204 yielded an empty corpus and the gate passed having probed nothing) and
route the sitemap request through a single credential-attaching constructor, so
--sitemap-host against the Access-gated canary no longer 403s on every sitemap.
Secrets: --access-id/--access-secret removed from both gates in favour of the
paired env vars. An argv value is readable from the process table and captured
by `set -x`; no call site used the flags. smoke.py reads --critical-list via
Path.read_text() rather than leaking a file handle.
Tests: 115 -> 138 worker tests, 46 -> 50 gate tests. Every new behavioural test
was verified to fail against the pre-fix source.
Advances: IS-572
* test: IS-572: compare the forbidden UA token case-insensitively
bestMatch() lowercases every ua-policy.json entry before matching, so a
"google-extended" entry would be functionally identical to the "Google-Extended"
token this guard exists to keep out. toContain() compares primitives by strict
equality, so the lowercase variant passed the assertion and would have quietly
restored the entry.
Lowercase both sides so the assertion matches the matcher's own case semantics.
Verified by injecting a lowercase "google-extended" into the log list: the test
now fails, and passed before this change.
Advances: IS-572
* fix: IS-572: scope the CF Access token per host; correct range + precondition semantics
Round-3 adversarial review findings. Each fix has a test that fails against 5b4f40f9.
parity.py sent the CF Access service token to every host the run touched, which
pre-cutover includes --sitemap-host docs.vyos.io — still served by ReadTheDocs.
Credentials are now modelled as an Access record bound to --probe-host, and
build_request() attaches them per destination rather than per run, so the token
reaches only the host it belongs to while the post-cutover same-host
configuration stays credentialed.
The apex Worker's range handling assumed its own regex agreed with R2's parser.
It does not: R2 accepts ASCII space only, this one accepts \s, so a
tab-separated Range parsed here, was ignored there, and produced a 206 carrying
the whole object in answer to a request for three bytes. classifyRangeHeader now
returns the concrete bounds it derived and the Worker checks them against the
bytes R2 actually returned before promising a 206 — any parser divergence
degrades to a 200 instead of corrupting a resumed download. Range positions are
compared as digit strings, so specs above 2^53 no longer collapse into each
other, and Range is ignored outright on every method but GET (§14.2).
If-Range was silently dropped: R2Conditional carries no such validator, so a
stale range request was answered with bytes of the new representation under a
206. It is now evaluated against the object's own ETag, with a failed validator
serving the complete representation.
Preconditions were inferred from which headers were present, which cannot be
right in both directions — If-Match satisfied plus If-None-Match satisfied owed
a 304 and got a 412. The conditionals are filtered to those §13.2.2 applies to
this request before R2 sees them, and a body-less result is resolved by
evaluating the validators in §13.2.2 order.
Also drops the UA gate's narrow-allow carve-out. It compared the two matched
policy entries rather than the UA's token structure, so a request-controlled UA
quoting both tokens bought itself an allow. No pair in ua-policy.json used it.
🤖 Generated by [robots](https://vyos.io)
* fix: IS-572: cancel abandoned R2 body streams; drop stale --access-* comments
CodeRabbit round-3 follow-up.
R2 hands back a body on two paths whose response carries none, and dropping the
reference leaves the stream open holding its connection until GC. CodeRabbit
flagged the If-Range re-read; the larger instance is the 416, where R2 answers an
unsatisfiable Range with the COMPLETE object — 29.2 MiB for the 1.3 PDF — and the
response sends none of it. Both now go through discardBody(), which cancels so
the transfer is aborted rather than drained, and swallows failures because the
response on those paths is already decided.
Keeping the get-then-re-read rather than switching to head()-then-get(): a head()
would avoid opening the slice stream, but it would add a round-trip to the path
where If-Range MATCHES — the normal resumed download — to tidy the rare path
where it does not, and it would open a TOCTOU window that the current shape does
not have, since here the object whose validator was checked is the very response
served.
The R2 test mock returned string bodies, which is why this class of bug was
invisible to it. Bodies are now real ReadableStreams that record their own
cancellation, so the three new tests observe the actual stream lifecycle: the 416
cancels the whole object, the stale If-Range cancels the discarded slice, and a
served body is never cancelled.
Also corrects two comments that still described --access-id/--access-secret as a
manual fallback; the flags were removed in round 2 and argparse rejects them. A
repo-wide sweep found no other prose describing the removed interface. The
docs-build.yml change is one comment line inside an env: block; check_head is
untouched.
🤖 Generated by [robots](https://vyos.io)
* fix: IS-572: compare Access origins; ignore conditionals per method
Round-4 adversarial review findings.
parity.py: _authority() compared (hostname, port) verbatim, so `p.invalid`,
`p.invalid:443` and the trailing-dot FQDN form were three different origins.
Post-cutover both --sitemap-host and --probe-host name the same Access-gated
host; spelling the default port out on either cost the sitemap fetch its token
and 403'd it, reverting the same-host keeper case. Compare normalized origins
instead: lowercased host, single trailing dot stripped, the scheme's default
port folded to None (bare arguments read under _SCHEME). IDN/punycode
equivalence stays out of scope -- both hosts are ASCII literals passed by CI.
apex worker: applicablePreconditions() filtered conditionals by validator
applicability but not by method, so RFC 9110 13.2.1 -- "a server MUST ignore
the conditional request header fields ... such as CONNECT, OPTIONS, or TRACE"
-- was unmet: an OPTIONS carrying a stale If-Match was refused 412 where the
same request without it succeeded. Take the method and return an empty set for
those three; R2 is then given nothing to evaluate.
apex worker: the stale-If-Range re-read was a bare get(), dropping the other
preconditions, so a key rewritten between the two reads was served under a 200
though the request's If-Match excluded it. The legacy snapshot deploy's
force_pdf_refresh input does rewrite this key. Carry the same onlyIf into the
re-read and resolve a body-less result through preconditionStatus() on the new
object's validators.
Advances: IS-572
* fix: IS-572: scope the Access token by origin, not authority; hide the secret from repr
Round-5 CodeRabbit findings, both on the CF Access credential primitive in
scripts/docs_gates/parity.py.
_authority() returned (hostname, port) and dropped the scheme, so an https
credential also applied to the plaintext http URL of the same name --
Access("p.invalid", ...).applies_to("http://p.invalid/") was True, and
build_request() would then have attached CF-Access-Client-Id/-Secret to a
cleartext request. `http://p.invalid:80/` is the sharp case: 80 folds to None
under http, matching the https-scoped ("p.invalid", None) exactly.
Not reachable in current callers: every URL in the module is built from
_SCHEME and both workflow invocations pass bare hosts, so a single run is
single-scheme and the http value exists only for the test monkeypatch. Fixed
as a contract defect regardless -- applies_to() documents itself as comparing
ORIGINs, an origin is scheme+host+port, and over-broad credential scope is the
failure mode this primitive already produced once (round 2, token sent to
docs.vyos.io). Return (scheme, host, port), resolving a bare argument's scheme
under _SCHEME as before, and fold the default port against the origin's own
scheme so http :80 and https :443 stay distinct rather than collapsing
together. The four origin-normalization cases and the :8443 denial are
unchanged.
Access.client_secret is now dataclasses.field(repr=False). The default
dataclass repr rendered every field, so a failed assertion, a debug print or
any exception interpolating an Access would have put the service token into CI
output, which is durable and world-readable for this repo. The client id stays
in repr: it names which token without being the credential, and dropping it
would make a scoping failure much harder to diagnose. The secret remains
readable as a field, which is the only way it is ever consumed.
Swept the rest of scripts/docs_gates/ for the same exposure: smoke.Probe holds
no credential fields, smoke passes the id/secret as plain locals that reach no
output path, and the str(e) texts recorded in parity-report.json and the
SMOKE-FAIL lines carry urllib's code/reason/URL, never request headers.
Two tests added, both verified failing against the pre-fix source.
Advances: IS-572
🤖 Generated by [robots](https://vyos.io)
* fix: IS-572: roll back to the newest deployment's 100%-traffic version id
`wrangler deployments list --json` sorts ASCENDING by created_on
(wrangler 4.123.0 versionsDeploymentsListHandler), so `.[0]` selected the
OLDEST deployment, not the current one the stale comment claimed. It also
took the deployment `id`, but `wrangler rollback` resolves its positional
through fetchVersion() and needs a VERSION id.
Select newest-first by created_on and take the version_id serving 100% of
traffic, walking to the next-newest deployment when traffic is split across
versions -- mirroring wrangler's own fetchDefaultRollbackVersionId(), minus
its .shift() (it picks after deploying; we capture before). Non-array or
unparseable output now fails safe to an empty id instead of a wrong one.
Rewrite the stale comment to describe the actual JSON shape, note in the
consumer that rollback_id is a version id, and add --message for rollback
audit context (supported at 4.123.0; --yes supplies it non-interactively).
Advances: IS-572
🤖 Generated by [robots](https://vyos.io)
* fix: IS-572: re-check the branch tip after smoke; harden rollback capture
Two production-safety fixes in docs-build.yml.
1. TOCTOU on the check_head guard. It resolved the branch tip ONCE, before the
candidate deploy and before the smoke gate (480s deadline), then gated
PROMOTE, the post-promote probe and the registry pointer publish on that
minutes-old answer. A push landing in that window left the run promoting a
superseded SHA to production and publishing a pointer naming it.
`cancel-in-progress: false` does not cover this: it makes the SECOND run
queue and skip, while the run already past the check sails on.
Add a second guard after smoke, immediately before the first
production-facing step, and move PROMOTE + probe + pointer publish onto its
output. The probe must move with PROMOTE: left on the first check it would
run after a skipped promote, see production serving the previous SHA, read
that as a promote failure and roll back a production nobody touched.
A branch that moved mid-run ends GREEN-and-skipped, matching the convention
the exit-78 redesign established: production and the registry are untouched,
and the queued run promotes the new tip, so there is nothing for an operator
to act on. The residual check-to-deploy window is one step boundary; no
compare-and-swap exists on the Cloudflare side to close it entirely.
2. The rollback-capture bootstrap branch swallowed every failure. `2>/dev/null`
plus a bare `if` could not tell "Worker does not exist yet" from auth,
network or malformed output, so a transient failure yielded an empty
rollback_id, logged "nothing to roll back to" and promoted with the
auto-rollback silently disarmed while the run looked healthy.
Only a genuine not-found may now produce an empty id. wrangler 4.123.0 exits
1 on every error path and writes nothing machine-readable to stdout when the
command fails, so the discriminator is the API error code it renders into
stderr: 10007/10090, the pair its own isWorkerNotFoundError() uses. Auth
carries 9106/10000 and connectivity failures carry no code, so neither can be
mistaken for bootstrap. Anything unmatched fails the step. Non-array output
now fails instead of being coerced to []; an empty list and a split-traffic
list are reported distinctly (notice / warning) rather than sharing one
ambiguous message with the failure paths.
The code pair was read out of the shipped 4.123.0 bundle, not observed
against a live missing Worker; the comment says so and asks for re-verification
on wrangler bumps.
Also corrects two comments the change falsifies: the concurrency note claiming
queued runs are covered by "the check_head guard", and the exit-78 note claiming
promote/registry gate on the first check.
Advances: IS-572
🤖 Generated by [robots](https://vyos.io)
* fix: IS-572: run the post-promote probe whenever the deploy succeeded
PROMOTE ran `wrangler deploy` and then the hostname purge, the tarball and
three `r2 object put` calls in one step, while the post-promote probe named no
status function in its `if:` and so carried an implicit success(). A transient
purge 5xx, a tar error or one failed upload therefore failed PROMOTE, skipped
the probe entirely, and left the new version LIVE on the production Worker,
never verified and never rolled back.
Split the step at the deploy. Everything fallible after it moves to a new
`promote_publish` step gated on `steps.promote.outcome == 'success'`, and the
probe becomes the verification finalizer: `!cancelled() && steps.promote.outcome
== 'success'`. `!cancelled()` counts as a status function, which is what
suppresses the implicit success() and lets the probe run after a failed
purge/upload; it is not `always()`, because firing an auto-rollback while an
operator tears the run down is the wrong reflex. Gating on PROMOTE's outcome
rather than re-reading check_head_2 is strictly stronger: 'skipped' on the
moved-branch path, 'success' only once the production deploy completed. The
registry-pointer publish keeps its bare condition, whose implicit success() is
now load-bearing — it is what stops a pointer from naming a rolled-back
generation.
Rollback-target capture no longer reaches backwards. The old jq flattened
`versions[]` across ALL deployments and took the first 100%-traffic id found
scanning newest→oldest, so a current deployment deliberately serving A/B at
90/10 resolved to an older deployment's version — and a probe failure would
"roll back" to it and destroy the split. It now inspects only the current
deployment and refuses otherwise. The verdict is discriminated inside jq so a
malformed record fails red instead of masquerading as split traffic:
`select(.percentage == 100) | .version_id` on `{"percentage":100}` yielded null,
`null // empty` yielded empty, and empty was read as "split" — an unparseable
API response promoting with the auto-rollback silently disarmed. Non-numeric
`percentage`, empty/absent `versions` and unsortable `created_on` are rejected
for the same reason.
The hostname purge is retried once. It is the one post-deploy failure with an
asymmetric knock-on: the probe measures the edge, so a purge that never lands
makes a healthy deploy look like a failed promote.
Finally, the apex PDF range-divergence fallback no longer answers with a 206
carrying bounds the client never requested (RFC 9110 §14.4). It discards the
slice and fails 503, keeping the r2-range-divergence log line that is the actual
alarm. Unreachable under today's workerd, which ignores multi-range.
Advances: IS-572
🤖 Generated by [robots](https://vyos.io)
* ci: IS-572: bind DOCS_CF_LIVE via env: in the purge/registry step
The purge branch expanded ${{ vars.DOCS_CF_LIVE }} directly into the shell
body while the other three consumers of the same value bind it through env:
(PDF carry-forward, candidate reset, post-promote probe). zizmor flags the
inline form as template expansion; the value is repo-controlled so it is not
exploitable, but the env form removes the class and matches the surrounding
steps.
* ci: IS-572: validate DOCS_CF_LIVE, pin wrangler, bound rollback percentages
Three Major findings from CodeRabbit review 4971096766, all outside-diff.
DOCS_CF_LIVE failed open. Four steps branch on it — the SKIP_PDF carry-forward
staleness check, the candidate-reset staleness check, the production hostname
purge and the post-promote probe — and every one compares against the literal
string "true", so an unset or misspelled repository variable silently took the
branch that skips the check, skips the purge and skips the probe, after which
the registry pointer published an unpurged, never-verified deployment on a green
run. Validate the value once at the top of the only job that reads it, so all
four comparisons are safe by construction instead of each re-deriving the guard.
`npx wrangler` in the previous-build metadata fetch ran before `npm ci`, so on
the normal (non-SKIP_PDF) path it downloaded whatever version the registry
served rather than the lockfile-pinned 4.123.0 every later step uses. Move the
install ahead of the first wrangler invocation and make it unconditional; the
inline `npm ci` in the SKIP_PDF branch and the conditional install step after
the check_head guard are both redundant once it is.
The rollback-target jq required `percentage` to be numeric but not in range, so
a current deployment reporting both 100 and 101 yielded exactly one 100%-traffic
match and classified as a determinate `ok` — arming a rollback with a version
selected from data already known to be nonsense. Out-of-range now routes to the
malformed path, never to `split:`.
Advances: IS-572
* ci: IS-572: validate rollback_id at capture, bind it via env at the finalizer
rollback_id originates from Cloudflare's API (`wrangler deployments list
--json`), so it is untrusted text. It was written to $GITHUB_OUTPUT
unvalidated and then reached shell source through a template expansion at
three sites in the post-promote finalizer.
The $GITHUB_OUTPUT write is the sharper of the two exposures: a value
containing a newline injects arbitrary ADDITIONAL step outputs regardless
of how carefully the consumer quotes, because no shell is involved. Validate
at capture — the only place that can stop it.
The character class is deliberately wider than the observed format. wrangler
4.123.0 performs no client-side validation of a version id (it interpolates
the string straight into the API URL path), so the canonical UUID shape is
what Cloudflare's bundled SDK documents rather than something the CLI
enforces. Hard-failing on anything but a UUID would turn a Cloudflare format
change into a blocked promote; hard-failing outside [A-Za-z0-9._-] cannot,
and still certainly excludes newlines, quotes, whitespace and every shell
metacharacter. A safe-but-non-UUID value warns instead of erroring.
Empty stays legitimate — it is the no-target state of all three benign paths
(first deploy, no deployments, split traffic) and the finalizer's -z branch
depends on it. A malformed non-empty value fails loudly, matching how the
surrounding capture already treats malformed API data.
At the finalizer, ROLLBACK_ID is bound through the step's env: mapping and
the three sites use "$ROLLBACK_ID", so the value is never substituted into
the shell source. The -z semantics are unchanged.
Advances: IS-572
* ci: IS-572: verify probe status, reconcile a failed pointer publish
Two merge blockers from the pre-merge adversarial gate on d079a2c9.
1. The post-promote probe accepted an error response as verification.
`withDocsHeaders` sets X-Docs-Build unconditionally, so error responses
carry it too; the probe read only that header and never the status line,
so a production answering 500 with the new SHA satisfied the match and
the registry pointer published for a generation serving errors.
The probe now parses the status line and requires an exact 200 alongside
the built SHA. Parsing resets on every status line, so 1xx interim
responses contribute nothing and only the final response block is read;
duplicate X-Docs-Build headers with identical values are accepted while
conflicting ones surface as "<conflicting>" and can never match. Requests
are bounded with --max-time 20 and the pipeline runs under pipefail so a
transport failure is distinguishable from a missing header. Early-attempt
non-200s and transport failures stay retryable — only the final attempt's
verdict decides. Every unverified outcome still rolls back, but the three
causes now carry distinguishable diagnoses.
--fail is deliberately not used: it collapses 4xx/5xx into curl exit 22
and discards the status line, which is the value needed to tell a broken
new version apart from a transport failure.
2. Production and the registry could diverge permanently. The round-8
promote/publish split correctly decoupled a publish failure from the
probe, but added no convergence action for the case where the probe then
succeeds: production served the new SHA while latest.json still named the
old one, with nothing to reconcile it. latest.json is the source of truth
for the candidate-reset staleness check and the SKIP_PDF carry-forward, so
both would later restore a generation production no longer serves.
A reconciliation step now runs when the promote and probe both succeeded
but the pointer did not publish. It converges FORWARD — re-uploading the
sha-scoped generation and only then publishing the pointer, so the pointer
can never name an incomplete generation — and on failure leaves the
pointer untouched and reports the divergence by name with both SHAs and a
copy-pasteable manual repair. Gating on the probe's success keeps it off
the rollback paths entirely, where production and the pointer already
agree. The pointer step's load-bearing implicit success() is unchanged.
Refs: IS-572
* ci: IS-572: drop registry-pointer reconciliation, keep the probe fix
Scope reduction. The `Reconcile registry pointer after a failed publish`
step introduced in the previous commit is removed; the post-promote probe
rewrite in that same commit stays.
Removing the step returns registry-divergence behaviour to what `rolling`
has today: when `promote_publish` fails after a successful deploy, the
pointer step's load-bearing implicit success() skips it, and production can
end up serving a SHA that `$slug/latest.json` does not name. That path is
now documented at the `Publish registry pointer` step and tracked in
https://vyos.dev/T9237 rather than repaired here.
Also reverts the DOCS_CF_LIVE header comment to "four later steps", which
restores its internal agreement with the "four comparisons downstream"
sentence in the same block.
Advances: IS-572
🤖 Generated by [robots](https://vyos.io)
|
|
* T9220: ci: upload JUnit test results to Codecov Test Analytics
Emit vitest JUnit XML (console reporter preserved via --reporter=default)
and add a pinned codecov-action@v5.5.5 test-results upload step
(tokenless, informational, continue-on-error). Test job semantics for the
deploy gate are unchanged. Codecov Test Analytics fleet rollout, T1 batch.
🤖 Generated by [robots](https://vyos.io)
* T9220: chore: ignore vitest JUnit output dir in workers
Also satisfies the workflow's workers/** path filter so this PR's CI run
exercises the new test-results upload (acceptance evidence).
🤖 Generated by [robots](https://vyos.io)
* T9220: ci: compare head repo identity, not fork flag (review finding)
🤖 Generated by [robots](https://vyos.io)
|
|
(#2201)
🤖 Generated by [robots](https://vyos.io)
|
|
* T9208: add scutum to docs CI branch enumerations
The scutum docs branch (VyOS 1.6 train) was cut from circinus on
2026-08-13. Register it in the branch enumerations that gate CI on the
default branch:
- context7-refresh.yml: add scutum to the push-trigger branch list, the
workflow_dispatch choice options and the defence-in-depth variant
allow-list, so a docs push on scutum refreshes its own Context7
variant instead of being dropped.
- context7.json: add a `scutum` entry to `previousVersions`. Context7
registers branch variants from this file on the default branch; a
refresh with `branch: "scutum"` 404s until that registration exists.
Enumerations only — no behaviour change for the existing rolling,
circinus and sagitta variants.
🤖 Generated by [robots](https://vyos.io)
* T9208: add scutum to the context7 branch-to-version mapping rule
The previousVersions entry registers the scutum variant, but the
branch-to-version mapping rule Context7 feeds to the model still jumped
straight from rolling to circinus and claimed rolling covers "1.6+" —
the version scutum now serves. Add the scutum row and drop the stale
version claim from rolling so the rule matches the variant list.
Caught by adversarial review on 2b10f78.
|
|
* docs: upgrade vendored DataTables 1.11.5 -> 2.3.8
Swap the vendored single-component DataTables downloader build from 1.11.5
to 2.3.8 (docs/_static/js/datatables.js, docs/_static/css/datatables.css).
The only init site, docs/_static/js/tables.js, uses the option-less
jQuery-style $('#id').DataTable() form, which is unchanged in 2.x, so
neither it nor docs/_templates/layout.html needed edits. jQuery 3.6.0 is
still injected ahead of the bundle by sphinxcontrib-jquery and satisfies
2.x.
Remove docs/_static/css/DataTables-1.11.5/ (10 sort-icon images). 2.x draws
sort indicators in pure CSS: the new datatables.css contains no url()
references at all, and the replaced 1.11.5 stylesheet was the only thing in
the repo that referenced those files.
Migrate custom selectors for the 2.x generated-class renames, each verified
against the downloaded bundle rather than assumed:
- .dataTables_info -> .dt-info (tables.css, text.css)
- .paginate_button -> .dt-paging-button (tables.css, text.css)
- #coverage a.paginate_button{,.current,.next,.previous}
-> #coverage button.dt-paging-button{...}
2.x renders paging controls as <button> (was <a> in 1.x), so the old
rules failed on both element type and class. The .current/.next/
.previous modifiers are still emitted and are retained.
The #table-cfgcmd_wrapper / #table-opcmd_wrapper rules are deliberately
untouched: only the wrapper *class* changed (dataTables_wrapper ->
dt-container); the element id is still <tableId>_wrapper.
Drop the CodeQL exclusion (.github/codeql/codeql-config.yml, plus the
optional codeql-cfg-path input in .github/workflows/codeql.yml). That
config existed solely to paths-ignore the 1.11.5 bundle, which tripped 7
alerts (5x js/incomplete-multi-character-sanitization, 2x
js/incomplete-sanitization). Upstream 2.x hardened the implicated helpers -
_stripHtml now runs a do/while fixpoint over the <script> strip, and
_escapeHtml uses all-global regexes - so those patterns are not expected to
recur. Removing the exclusion restores default CodeQL coverage of the file;
this PR's own CodeQL run is the empirical test.
Provenance (DataTables downloader builder, styling "DataTables default",
component set dt only - the 2.x equivalent of the previous #dt/dt-1.11.5):
https://cdn.datatables.net/v/dt/dt-2.3.8/datatables.js
sha256 184fb4bd0b9a81a955acd608ba94d0643c74271e78c0fae30f1f40b824f88b1d
https://cdn.datatables.net/v/dt/dt-2.3.8/datatables.css
sha256 e37677437e0fbe4a463aafc83bc4ea8d60986b72050ba9f47f740b41148184ae
Verified with a local make html: build succeeded, and coverage.html renders
both tables (table-cfgcmd 9095 rows, table-opcmd 3036 rows) with the 2.3.8
assets injected in the correct order after jQuery.
🤖 Generated by [robots](https://vyos.io)
* docs: simplify paging-button hover background to one declaration
The hover rule carried 'background-color: #E1E4E5 !important' followed by
'background: none'. The important longhand outranks the later normal-importance
shorthand, so the gray hover fill did apply — but the pair is confusing and
CodeRabbit read it as a dead declaration. Collapse to a single
'background: #E1E4E5 !important', which produces the identical effective state
(gray fill, upstream 2.x hover gradient suppressed) in one declaration.
🤖 Generated by [robots](https://vyos.io)
* docs: re-target mobile search-input margin at the 2.x sibling DOM
DataTables 1.x nested the search <input> inside its <label>; 2.x renders them
as siblings inside div.dt-search, so the narrow-viewport descendant rule
'label input' no longer matched and the 10px top margin was silently lost
below 576px. Target '.dt-search input' instead. Flagged independently by the
implementation pass and both adversarial reviewers (Codex + agy).
🤖 Generated by [robots](https://vyos.io)
|
|
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)
|
|
* security: remediate CodeQL code-scanning alerts (picker XSS sinks, test sanitization, vendored DataTables exclusion)
Remediates all 11 open CodeQL alerts on the default branch:
- version-picker.js (js/xss-through-dom, alerts 1-3): percent-encode every
DOM-derived path component (select.value, parsed location segments) at URL
construction time via encodePath()/langUrlFor(), and tighten the
parseLocation slug charset to [A-Za-z0-9._-]. No-op on legitimate sphinx
slugs — URLs stay byte-identical (asserted by tests).
- workers/apex/test/manifest.test.ts (js/incomplete-multi-character-
sanitization, alert 6): strip HTML comments from the root.html fixture
repeatedly to a fixpoint instead of a single pass.
- docs/_static/js/datatables.js (alerts 4,5,7-11): excluded from CodeQL
analysis via .github/codeql/codeql-config.yml (new codeql-cfg-path input
to the fleet reusable workflow). The file is vendored stock DataTables
1.11.5; the flagged helpers are display/sort normalization, not
sanitization boundaries. Excluding keeps the vendored copy byte-identical
to upstream instead of hand-patching it.
Adds 9 picker tests (hostile-input encoding + slug-charset accept/reject);
workers suite 103/103 green.
🤖 Generated by [robots](https://vyos.io)
* security: normalize pre-existing percent escapes in encodePath
Adversarial-review finding (Codex, medium): location.pathname returns
well-formed escapes verbatim, so blind encodeURIComponent double-encoded
them (%2E -> %252E), broke the HEAD probe on escaped deep links, and
dumped the user at the version root. Each segment is now decoded first
(malformed escapes keep the raw segment — no throw), then re-encoded to
canonical single encoding. Decoding cannot resurrect dot-segments:
the URL parser resolves '.'/'..' and their percent-encoded forms during
navigation, so pathname never presents them (verified against the WHATWG
parser in Node).
workers suite 106/106 (+2 regression tests, mutation-verified).
🤖 Generated by [robots](https://vyos.io)
* security: normalize percent escapes per run, not per segment
Round-2 adversarial finding (Codex, medium): whole-segment decode meant
one malformed escape (a%20b%zz) threw for the segment and double-encoded
the valid escapes beside it. encodeSegment now decodes+re-encodes each
well-formed %HH run independently; literal spans (including a bare '%')
always pass through encodeURIComponent, so taint neutralization holds
unconditionally; a run decoding to invalid UTF-8 stays verbatim (already
pure %HH text).
workers suite 108/108 (+2 discriminating regression tests).
🤖 Generated by [robots](https://vyos.io)
|
|
🤖 Generated by [robots](https://vyos.io)
|
|
Phase-0 CodeRabbit finding on the PR-trigger commit: github.head_ref is
not unique across forks, so two PRs from different forks with the same
branch name would share a concurrency group. Key on
github.event.pull_request.number instead.
🤖 Generated by [robots](https://vyos.io)
|
|
apex-deploy.yml's test job only triggered on push to rolling, so no
PR-level check ran the workers/ vitest suite — that's how the vitest
3.2/pool-workers 0.18.4 peer conflict landed on rolling undetected
(two dependabot PRs merged clean since nothing exercised `npm ci` in
workers/ pre-merge).
Add a `pull_request` trigger scoped to `workers/**` so the test job
runs pre-merge. Gate deploy-canary and deploy-production with
`if: github.event_name != 'pull_request'` so PR runs test only —
no canary/production deploys fire on a PR (deploy-production would
also auto-skip via `needs: deploy-canary` once canary skips, but the
explicit `if:` keeps intent visible). Concurrency group now branches
per-event so PR runs queue/cancel independently of the rolling deploy
group instead of colliding with it.
Validated with actionlint + a YAML parse check.
🤖 Generated by [robots](https://vyos.io)
|
|
🤖 Generated by [robots](https://vyos.io)
|
|
🤖 Generated by [robots](https://vyos.io)
|
|
pdflatex hard-errors on the Devanagari etymology text in
docs/introducing/history.md ("! LaTeX Error: Unicode character व
(U+0935) not set up for use with LaTeX") and separately cannot embed
several pre-existing .webp images (no BoundingBox) — both previously
undiscovered because the Devanagari error always halted the build
first. latexmk -f (force mode) + pdflatex -interaction=nonstopmode
makes the build tolerate both classes of per-glyph/per-image failure
and finish, matching how ReadTheDocs has always built this project's
PDF (verified against the live docs.vyos.io PDF: same Devanagari
glyphs blanked, same ~4 images embedded out of 2000+ pages).
latexmk exits non-zero in force mode even on a fully-produced PDF, so
the workflow step now verifies the artifact itself (exists, >2MB)
instead of relying on the command's exit code.
🤖 Generated by [robots](https://vyos.io)
|
|
previews) (#2140)
* docs-infra: scaffold Cloudflare workers workspace (versions.json v2, matrix, toolchain)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: record full Phase-0 plan decision in workers/PLAN.md
🤖 Generated by [robots](https://vyos.io)
* docs-infra: shared content worker — asset serving, cache classes, X-Docs-Build, canary no-store
🤖 Generated by [robots](https://vyos.io)
* docs-infra: run worker script before assets; test fetch entrypoint
🤖 Generated by [robots](https://vyos.io)
* docs-infra: apex manifest loader + dispatch map + runtime binding guard (TDD)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: apex redirects (aliases, PDF, trailing-slash) + special paths (TDD)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: PDF redirect honors pdf:null and preserves query
🤖 Generated by [robots](https://vyos.io)
* docs-infra: apex UA gate — allowlist-wins, log-only AI crawlers, empty block list at launch
🤖 Generated by [robots](https://vyos.io)
* docs-infra: apex router (pipeline §3.2), themed 404/503, /kb seam, env configs + congruence test
🤖 Generated by [robots](https://vyos.io)
* docs-infra: add missing-User-Agent regression test for apex UA gate
🤖 Generated by [robots](https://vyos.io)
* docs-infra: R2-streaming preview worker — MIME map, noindex, no-store (TDD)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: preview 404 no-store + fetch handler tests
🤖 Generated by [robots](https://vyos.io)
* docs-infra: bootstrap script — binding-target workers must exist before apex deploys
🤖 Generated by [robots](https://vyos.io)
* docs-infra: apex run_worker_first, lockfile for npm ci, PDF Location from manifest
🤖 Generated by [robots](https://vyos.io)
* docs-infra: derive html_baseurl from DOCS_VERSION_SLUG with RTD fallback (canonical gate prereq)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: version picker + status banner + language scaffold (vanilla JS, TDD pure core)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: picker preserves query+hash across switch; valid breadcrumb markup
🤖 Generated by [robots](https://vyos.io)
* docs-infra: Pagefind search wrapper with runtime base-path + preview prefix handling (TDD)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: pagefind wrapper — asset-failure notice + UI stylesheet load
🤖 Generated by [robots](https://vyos.io)
* docs-infra: gate Pagefind searchbox to CF builds (RTD keeps stock search until cutover)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: deploy sanity gates — limits, critical pages, count-delta, canonical (TDD)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: hermetic gate tests via fixture versions.json
🤖 Generated by [robots](https://vyos.io)
* docs-infra: docs-build workflow — candidate/smoke/promote two-stage deploy + registry + rollback
Two-stage CF Workers pipeline: build in pinned container, assemble artifact,
sanity gates, deploy candidate, scoped pre-traffic smoke via canary apex,
promote (rollback-id capture, hostname purge, registry upload), post-promote
probe + auto-rollback. DOCS_CF_LIVE repo variable gates every docs.vyos.io
production interaction pre-cutover.
scripts/docs_gates/smoke.py adds one authorized check beyond the spec: the
version's index.html probe asserts the #vyos-search mount div is present in
the response body, guarding CI silently forgetting DOCS_VERSION_SLUG (which
would otherwise ship stock RTD search without the Pagefind gate noticing).
🤖 Generated by [robots](https://vyos.io)
* docs-infra: build docs image in-workflow with buildx cache (v4.1 — digest pin dropped)
Plan v4.1 amendment: the ghcr.io digest-pinned image does not exist (workflow
would hard-fail at the first docker step on every push). Replace the BUILD_IMAGE
env placeholder with an in-workflow docker build from docker/Dockerfile via
docker/setup-buildx-action@v3 + docker/build-push-action@v6 (context: docker/,
load: true, tags: docs-build:local, GHA cache from/to). The checked-out commit
is the pin; buildx GHA cache keeps repeat builds cheap. Sphinx-build step swaps
to docs-build:local; inner script unchanged.
🤖 Generated by [robots](https://vyos.io)
* docs-infra: apex/preview deploy workflow — canary auto, production behind environment approval
* docs-infra: apex-deploy concurrency guard (per-ref, cancel-in-progress)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: fork-safe PR preview pipeline — approval record, R2 prefixes, label consumption, cleanup
* docs-infra: nightly preview sweep — pipefail + per-prefix failure isolation
🤖 Generated by [robots](https://vyos.io)
* docs-infra: nightly canary QA — per-entry sweep + URL-parity corpus vs RTD
🤖 Generated by [robots](https://vyos.io)
* docs-infra: parity sweep scoped to CF-built versions; transport-error resilience
🤖 Generated by [robots](https://vyos.io)
* docs-infra: one-off bootstrap workflow (binding targets — runs once on this push)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: remove one-off bootstrap workflow (bootstrap complete)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: one-off canary apex + preview deploy (route targets for Task 3.6 step 2c)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: remove one-off canary deploy workflow (targets live)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: address Phase-0 CodeRabbit findings (canonical gate, error caching, registry pointer, validation)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: strengthen manifest tests (full dispatch iteration, mutation-free validate)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: address GitHub CodeRabbit review (pointer-after-probe, fail-closed sweeps, block-precedence UA gate, preview hardening)
🤖 Generated by [robots](https://vyos.io)
* docs-infra: adversarial review fixes — error no-store, probe retry, PR-list membership, preview dotted-segment
🤖 Generated by [robots](https://vyos.io)
* docs-infra: serve oversized legacy PDF from R2 via apex (spec §5 fallback)
The 1.3 PDF (29.2 MiB) exceeds the 25 MiB static-asset cap and is absent
from the legacy content Worker's build, so /_/downloads/en/1.3/pdf/ (and
the picker's PDF link) 301'd into a dead-end 404 post-cutover. Add the R2
object fallback spec §5 already documented but never implemented: a
DOCS_PDFS R2 bucket binding on the apex Worker, a manifest pdf_r2_key
field (1.3 only), and a router step ahead of version dispatch that streams
the object with its own cache class (canary/error still force no-store).
🤖 Generated by [robots](https://vyos.io)
* docs-infra: PDF R2 fallback honors Range + If-None-Match, preserves ETag
🤖 Generated by [robots](https://vyos.io)
|
|
* ci: track Context7 LTS variants by branch instead of moved git tags
Switch the Context7 1.5/1.4 variants from tag-backed to branch-backed:
- context7.json: previousVersions entries `{tag:"1.5"}`/`{tag:"1.4"}` →
`{branch:"circinus"}`/`{branch:"sagitta"}`. Context7 now indexes the
LTS branch HEADs directly.
- Delete update-version-tags.yml. Its sole purpose was force-moving the
git tags 1.5/1.4 to circinus/sagitta HEAD so the tag-backed Context7
variants tracked the branches. With branch-backed variants Context7
tracks the HEAD natively, so the tag indirection — and its re-run
"moves the tag backward" footgun — is gone.
- context7-refresh.yml: trigger directly on push to rolling/circinus/
sagitta (docs/** + context7.json paths) instead of chaining off the
deleted tag-mover via workflow_run; refresh payload uses
`branch:"circinus"`/`branch:"sagitta"` for the non-default variants.
Operator follow-up (post-merge): the circinus/sagitta branch variants
must be registered in the Context7 dashboard (a refresh by branch 404s
until Context7 crawls the new config), and the now-unused git tags
1.5/1.4 can be deleted.
🤖 Generated by [robots](https://vyos.io)
* ci: harden jq repo interpolation in context7-refresh
Adversarial review (agy): use the $GITHUB_REPOSITORY runner env var
(mapped as REPO) instead of inlining ${{ github.repository }} into the
jq command string, matching the safe env-mapping pattern already used
for EVENT_NAME/REF_NAME. github.repository is validated and low-risk,
but this structurally removes the interpolation-into-shell surface.
🤖 Generated by [robots](https://vyos.io)
|
|
Removes the eps1lon `check-open-prs-conflict.yml` caller (which ran
`eps1lon/actions-label-merge-conflict` on push to base branches to label
conflicting PRs + comment) and replaces it with Mergify:
- Conflict LABEL (`conflicts`): already handled by the inherited central
`Label conflicting pull requests` rule (predicate `conflict`, toggle).
Mergify re-evaluates continuously, so the GHA's push-triggered re-scan
is covered natively — no local rule needed for the label.
- Conflict COMMENT: a local additive rule `Comment on conflicting pull
requests` (distinct name → does not replace the inherited label rule)
posts the contributor-facing "please resolve" notice once when a PR
enters conflict (Mergify dedupes the comment per rule+message).
The GHA's clean-side "conflicts resolved" comment is intentionally NOT
replicated — Mergify has no "was-conflicting-now-clean" predicate, and
keying on `-conflict` would match every non-conflicting PR.
This is the final piece of the T8937 Mergify-GHA retirement (the central
`check-pr-conflict.yml` reusable was preserved pending this consumer; it
is deleted in a follow-up vyos/.github PR once this merges).
Advances: T8937
🤖 Generated by [robots](https://vyos.io)
|
|
centrally) (#2083)
|
|
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)
|
|
Rewrites uses: pins to the three HIGH-fanout producers (vyos/.github,
vyos/vyos-cla-signatures, VyOS-Networks/vyos-reusable-workflows) from their
old default branch to the new production compat branch staged in Task 1.
No functional change; pin-ref rewrite only.
Tracking: T8943
|
|
Replaces 5 caller workflows now superseded by central Mergify rules.
See https://vyos.dev/T8937 for the design + spec + plan.
Advances: T8937
🤖 Generated by [robots](https://vyos.io)
|
|
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)
|
|
The mirror caller has been disabled and the target mirror repo at
VyOS-Networks/vyos-documentation has been removed. Drop the workflow
file so the repo state reflects "this repo does not mirror" without
relying on a dangling disabled workflow.
|
|
|
|
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)
|
|
ci(doc-linter): fix 12 accumulated bugs flagged across PR #2014/#2019/#2020 reviews
|
|
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)
|
|
Each `uses:` line was pinned to a mutable version tag (`@v6`,
`@v0.8.4`, …). Tags can be rewritten to point to malicious code —
CVE-2025-30066 (reviewdog/action-setup) and the tj-actions/changed-files
incident in 2025 are the canonical real-world examples. GitHub's
hardening guide for Actions recommends pinning to full-length commit
SHAs and keeping the tag as a trailing comment for human readability.
Resolved each action's tag to its commit SHA via `gh api
/repos/<repo>/git/refs/tags/<tag>` and verified the SHA is a commit
(not an annotated-tag object) via `gh api
/repos/<repo>/git/commits/<sha>`:
- actions/checkout v6 -> de0fac2e4500dabe0009e67214ff5f5447ce83dd
- bullfrogsec/bullfrog v0.8.4 -> 1831f79cce8ad602eef14d2163873f27081ebfb3
- trilom/file-changes-action v1.2.4 -> a6ca26c14274c33b15e6499323aac178af06ad4b
- actions/setup-python v6 -> a309ff8b426b58ec0e2a45f0f869d46889d02405
This change covers `lint-doc.yml` only. A fleet-wide sweep across
every workflow in `.github/workflows/` is a separate effort —
worth doing because the drift / supply-chain risk is the same in
every one. Tracked as a follow-up to this PR's review.
Tracked as item 11 of the rolling-side cleanup backlog from PR #2014
/ #2019 / #2020 reviews.
🤖 Generated by [robots](https://vyos.io)
|
|
Previous workflow:
env:
FILES_MODIFIED: ${{ steps.file_changes.outputs.files_modified }}
run: python scripts/doc-linter.py "$FILES_MODIFIED"
`trilom/file-changes-action`'s `files_modified` output is
modifications-only. A PR adding a new `.md`/`.rst` doc page passed
`files_added`, never `files_modified`, so a brand-new page with long
lines or real public IPs slipped past the linter entirely.
Workflow: also pass `files_added` and `files_renamed` as separate
positional args. Each output is a JSON array (action v1.2.4) and is
passed via env to avoid shell-quoting issues.
Linter: `main()` now accepts one OR multiple positional argv entries,
each a JSON array of paths. Arrays are merged and deduplicated before
linting. Single-arg invocations remain backward-compatible. Switched
from `ast.literal_eval` to `json.loads` — the action's outputs are
JSON, and `json.loads` is the right tool (and dodges
literal_eval-via-`eval`-substring linter warnings).
Test coverage:
- Two JSON arrays merge -> single linter run on union.
- Empty-string argv entry skipped (no `files_renamed` in many PRs).
- Malformed JSON -> falls back to walking DOCS_ROOT.
- No argv -> walks DOCS_ROOT.
- Single-arg invocation -> backward-compat preserved.
Tracked as item 7 of the rolling-side cleanup backlog from PR #2014 /
#2019 / #2020 reviews.
🤖 Generated by [robots](https://vyos.io)
|
|
The override shipped in 3d44fe06 used rule names `Flag PR title not
matching T-ID format` + `Flag commit message not matching T-ID format`.
Per Mergify `extends:` semantics, same-name rules in the child REPLACE
the parent's — and the central baseline in vyos/mergify uses a single
rule named `Flag T-ID format violation in PR title or commit messages`.
Different names → no replacement → parent rule keeps firing.
Symptom (surfaced on PR #2014, title `docs(readme): …`): every PR
without a `T<digits>:` token gets `invalid-title` labeled.
Fix: collapse to one rule, rename to match the parent exactly. The
condition (`closed AND -closed`) still never matches, so `toggle`
removes the label on every evaluation — picks up any stale
`invalid-title` from the broken-override window.
The parent rule does not toggle `invalid-commit-title`; the
second child rule for that label was extraneous and is dropped.
🤖 Generated by [robots](https://vyos.io)
|
|
The central baseline at vyos/mergify:.mergify.yml enforces a
`T<digits>: <text>` (optional `scope: ` prefix) format on PR titles
and every commit's first line — a convention from vyos/vyos-1x and
sibling code repos where engineering work tracks against a Phorge
task.
vyos-documentation has never enforced this. Recent merged PRs use
conventional-commits style (`fix(ext): …`, `ci(...): …`,
`chore(deps): …`) without an upstream T-ID. After the
`extends: mergify` adoption in #2005, the inherited rules
started labeling every such PR `invalid-title` and
`invalid-commit-title` — pure noise for this repo.
Override the two rules by name (Mergify `extends:` semantics:
same-name rules in the child REPLACE the parent's). Conditions are
constructed to never match (`closed and -closed`), so the `toggle`
action always falls into the "remove label" branch and also clears
the labels from any PR that picked them up since #2005 merged.
🤖 Generated by [robots](https://vyos.io)
|
|
Adds .github/mergify.yml that inherits from vyos/mergify:.mergify.yml.
The central baseline provides:
- defaults.actions.backport.ignore_conflicts: false (new in T8782 —
makes backport conflicts fail loudly instead of committing literal
`<<<<<<< / ======= / >>>>>>>` markers; see incident below)
- pull_request_rules → label conflicting PRs with `conflicts`
- commands_restrictions → restrict @Mergifyio slash commands to
@vyos/maintainers + vyosbot
Why this repo, why now: 2026-05-12, PR #1994 was backported via
`@Mergifyio backport sagitta circinus`. Cherry-pick conflicted in
docs/conf.py; Mergify's pre-T8782 default (`ignore_conflicts: true`)
committed the markers, the resulting PRs (#1998 / #1999) were merged
anyway, and Read the Docs builds on both branches broke with
`SyntaxError: invalid syntax (conf.py, line 309)`. Fix-forwards
#2000 (circinus) and #2001 (sagitta) restored both branches. Adopting
the central baseline closes the gap for this repo so the same failure
mode cannot recur via the Mergify-managed backport path.
Spec: https://vyos.atlassian.net/wiki/spaces/VYOS/pages/849477640
IS-432: https://vyos.atlassian.net/browse/IS-432
🤖 Generated by [robots](https://vyos.io)
|
|
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)
|
|
|
|
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.
|
|
vyos/yuriy/ai-validation-claude-action-github-token-rolling
ci(ai-validation): pass github_token to claude-code-action (skip OIDC exchange)
|
|
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).
|
|
Add `paths:` filter to `Update version tags` push trigger so infra-only
pushes (workflows, scripts, Docker, README, etc.) skip the tag move and
therefore skip the downstream Context7 refresh. Context7 only ingests
`docs/**` (per context7.json `folders`), so refreshing on infra commits
wasted API calls and gave no benefit. `context7.json` is also covered
because changes to its `rules`/`folders`/`excludeFolders` affect what
Context7 returns even when no .md files moved.
The version tag's only consumer is Context7, so skipping the move on
infra-only pushes is semantically correct — the tag still represents
the latest docs state. `workflow_dispatch` on context7-refresh.yml
remains available to force a refresh out of band.
🤖 Generated by [robots](https://vyos.io)
|
|
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.
|
|
CRITICAL REGRESSION fix. PR #1969 changed the reference-DB download
from `latest: true` to `tag: ${{ env.REVIEWER_REF }}` (set to
`reviewer-v1.0.1`). Verification post-merge showed that:
- The `reviewer-v1.0.1` tag exists but has **no GitHub release**
attached:
$ gh api /repos/VyOS-Networks/vyos-docs-opus-reviewer/releases/tags/reviewer-v1.0.1
{"message": "Not Found", "status": "404"}
- Reference DBs are published to a separate release stream by the
matrixed `rebuild-reference` workflow, tagged
`ref-db-<UTC-timestamp>` (e.g. `ref-db-20260510-193946`). The
reviewer Python package and the reference DBs have independent
release cadences.
`robinraju/release-downloader` with `tag: reviewer-v1.0.1` therefore
404s. The download step has `continue-on-error: true` so the run
proceeds, but the fail-closed gate at "Fail if reference DB missing
while MyST files are in the diff" then errors every validate run on a
PR with MD changes.
Our own recent test PRs (#1947 / #1956 / #1957 / #1968 / #1969 / #1970
on rolling, plus the circinus/sagitta backports) all happened to be
infrastructure-only — `has_md_changes` was `false`, validate was
skipped at the job level, and the DB step was never invoked. The
regression was therefore invisible until the next real `.md`-changing
PR would have failed red.
Reverting to `latest: true` is the correct behavior. The reference DB
schema is intentionally backwards-compatible across reviewer Python
releases; the freshest DB is always usable. If the schema ever
changes incompatibly, the way to opt out is bump `REVIEWER_REF` and
the new reviewer code will refuse to load an older DB at the
fail-closed gate — _exactly_ what we have today, just one level up.
A paired PR on VyOS-Networks/vyos-docs-opus-reviewer/scripts/
ai-validation.yml ([#15](https://github.com/VyOS-Networks/vyos-docs-opus-reviewer/pull/15))
applies the same revert + bumps REVIEWER_REF default to v1.0.2 so
canonical and deployed stay aligned after the next reviewer tag.
A separate companion PR will backport this revert to circinus and
sagitta to keep the workflow file byte-identical across all three
release branches.
|
|
ci: refresh Context7 LTS variants via tag field, not branch field
|
|
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.
|
|
CodeRabbit review on #1962 flagged that concurrency.group is evaluated
at workflow scheduling time, BEFORE the shell-level allowlist case in
the job runs. An API-triggered workflow_dispatch with arbitrary variant
value (e.g. 'gh workflow run -f variant=foobar') would produce
concurrency key 'context7-refresh-foobar' and bypass dedupe with
legitimate runs.
Gate inputs.variant in the expression against the same allowlist
('rolling'/'1.5'/'1.4'). Map head_branch values to their canonical
variant names so the workflow_run path also resolves cleanly. Fall
through to 'invalid' if neither path matches — the shell allowlist
then fails the run loud.
🤖 Generated by [robots](https://vyos.io)
|
