From a82538e316df9109b046b190c9a6e6cc8d66b693 Mon Sep 17 00:00:00 2001 From: Burt Gardner Date: Tue, 25 Aug 2026 12:19:59 +0000 Subject: [PATCH 1/3] fix: report a missing ref once, and audit the two repositories it never saw MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01VrPrR9jZDHewD5dKR5dyRk --- README.md | 4 +++- scripts/audit.sh | 59 ++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index c935505..3b79a89 100644 --- a/README.md +++ b/README.md @@ -56,9 +56,11 @@ Configuration is a statement of intent. Only the attempt is evidence. | [ToDoApp](https://github.com/bgard68/ToDoApp) | .NET 10 API and React frontend — JWT auth, refresh-token revocation, Google sign-in | | [LotteryApp](https://github.com/bgard68/LotteryApp) | .NET API with an Angular frontend | | [Net10Sudoku](https://github.com/bgard68/Net10Sudoku) | Blazor Interactive Server sudoku generator and solver | +| [DevSecOpsSentinel](https://github.com/bgard68/DevSecOpsSentinel) | GitHub Actions supply-chain analyzer — deterministic rules, model-written explanations | +| [WidgetWorks](https://github.com/bgard68/WidgetWorks) | E-commerce store — rotating refresh tokens, TOTP 2FA, server-side re-priced checkout | | **this repository** | audits itself, on the same checklist | -Eight deploying branches between them. Several of these projects deploy from +Ten deploying branches between them. Several of these projects deploy from more than one branch, which is why the audit takes `repo:branch` pairs rather than assuming the default. diff --git a/scripts/audit.sh b/scripts/audit.sh index d916ef1..efdb6fc 100755 --- a/scripts/audit.sh +++ b/scripts/audit.sh @@ -43,8 +43,8 @@ OWNER="${OWNER:-bgard68}" # is the failure this exists to prevent, in miniature: the checklist was written # because "is it secure?" kept being answered from memory, and a list with a # hole in it where the auditor sits is the same gap wearing a different hat. -REPOS="${REPOS:-ClaudeChessApp ToDoApp LotteryApp Net10Sudoku devsecops-audit}" -BRANCHES="${BRANCHES:-ClaudeChessApp:main ToDoApp:main ToDoApp:dapper ToDoApp:frontend LotteryApp:main LotteryApp:frontend Net10Sudoku:main devsecops-audit:main}" +REPOS="${REPOS:-ClaudeChessApp ToDoApp LotteryApp Net10Sudoku DevSecOpsSentinel WidgetWorks devsecops-audit}" +BRANCHES="${BRANCHES:-ClaudeChessApp:main ToDoApp:main ToDoApp:dapper ToDoApp:frontend LotteryApp:main LotteryApp:frontend Net10Sudoku:main DevSecOpsSentinel:main WidgetWorks:main devsecops-audit:main}" case "${1:-}" in -h|--help) sed -n '2,/^$/p' "$0" | sed 's/^# \{0,1\}//'; exit 0 ;; esac @@ -52,10 +52,25 @@ esac fails=0 note() { echo " FAIL $*"; fails=$((fails+1)); } -wf() { gh api "repos/${OWNER}/$1/contents/.github/workflows/$3?ref=$2" --jq '.content' 2>/dev/null | tr -d '\n' | base64 -d 2>/dev/null; } -wfs() { gh api "repos/${OWNER}/$1/contents/.github/workflows?ref=$2" --jq '.[].name' 2>/dev/null; } -rt() { b=$(echo "$1"|cut -d/ -f1,2); s=$(echo "$1"|cut -d/ -f3-); p="action.yml"; [ -n "$s" ] && p="$s/action.yml" - gh api "repos/${OWNER}/$b/contents/$p?ref=$2" --jq '.content' 2>/dev/null | tr -d '\n' | base64 -d 2>/dev/null | grep -oE "node[0-9]+" | head -1; } +# gh prints the JSON error body to STDOUT on a non-2xx and exits non-zero, so +# the exit status is the only thing separating an answer from an error wearing +# an answer's clothes. Reading only the text turned one deleted branch into +# eight findings: the missing ref returned +# {"message":"No commit found for the ref frontend",...,"status":"404"} +# an unquoted expansion split it into seven words, and each word was audited as +# though it were a workflow file - printing a fragment of the 404 where a file +# name belongs. +# +# The same lesson as the missing-jq run that reported twelve controls disabled: +# a check that could not run must never be reported as a check that ran. +api() { api_out=$(gh api "$@" 2>/dev/null) || return 1; printf '%s\n' "$api_out"; } + +wf() { api "repos/${OWNER}/$1/contents/.github/workflows/$3?ref=$2" --jq '.content' | tr -d '\n' | base64 -d 2>/dev/null; } +wfs() { api "repos/${OWNER}/$1/contents/.github/workflows?ref=$2" --jq '.[].name'; } +# Own names for the temporaries: rt is only ever called inside $( ), which is +# what currently keeps b, s and p from leaking over the branch loop's own b. +rt() { rt_b=$(echo "$1"|cut -d/ -f1,2); rt_s=$(echo "$1"|cut -d/ -f3-); rt_p="action.yml"; [ -n "$rt_s" ] && rt_p="$rt_s/action.yml" + api "repos/${OWNER}/$rt_b/contents/$rt_p?ref=$2" --jq '.content' | tr -d '\n' | base64 -d 2>/dev/null | grep -oE "node[0-9]+" | head -1; } echo "=== repo-level ===" for r in $REPOS; do @@ -84,6 +99,16 @@ done echo "=== branch-level ===" for e in $BRANCHES; do r=${e%%:*}; b=${e##*:} + + # A branch that is not there cannot be audited, and must not be reported as + # though it were. Asked once, up front: every check below reads this ref, so + # without this its absence is rediscovered by each of them in turn and filed + # as a separate finding against a file that never existed. + if ! api "repos/${OWNER}/$r/branches/$b" --jq '.name' >/dev/null; then + note "$r/$b branch not found - restore it, or drop it from BRANCHES" + continue + fi + c=$(gh api "repos/${OWNER}/$r/branches/$b/protection" --jq '[.required_status_checks.contexts[]?]|length' 2>/dev/null | grep -E '^[0-9]+$' || echo 0) k=$(gh api "repos/${OWNER}/$r/rules/branches/$b" --jq '[.[]|select(.type=="required_status_checks")|.parameters.required_status_checks[].context]|length' 2>/dev/null | grep -E '^[0-9]+$' || echo 0) [ $((c+k)) -gt 0 ] || note "$r/$b no required status checks" @@ -91,8 +116,20 @@ for e in $BRANCHES; do cls=$(gh api "repos/${OWNER}/$r/branches/$b/protection" --jq '"classic"' 2>/dev/null) echo "$types" | grep -q pull_request || [ -n "$cls" ] || note "$r/$b no PR requirement" - for f in $(wfs "$r" "$b"); do + if ! names=$(wfs "$r" "$b"); then + note "$r/$b cannot list workflows - the token needs Contents: Read on this repository" + continue + fi + + # read -r over a here-string, not $(...): a file name is a line, and word + # splitting is what let a 404 body pose as seven of them. + while IFS= read -r f; do + [ -n "$f" ] || continue c2=$(wf "$r" "$b" "$f") + if [ -z "$c2" ]; then + note "$r/$b $f could not be read - reporting that rather than auditing an empty file" + continue + fi echo "$c2" | grep -qE "^permissions:|^\s+permissions:" || note "$r/$b $f no permissions block" echo "$c2" | grep -qE "^\s*pull_request_target:" && note "$r/$b $f uses pull_request_target" echo "$c2" | grep -qE '\$\{\{ *github\.event\.(issue|pull_request|comment|review|head_commit)' && note "$r/$b $f interpolates untrusted input" @@ -102,7 +139,7 @@ for e in $BRANCHES; do echo "$ref" | grep -qE "@[0-9a-f]{40}" || { note "$r/$b $f unpinned: $ref"; continue; } [ "$(rt "$(echo $ref|cut -d@ -f1)" "$(echo $ref|cut -d@ -f2)")" = "node20" ] && note "$r/$b $f node20: $(echo $ref|cut -d@ -f1)" done - done + done <<< "$names" done echo "=== secrets ===" @@ -111,7 +148,11 @@ for r in $REPOS; do hit=0 for e in $BRANCHES; do [ "${e%%:*}" = "$r" ] || continue - for f in $(wfs "$r" "${e##*:}"); do wf "$r" "${e##*:}" "$f" | grep -q "$s" && hit=1; done + names=$(wfs "$r" "${e##*:}") || continue + while IFS= read -r f; do + [ -n "$f" ] || continue + wf "$r" "${e##*:}" "$f" | grep -q "$s" && hit=1 + done <<< "$names" done [ "$hit" -eq 0 ] && note "$r stale secret: $s" done From d6fc38a15e79e42e82bc4aaa4ea03ab952d79e7c Mon Sep 17 00:00:00 2001 From: Burt Gardner Date: Tue, 25 Aug 2026 12:24:30 +0000 Subject: [PATCH 2/3] fix: drop LotteryApp:frontend, whose work now lives in main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01VrPrR9jZDHewD5dKR5dyRk --- README.md | 2 +- scripts/audit.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 3b79a89..c97ee11 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ Configuration is a statement of intent. Only the attempt is evidence. | [WidgetWorks](https://github.com/bgard68/WidgetWorks) | E-commerce store — rotating refresh tokens, TOTP 2FA, server-side re-priced checkout | | **this repository** | audits itself, on the same checklist | -Ten deploying branches between them. Several of these projects deploy from +Nine deploying branches between them. Several of these projects deploy from more than one branch, which is why the audit takes `repo:branch` pairs rather than assuming the default. diff --git a/scripts/audit.sh b/scripts/audit.sh index efdb6fc..0ab0b9a 100755 --- a/scripts/audit.sh +++ b/scripts/audit.sh @@ -44,7 +44,7 @@ OWNER="${OWNER:-bgard68}" # because "is it secure?" kept being answered from memory, and a list with a # hole in it where the auditor sits is the same gap wearing a different hat. REPOS="${REPOS:-ClaudeChessApp ToDoApp LotteryApp Net10Sudoku DevSecOpsSentinel WidgetWorks devsecops-audit}" -BRANCHES="${BRANCHES:-ClaudeChessApp:main ToDoApp:main ToDoApp:dapper ToDoApp:frontend LotteryApp:main LotteryApp:frontend Net10Sudoku:main DevSecOpsSentinel:main WidgetWorks:main devsecops-audit:main}" +BRANCHES="${BRANCHES:-ClaudeChessApp:main ToDoApp:main ToDoApp:dapper ToDoApp:frontend LotteryApp:main Net10Sudoku:main DevSecOpsSentinel:main WidgetWorks:main devsecops-audit:main}" case "${1:-}" in -h|--help) sed -n '2,/^$/p' "$0" | sed 's/^# \{0,1\}//'; exit 0 ;; esac From e31769952f5b9d3b7b62ffd9f50359ddcd5da683 Mon Sep 17 00:00:00 2001 From: Burt Gardner Date: Tue, 25 Aug 2026 12:28:20 +0000 Subject: [PATCH 3/3] fix: stop reporting the mitigation, and the unasked question, as findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01VrPrR9jZDHewD5dKR5dyRk --- scripts/audit.sh | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/scripts/audit.sh b/scripts/audit.sh index 0ab0b9a..40c1529 100755 --- a/scripts/audit.sh +++ b/scripts/audit.sh @@ -67,6 +67,24 @@ api() { api_out=$(gh api "$@" 2>/dev/null) || return 1; printf '%s\n' "$api_out" wf() { api "repos/${OWNER}/$1/contents/.github/workflows/$3?ref=$2" --jq '.content' | tr -d '\n' | base64 -d 2>/dev/null; } wfs() { api "repos/${OWNER}/$1/contents/.github/workflows?ref=$2" --jq '.[].name'; } +# Emit only the lines inside run: block scalars. An interpolation is dangerous +# because a shell executes it; the same expression in an env: or with: mapping +# is the documented mitigation, where the value arrives as a quoted variable and +# is never expanded into script text. Grepping the whole file reported both the +# disease and the cure - it flagged DevSecOpsSentinel's ci.yml, which passes its +# SHAs through env: and carries a comment above them saying why. +# +# 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. +runbody() { + awk ' + /^[[:space:]]*(-[[:space:]]+)?run:[[:space:]]*[|>]/ { inrun=1; ind=match($0,/[^ ]/); next } + inrun { + if ($0 ~ /[^[:space:]]/ && match($0,/[^ ]/) <= ind) { inrun=0; next } + print + } + ' +} # Own names for the temporaries: rt is only ever called inside $( ), which is # what currently keeps b, s and p from leaking over the branch loop's own b. rt() { rt_b=$(echo "$1"|cut -d/ -f1,2); rt_s=$(echo "$1"|cut -d/ -f3-); rt_p="action.yml"; [ -n "$rt_s" ] && rt_p="$rt_s/action.yml" @@ -82,6 +100,12 @@ for r in $REPOS; do sa=$(gh api repos/${OWNER}/$r --jq '.security_and_analysis // "MISSING"' 2>/dev/null) if [ "$sa" = "MISSING" ] || [ -z "$sa" ]; then note "$r cannot read security settings - the token needs Administration: Read on this repository" + # Every check below reads through the same token, so without that access + # their answers are 404s - and a 404 is not a finding. It reported + # WidgetWorks' private vulnerability reporting as off and its + # dependency-review as missing, when neither question was ever answered. + # One honest line, naming the one thing to fix, beats three guesses. + continue else [ "$(gh api repos/${OWNER}/$r --jq '.security_and_analysis.secret_scanning.status' 2>/dev/null)" = "enabled" ] || note "$r secret scanning off" [ "$(gh api repos/${OWNER}/$r --jq '.security_and_analysis.secret_scanning_push_protection.status' 2>/dev/null)" = "enabled" ] || note "$r push protection off" @@ -132,7 +156,7 @@ for e in $BRANCHES; do fi echo "$c2" | grep -qE "^permissions:|^\s+permissions:" || note "$r/$b $f no permissions block" echo "$c2" | grep -qE "^\s*pull_request_target:" && note "$r/$b $f uses pull_request_target" - echo "$c2" | grep -qE '\$\{\{ *github\.event\.(issue|pull_request|comment|review|head_commit)' && note "$r/$b $f interpolates untrusted input" + echo "$c2" | runbody | grep -qE '\$\{\{ *github\.event\.(issue|pull_request|comment|review|head_commit)' && note "$r/$b $f interpolates untrusted input" nco=$(echo "$c2" | grep -c "actions/checkout@"); npc=$(echo "$c2" | grep -c "persist-credentials") [ "$nco" -gt 0 ] && [ "$npc" -lt "$nco" ] && note "$r/$b $f checkout without persist-credentials" for ref in $(echo "$c2" | grep -ohE "uses: [a-zA-Z0-9._-]+/[a-zA-Z0-9._/-]+@[^ ]+" | sed 's/uses: //'); do