fix(ci): the advisory guard fails OPEN on an API error -- move the || outside (#1209)

[wshallwshall]

Guardrail #2 reads count="$(gh api ... || echo "ERR")". The || runs inside the command substitution, so it APPENDS to stdout rather than replacing it — and gh api copies the JSON error body to stdout on any HTTP error (only its gh: ... (HTTP nnn) line goes to stderr, which 2>/dev/null eats).

So count held {"message":"API rate limit exceeded",...}ERR. The sentinel missed, being neither "ERR" nor empty. [ "$count" -lt 1 ] then failed with "integer expression expected" and returned 2 — and an if condition is exempt from set -e. The step printed "published advisory confirmed", wrote advisory_ok=true, and exited 0.

The guard inverted to fail-open on exactly the rate-limit/API-error class its own header promises routes to manual review. The comment directly above it says "Fail closed on any error" — a compensating control resting on a false premise.

Measured, against the real shipped body

origin/main vs this branch, run under bash -e with a gh stub reproducing the stream split:

                  gh ERRORS          gh returns 1
pre-fix (main)    advisory_ok=true   advisory_ok=true     <- FAIL OPEN
fixed             advisory_ok=false  advisory_ok=true     <- closed, happy path intact

A stub that merely exited non-zero would have passed against the defective code and proved nothing. The defect is that the body reached the variable, so the stub must write a body.

Bounded today, and the bound is worth stating rather than leaning on

The merge condition also requires age_ok, and the age step denies every ecosystem that can be eligible — a disjointness this file documents about itself. So a falsely-true advisory_ok cannot alone merge anything as shipped. But the file also labels that conjunct "a FORWARD guard ... load-bearing the day a Python allow row is populated": one line's edit from making this directly merge-affecting. The vault's copy has no such conjunct and is fixed in its own PR.

The existing test could not see it

test_ghsa_step_queries_the_advisory_api_and_emits_a_guard asserted the string "advisory_ok=false" in body — satisfied by a step that merely contains the words, and the fail-open lived underneath a passing version of exactly that check.

The file already had the right instrument: _run_step_body executes shipped run: bodies under bash -e and returns the parsed $GITHUB_OUTPUT. Guardrail #2 was the one guard not using it. It now does, across three executed rows including a discriminating pass, so the suite cannot be satisfied by a step that denies unconditionally.

Those rows are gated on jq, so they run on the ubuntu and the two required windows legs and skip on a box without it. Locally I verified the same behaviour by driving the extracted body with a jq stub, since this machine has no jq.

Domain, because fixing one instance is how this class survives

A sweep of 63 workflow and script files across both repositories found 24 instances of the idiom; 16 are provably harmless — git rev-parse --verify --quiet writes nothing when the ref is absent. All six in this file are fixed.

Moving the || outside also fixes the streaming cases for free: jq emits rows before a mid-array error, and the old form would have appended the sentinel to a truncated dependency list while still reporting success. Assigning on failure discards partial output instead of inheriting it.

The count guard additionally moves from equality against one sentinel to a shape test (case "$count" in ""|*[!0-9]*)). An equality test recognises exactly the failure it was told about, which is how a JSON body walked through it; the numeric comparison's real question is "is this a number", and only a shape test answers that for values nobody anticipated.

Backlog entry #1209 lands in this commit.