fix: report a missing ref once, and audit the two repositories it never saw - #5
Merged
Conversation
…er saw
One deleted branch produced eight findings. `gh` prints the JSON error body to
stdout on a non-2xx, and `wfs()` returned that body as though it were an answer:
FAIL LotteryApp/frontend no required status checks
FAIL LotteryApp/frontend {"message":"No no permissions block
FAIL LotteryApp/frontend commit no permissions block
FAIL LotteryApp/frontend found no permissions block
FAIL LotteryApp/frontend for no permissions block
FAIL LotteryApp/frontend the no permissions block
FAIL LotteryApp/frontend ref no permissions block
FAIL LotteryApp/frontend frontend","documentation_url":"...","status":"404"} no permissions block
The unquoted `$(wfs ...)` split `{"message":"No commit found for the ref
frontend",...}` into seven words, and each word was audited as a workflow file
— printing a fragment of the 404 where a file name belongs.
This is the rule in AUDIT.md inverted. A check that cannot run must not look
like a check that passed; here a check that could not run looked like seven
that failed, and buried the one fact worth reporting — that a configured
branch is gone — under six impostors and a piece of JSON.
The fix is to read the exit status rather than the text. `api()` captures the
output and returns non-zero when gh does, so an error can no longer be mistaken
for data. Callers then say what actually happened:
- a branch is checked for existence once, up front, and its absence reported
as the configuration problem it is, rather than rediscovered by every check
that reads the ref
- a workflow listing that fails says so, instead of yielding fake file names
- a workflow that cannot be read says so, instead of being audited as empty
and failing the permissions check on that basis
- file names are read a line at a time, so word splitting cannot invent them,
and a name containing a space cannot either
LotteryApp:frontend stays on the list. It now reports as one clear line —
"branch not found - restore it, or drop it from BRANCHES" — which is the
decision to make, stated once, by the tool whose job is to notice.
The secrets sweep had the same read: a 404 body could not contain a secret
name, so an unreadable branch quietly pushed every secret toward "used
nowhere". It now skips a branch it cannot list instead of counting it as
evidence of absence.
Targets: DevSecOpsSentinel and WidgetWorks are added to REPOS and BRANCHES.
Both are public, both deploy from main, and neither has ever been audited —
the weekly run has been reporting on five repositories while saying nothing
about the two on the front of the profile. Ten deploying branches now.
Also gives rt() its own temporaries. b, s and p are the branch loop's names
too; nothing leaks today only because rt is called inside $( ), which is a
thin guarantee to leave standing.
Historical statements are left as they are: README, FINDINGS and HARDENING
say "four repositories" about the sitting that produced this, and that is
still what happened.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VrPrR9jZDHewD5dKR5dyRk
The branch was deleted; its frontend moved into main as lottery-web/, which `git ls-tree HEAD` on main confirms. So the entry was not a broken reference waiting to be restored — it named a deploying branch that no longer deploys, because there is nothing left on it to deploy. BRANCHES lists deploying branches, and LotteryApp now deploys from one. The repository stays under audit; only the second pair goes. Nine deploying branches, not ten. The branch-not-found check added in the previous commit stays, and is what turned this from eight lines of nonsense into one line naming the decision. It earns its place on the next deletion rather than this one. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VrPrR9jZDHewD5dKR5dyRk
…ings
Two checks were answering questions they had not asked.
**Untrusted input matched the cure as well as the disease.** The check grepped
the whole workflow file, so an expression was a finding wherever it appeared.
Putting `${{ github.event.pull_request.base.sha }}` in an `env:` block is the
recommended mitigation — the value reaches the script as a quoted variable and
is never expanded into script text — and it was reported identically to an
expression interpolated straight into a `run:` body, which is the injection.
That is how DevSecOpsSentinel/main ci.yml came back flagged. It passes its SHAs
through `env:` and carries a comment above them explaining that it does so
precisely because direct expansion is the pattern this project exists to
discourage. The audit read the comment's subject and filed the finding anyway.
`runbody()` now emits only the lines inside `run:` block scalars, and the check
reads those. Verified against the file that produced the finding: two matches
whole-file, zero in run bodies, while a synthetic workflow interpolating
`github.event.issue.title` inside `run:` is still caught.
Match structure, not text — the same rule the pull_request_target check was
rewritten under, after a comment that merely mentioned it produced a finding.
The rule was written down and the neighbouring check never got it.
**A repository the token cannot read is one finding, not three.** Every
repo-level check reads through the same token. Without Administration: Read
they all return 404, and the code turned each 404 into a finding: WidgetWorks
was reported as having private vulnerability reporting off and no
dependency-review workflow, when neither question had been answered. Both may
be true. Nothing in that run was evidence either way.
The security-settings check already reported this honestly. Now it stops there
for that repository, so the output names the one thing to fix instead of
burying it under guesses about what the fix might reveal.
Same rule as the missing-jq run and the missing ref: a check that could not run
must never be reported as a check that ran — in either direction.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VrPrR9jZDHewD5dKR5dyRk
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
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.
What happened
The weekly run failed on 24 Aug with
ToDoApp 4 open code-scanning/alertsand opened #4. That finding is now stale — ToDoApp#140 cleared those four Trivy alerts by refreshing the pinned base-image digests, and ToDoApp does not appear in today's output at all.A manual run today (#14) came back red for a different reason:
LotteryAppis down to a single branch —git ls-remote --headsreturns onlymain— soLotteryApp:frontendinBRANCHESpoints at a ref that no longer exists.Why one missing branch became eight findings
ghprints the JSON error body to stdout on a non-2xx and exits non-zero.wfs()discarded only stderr, so the 404 body became its return value:{"message":"No commit found for the ref frontend","documentation_url":"...","status":"404"}for f in $(wfs "$r" "$b")then word-split that into seven tokens, and each token was audited as a workflow file — each failing thepermissions:grep, each filing its own failure, one of them printing a fragment of the 404 where a file name belongs.This is the rule in
AUDIT.mdinverted. A check that cannot run must not look like a check that passed; here a check that could not run looked like seven that failed, and buried the one fact worth reporting — a configured branch is gone — under six impostors and a piece of JSON. Same family as the missing-jqrun in the README that read twelve empty answers as twelve disabled controls.The fix
Read the exit status, not just the text.
api()captures output and returns non-zero whenghdoes, so an error can no longer be mistaken for data. Callers then report what actually happened:LotteryApp:frontendstays on the list, as requested. It now reports as one line:Which is the decision to make, stated once, by the tool whose job is to notice.
Targets
DevSecOpsSentinelandWidgetWorksare added toREPOSandBRANCHES. Both are public, both deploy frommain, and neither has ever been audited — the weekly run has been reporting on five repositories while saying nothing about the two at the top of the profile. Ten deploying branches now, up from eight.Expect this PR's audit run to be red. Two repositories are being looked at for the first time, plus the LotteryApp line above. That backlog is the point of adding them; it is not a regression from this change.
Also
rt()now uses its own temporaries.b,sandpare the branch loop's names too — nothing leaks today only becausertis called inside$( ), which is a thin guarantee to leave standing.Left alone deliberately
README.md:24,docs/FINDINGS.mdanddocs/HARDENING.mdsay "four repositories" about the sitting that produced this repository. That is history, and still true.docs/PIPELINE.md:3says "across all five repositories". It now documents five of the seven under audit — writing up the two new pipelines is a larger job than this PR, flagging rather than silently rewriting.Verification
bash -n scripts/audit.shpasses; mode stays100755.shellcheckis not installed in my environment — the Lint job runs it.scripts/audit.sh, so the Weekly audit workflow runs on it — that run is the real test of the new output.Closes #4 once green.
Generated by Claude Code