diff --git a/README.md b/README.md index c935505..c97ee11 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 +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 d916ef1..40c1529 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 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,43 @@ 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'; } +# 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" + 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 @@ -67,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" @@ -84,6 +123,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,18 +140,30 @@ 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" + 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 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 +172,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