fix(ci): a Verify-Filter matching zero artifacts must not pass the gate - #342
Open
avrabe wants to merge 5 commits into
Open
fix(ci): a Verify-Filter matching zero artifacts must not pass the gate#342avrabe wants to merge 5 commits into
avrabe wants to merge 5 commits into
Conversation
The verification gate is a REQUIRED check, and it could be silenced by a one-word change to the PR body. `scripts/run-falcon-verification.py` exits 0 when its filter matches nothing — correctly, in isolation: no artifact ran, so none failed. But the gate's verdict is exactly that exit code (`steps.verify.outcome`), so a `Verify-Filter:` that matches zero artifacts turns a required safety gate green while verifying nothing. It even renders "✅ Rivet verification gate — 0/0 passed" into the sticky PR comment, so the deception is not visible to a reviewer skimming it. The PR body is attacker-controllable. The INJECTION risk was already handled (env-bound, quoted, never interpolated into run:) — but the SEMANTIC trust of the filter was not: an untrusted string decides how much verification happens, and "none" was an accepted answer. Caught empirically, not by review: `(has-tag "traceability")` reads perfectly plausible and matches 0 of the 195 sw-verification artifacts, because that tag lives on sw-reqs, not on verifications. I had put exactly that filter on #341. Fix: after the sweep, fail if the script reports `0 artifact(s) matched`. Verified locally: (has-tag "traceability") -> 0 artifacts -> guard fires (has-tag "oci") -> 1 artifact -> no false positive (has-tag "falcon") -> 156 artifacts (the default) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG
avrabe
enabled auto-merge (squash)
August 7, 2026 19:05
avrabe
added a commit
that referenced
this pull request
Aug 8, 2026
…349) The demo binds a FIXED UDP port pair (default 14700/14701). Several of these jobs can land on the SAME self-hosted runner concurrently, and the second one dies: error: bind 127.0.0.1:14700: Address already in use (os error 98) error: bind 127.0.0.1:14701: Address already in use (os error 98) Observed 2026-08-08 after four PRs were refreshed together. It is not rare — it happens whenever more than one PR is updated at once, which is routine when branch protection is strict and every merge puts the rest BEHIND. WHY IT IS WORTH FIXING RATHER THAN RE-RUNNING: it surfaces as a red REQUIRED check on a PR that cannot possibly have caused it — #342 changed a single workflow file — so it reads as a real regression and costs a diagnosis cycle. Worse, the "fix" that works is hitting re-run, which is exactly the habit that gets a genuine failure clicked through. The script ALREADY honours FALCON_HELLO_PORT_BASE (scripts/falcon-hello-demo.sh line 28). CI simply never set it. Derive a per-run base from GITHUB_RUN_ID. Stepping by 2 is load-bearing: the demo uses base AND base+1, so consecutive bases would still overlap on one port. base = 20000 + (GITHUB_RUN_ID % 10000) * 2 -> 20000..39998 pair = base, base+1 -> 20000/20001 .. 39998/39999 consecutive run ids differ by 2 -> no shared port GITHUB_RUN_ID is injected by Actions as a plain env var, so nothing is interpolated into the run: block. Verified locally end-to-end, not merely by reading the script: $ FALCON_HELLO_PORT_BASE=20246 bash scripts/falcon-hello-demo.sh [falcon-hello-demo] launching gcs on 127.0.0.1:20246 exit=0 Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…350) The sweep has grown into its own ceiling. Measured across one day, same 156-artifact sweep: #340 ~63m success #344 1h30m21s FAILURE — killed at timeout-minutes: 90 #344 (earlier) cancelled #342 failed twice the same way #343 success twice Variance now exceeds headroom, so a REQUIRED check is decided by runner load rather than by whether anything is wrong. And a timeout surfaces as `fail`, indistinguishable from a real failure unless you read the duration — which already cost a diagnosis cycle. Worst of all, it was blocking the two PRs meant to improve this gate (#342 fail-open fix, #343 sweep speedup). Raising the ceiling does not weaken the gate: it is being KILLED, not failing. Landing it here because this PR already owns this file and is itself blocked by the timeout it fixes. THIS IS A STOPGAP and the third reactive bump (60 -> 90 -> 150). The trend is the real problem: 45m in July, >=90m now. #350 tracks it, and names the next measurement — the compile-vs-test split ON the runner. The sweep spans 64 crates with 29 `--release` steps, so compilation is the likely dominant cost; whole-crate -> named conversion (#343/#262) is right for traceability but, measured, will not close a 30-minute gap on its own (the non-falcon-core offenders run in 0-5s). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG
…ation at column 0
Self-inflicted, and it broke the very gate the guard was meant to strengthen.
The guard's error message used a backslash-continued string whose continuation
lines started at COLUMN 0:
echo "::error::Verify-Filter matched 0 ... — \
refusing to pass a gate that verified nothing. Fix the filter or drop the \
override to use the default."
Inside a `run: |` block scalar, a line at column 0 TERMINATES the block. The
whole workflow file was therefore invalid YAML from the moment I added the
guard, which is almost certainly why this PR's verification gate failed twice —
NOT the 90-minute timeout I attributed it to in the previous commit.
`python3 -c "yaml.safe_load(...)"` reports:
ScannerError: while scanning a simple key, line 234 ... could not find
expected ':'
Fixed by keeping the message on one line, with a comment saying why, so the
next person does not "tidy" it back into a continuation.
Lesson worth the comment: I verified this file for the thing I was thinking
about (no `${{ }}` inside `run:` — which I DID check, with a parser) and not
for whether it still parsed at all. Both checks now run.
YAML parses: OK, timeout-minutes=150
${{ }} inside run: blocks: none
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A one-word change to a PR body could silence a required verification gate.
The hole
The gate’s verdict is
steps.verify.outcome— the exit code ofscripts/run-falcon-verification.py. That script exits 0 when its filtermatches nothing, which is correct in isolation: no artifact ran, so none failed.
So a
Verify-Filter:matching zero artifacts turns the required check greenwhile verifying nothing — and renders this into the sticky PR comment:
A reviewer skimming that sees a green check.
The PR body is attacker-controllable. The injection risk was already handled
(env-bound, quoted, never interpolated into
run:). What was missing is thesemantic trust: an untrusted string decided how much verification happens,
and "none" was an accepted answer.
How it was caught
Not by review — empirically. I put
Verify-Filter: (has-tag "traceability")on#341. It reads perfectly plausible. It matches 0 of 195 sw-verification
artifacts, because
traceabilityis a tag on sw-reqs, not on verifications.The fix
Fail the step when the script reports
0 artifact(s) matched.Evidence (local, no CI needed)
(has-tag "traceability")(has-tag "oci")(has-tag "falcon")(default)Notes
${{ }}is expanded inside anyrun:block; the filter stays env-bound.run_full=false, no verification-relevant files) isuntouched — that one legitimately verifies nothing and stays green.
full 156-artifact sweep. That sweep is slow enough to have read as a timeout
before — tracked separately in rivet hygiene: backfill release: fields + convert whole-crate verification steps to named tests #262.
dispatch until Actions recovers.
🤖 Generated with Claude Code
https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG