diff --git a/.github/workflows/bypass-audit.yml b/.github/workflows/bypass-audit.yml index f76c6b9..ef531b4 100644 --- a/.github/workflows/bypass-audit.yml +++ b/.github/workflows/bypass-audit.yml @@ -1,6 +1,6 @@ name: Admin Bypass Audit -# helmet-pipeline: v1.21.0 +# helmet-pipeline: v1.21.2 # # Detects commits on main that bypassed the required-status-checks gate — i.e., # direct pushes with no associated PR — and opens a GitHub Issue labeled @@ -21,10 +21,14 @@ name: Admin Bypass Audit # branch-protection bypasses is GitHub's ORGANIZATION AUDIT LOG, not this workflow. # Every override is recorded server-side as a `protected_branch.policy_override` # event (real actor, token, before/after SHAs), immune to CI-skip markers and -# unforgeable because it sits OUTSIDE the repo at GitHub's API boundary. Query it: +# unforgeable because it sits OUTSIDE the repo at GitHub's API boundary. It is the +# authoritative record of the bypass in the organization audit log (audit-log API +# access requires GitHub Enterprise Cloud; otherwise use the org audit-log export). +# Example query: # gh api '/orgs//audit-log?phrase=action:protected_branch.policy_override' -# (User-owned repos have no org audit log; for them this workflow plus GitHub's -# account security log are the record.) Either way this workflow is a CONVENIENCE +# (User-owned repos have no org audit log and no equivalent server-side bypass +# record, so for them this push-time workflow is the primary signal.) Either way +# this workflow is a CONVENIENCE # layer — a low-latency GitHub Issue at push time so routine direct-push bypasses # are visible without polling the audit log. # @@ -32,11 +36,13 @@ name: Admin Bypass Audit # - `gh pr merge --admin` associates the merge commit with a PR, so this workflow # won't flag admin-merges. Those are the pr-grind opt-in's own authorized path # and are logged separately to .claude/bypass-log.jsonl. -# - A direct push whose head commit carries a native CI-skip marker ([skip ci], -# [skip actions], skip-checks:true, …) suppresses this workflow, so the -# convenience Issue won't be opened for that push. This is NOT an audit gap: the -# AUTHORITATIVE TRAIL above still records the bypass server-side, immune to -# CI-skip markers. A post-hoc workflow "sweep" to close the notification gap was +# - A direct push carrying a native CI-skip marker ([skip ci], [skip actions], +# skip-checks:true, …) in its commit message suppresses this workflow, so the +# convenience Issue won't be opened for that push. For ORG-OWNED repos this is +# not an audit gap — the org audit log above still records the bypass server-side, +# immune to CI-skip markers. USER-OWNED repos have no equivalent audit-log +# fallback, so a CI-skip direct push there is an accepted residual gap. A post-hoc +# workflow "sweep" to close the gap was # evaluated and REJECTED — a workflow cannot distinguish legitimate automation # from a forged bypass after the fact (author identity is spoofable; CI-skip # markers are push-level, so a paired unmarked commit evades a per-commit check; @@ -49,9 +55,18 @@ name: Admin Bypass Audit # MUTABLE, so even an author filter is insider-editable) and short-SHA-collision # -prone. A manual workflow re-run is rare and a duplicate issue is harmless — # far better than a suppression/collision vector. +# - The merged-PR check relies on GitHub's commit->PR association, which marks a SHA +# as belonging to a merged PR even for the ORIGINAL commits of a SQUASH-merged PR +# (those never landed on main — only the squash commit did). A human who direct- +# pushes one of those exact original SHAs is therefore not flagged here. Accepted +# residual of the association API: for ORG-OWNED repos the org audit log records +# the actual push; USER-OWNED repos have no equivalent fallback, so it stays a +# silent gap there. # -# SECURITY: All user-controlled inputs (commit message, actor name) are passed -# via env: block and quoted in shell. Never interpolate github.event.* directly +# SECURITY: User-controlled inputs are never interpolated into run: commands. +# The actor name is passed via the env: block and quoted in shell; the commit +# list and messages are read from $GITHUB_EVENT_PATH with jq (file data, not a +# shell-expanded ${{ }} expression). Never interpolate github.event.* directly # into run: commands. on: @@ -72,8 +87,9 @@ jobs: # Push-time audit: immediate detection of a direct push to main with no PR. audit: runs-on: ubuntu-latest - timeout-minutes: 3 + timeout-minutes: 10 permissions: + contents: read # required by /commits/{sha}/pulls on PRIVATE repos (public repos work without it) issues: write # create audit issues pull-requests: read # look up PR for commit SHA steps: @@ -85,109 +101,183 @@ jobs: - name: Detect direct-push bypass env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - COMMIT_SHA: ${{ github.sha }} ACTOR: ${{ github.actor }} REPO: ${{ github.repository }} - COMMIT_MSG: ${{ github.event.head_commit.message }} RUN_ID: ${{ github.run_id }} + HEAD_SHA: ${{ github.sha }} run: | - # NOTE: `set -e` only (NOT `set -euo pipefail`). `pipefail` would make the - # `printf '%s' "$COMMIT_MSG" | head -3` early-exit pipe SIGPIPE-fail its - # upstream process, aborting the job after a bypass is detected but before - # the issue is created. + # GitHub runs `shell: bash` steps with `-eo pipefail` (see defaults above), + # so a pipe whose reader closes early (e.g. `head`) would SIGPIPE its + # still-writing upstream and fail the step. Every pipe below is pipefail- + # safe: readers consume their full input, and first-line extraction uses + # bash parameter expansion rather than `head`. `set -e` here is belt-and- + # braces (the wrapper already sets it). set -e # Skip automated actors ONLY (bots run via GITHUB_TOKEN, semantic-release, - # dependabot, etc.). This is an IDENTITY-based skip, not a message-content - # skip: a human direct-pusher cannot suppress the audit via commit text. + # dependabot, etc.). IDENTITY-based skip — the pusher is the same for every + # commit in the push — NOT a message-content skip: a human direct-pusher + # cannot suppress the audit via commit text. Commit messages are likewise + # never consulted to skip (`chore(release)`/`[skip ci]` are attacker- + # forgeable); legitimate release commits are authored by a bot actor and + # are already skipped here. if [[ "$ACTOR" == *"[bot]"* ]] || [[ "$ACTOR" == "github-actions" ]]; then echo "Skipping audit — automated actor: $ACTOR" exit 0 fi - # NOTE: COMMIT_MSG is intentionally NOT consulted to skip the audit. - # Trusting `chore(release)` / `[skip ci]` in attacker-controlled commit - # text would let any human bypasser evade detection. Legitimate release - # commits are authored by a bot actor and are already skipped above. - - # Look up PRs associated with this commit SHA. - # CRITICAL: Distinguish "API succeeded, no PR found" from "API failed". - # The former = bypass (alert via issue). The latter = indeterminate, so we - # FAIL the run (a red X persists in Actions history) rather than create a - # misleading "confirmed bypass" issue OR silently pass. A failed run is a - # durable, investigable signal without false-positive issue noise. - if ! PRS_JSON=$(gh api "repos/$REPO/commits/$COMMIT_SHA/pulls" 2>&1); then - echo "::error::gh api failed to list PRs for $COMMIT_SHA — cannot determine bypass status. Failing the run so the push is not silently left unaudited; re-run or investigate manually." - echo "Response: $PRS_JSON" + # Audit every commit added by this push, not just HEAD. A multi-commit + # direct push whose HEAD happens to be PR-associated (e.g. a re-pushed + # already-merged commit) would otherwise hide earlier unaudited commits. + # Source: the push payload's commit list (capped by GitHub at 2048), read + # from $GITHUB_EVENT_PATH — the event JSON already on disk. We deliberately + # do NOT pass `toJSON(github.event.commits)` through an env var: with up to + # 2048 commits (each carrying message + file lists + URLs) the serialized + # array can exceed the runner's env/ARG_MAX limit (~2 MB), making the step + # fail to exec BEFORE any audit logic runs — silently suppressing the very + # bypass this workflow exists to catch. jq reads the file as data (never + # interpolated into the shell). + # + # Fail-closed on an INDETERMINATE payload — unreadable, not valid JSON, + # or `.commits` missing / not an array. Silently degrading to a HEAD-only + # audit could miss earlier commits in a multi-commit direct push, so we + # take the same fail-on-indeterminate stance used for the per-commit PR + # lookups below. jq's `error()` makes the whole command exit non-zero on a + # non-array `.commits`, which the `if !` catches (a bare `.commits[]` would + # instead emit nothing on stderr→/dev/null and look like an empty list). A + # valid but EMPTY array is NOT indeterminate (e.g. a non-commit push) — jq + # exits 0 with no output, so ALL_SHAS ends up empty and falls back to HEAD. + if ! COMMIT_IDS=$(jq -r 'if (.commits|type)=="array" then (.commits[].id // empty) else error("commits not an array") end' "$GITHUB_EVENT_PATH" 2>/dev/null); then + echo "::error::Could not read a valid .commits array from \$GITHUB_EVENT_PATH — bypass status indeterminate; failing the run." exit 1 fi - - # Parse once, reuse. A 200-OK-but-unexpected body (not a JSON array) is - # INDETERMINATE, not "zero PRs" — coercing it to 0 would manufacture a - # false-positive bypass issue. `jq -e` errors (exit 5) on a non-array, so - # fail the run rather than silently misclassify. - if ! PR_COUNT=$(printf '%s' "$PRS_JSON" | jq -e 'if type=="array" then length else error("not an array") end' 2>/dev/null); then - echo "::error::Unexpected PR-list response for $COMMIT_SHA — cannot determine bypass status; failing the run." - exit 1 + mapfile -t ALL_SHAS < <(printf '%s' "$COMMIT_IDS") + if [ "${#ALL_SHAS[@]}" -eq 0 ]; then + ALL_SHAS=("$HEAD_SHA") fi - if [ "$PR_COUNT" != "0" ]; then - PR_NUM=$(printf '%s' "$PRS_JSON" | jq -r '.[0].number // "?"' 2>/dev/null || echo "?") - echo "No bypass — commit came from PR #$PR_NUM" - exit 0 + # Bound the per-run PR-lookup work: each commit costs ONE cheap read call + # (bypasses are summarized into a SINGLE issue below, so there is no + # per-commit issue-creation rate-limit pressure). If the push exceeds + # MAX_AUDIT, audit the first MAX_AUDIT AND fail the run (durable red-X) so + # the unaudited remainder is never silently passed — investigate it + # manually. MAX_AUDIT is well above any realistic push so a legitimate + # large merge does not trip it. + AUDIT_FAILED=0 + MAX_AUDIT=256 + SHAS=("${ALL_SHAS[@]:0:$MAX_AUDIT}") + if [ "${#ALL_SHAS[@]}" -gt "$MAX_AUDIT" ]; then + echo "::error::Push has ${#ALL_SHAS[@]} commits (> $MAX_AUDIT) — audited the first $MAX_AUDIT and failing the run; review the remainder manually." + AUDIT_FAILED=1 fi + echo "Auditing ${#SHAS[@]} of ${#ALL_SHAS[@]} commit(s) added by this push." + + # Collect bypassing commits into ONE summary issue (filed after the loop) + # rather than an issue per commit: a multi-commit bypass would otherwise + # fire N issue-create calls and hit GitHub's content-creation secondary + # rate limit, and one issue is easier to triage. AUDIT_FAILED (set above) + # also accumulates per-commit indeterminate lookups so a single bad commit + # fails the whole run WITHOUT skipping the rest. + BYPASS_ROWS="" + BYPASS_COUNT=0 + for COMMIT_SHA in "${SHAS[@]}"; do + # Look up PRs associated with this commit SHA. + # CRITICAL: Distinguish "API succeeded, no PR found" (= bypass) from + # "API failed/garbled" (= INDETERMINATE → flag the run, keep auditing). + # Capture stderr SEPARATELY (not 2>&1) — a successful call that emits a + # warning on stderr would otherwise corrupt PRS_JSON and manufacture a + # false indeterminate. + PRS_ERR=$(mktemp) + if ! PRS_JSON=$(gh api "repos/$REPO/commits/$COMMIT_SHA/pulls" 2>"$PRS_ERR"); then + echo "::error::gh api failed to list PRs for $COMMIT_SHA — bypass status indeterminate; failing the run." + echo "Response: $(head -c 500 "$PRS_ERR" | tr -d '\r')" + rm -f "$PRS_ERR" + AUDIT_FAILED=1 + continue + fi + rm -f "$PRS_ERR" + + # Count ONLY PRs actually MERGED into main. `/commits/{sha}/pulls` also + # returns PRs that merely CONTAIN the commit (open PRs, PRs targeting + # other branches), so a direct-pushed commit that also lives on an + # unmerged branch would otherwise read as "no bypass". A real merge has + # `merged_at != null` AND `base.ref == "main"` (this workflow only runs + # on pushes to main). A 200-OK-but-unexpected body (not a JSON array) is + # INDETERMINATE — `jq -e` errors on a non-array, so flag rather than + # silently misclassify. + if ! MERGED_PRS=$(printf '%s' "$PRS_JSON" | jq -e 'if type=="array" then [.[] | select(.merged_at != null and .base.ref == "main")] else error("not an array") end' 2>/dev/null); then + echo "::error::Unexpected PR-list response for $COMMIT_SHA — indeterminate; failing the run." + AUDIT_FAILED=1 + continue + fi + if [ "$(printf '%s' "$MERGED_PRS" | jq 'length' 2>/dev/null || echo 0)" != "0" ]; then + PR_NUM=$(printf '%s' "$MERGED_PRS" | jq -r '.[0].number // "?"' 2>/dev/null || echo "?") + echo "No bypass — $COMMIT_SHA merged into main via PR #$PR_NUM" + continue + fi + + # No associated PR → direct push = bypass. Accumulate a table row; + # file ONE summary issue after the loop. + echo "::warning::Admin bypass: direct push to main by $ACTOR (commit ${COMMIT_SHA})" + SHORT_SHA=$(printf '%s' "$COMMIT_SHA" | cut -c1-7) + # First line of the commit message (attacker-controlled). Sanitize for a + # Markdown TABLE CELL: strip backtick (octal 140) / CR / LF / pipe, + # truncate, then wrap in inline code so no Markdown (links/emphasis/HTML) + # parses inside the cell. + MSG=$(jq -r --arg s "$COMMIT_SHA" '(.commits[] | select(.id==$s) | .message) // ""' "$GITHUB_EVENT_PATH" 2>/dev/null || echo "") + FIRST_LINE=${MSG%%$'\n'*} + SUBJECT=$(printf '%s' "$FIRST_LINE" | tr -d '\140\r\n|' | cut -c1-100) + BYPASS_ROWS="${BYPASS_ROWS}| [\`${SHORT_SHA}\`](https://github.com/${REPO}/commit/${COMMIT_SHA}) | \`${SUBJECT}\` |"$'\n' + BYPASS_COUNT=$((BYPASS_COUNT + 1)) + done + + # File ONE summary issue if any bypass was detected. + if [ "$BYPASS_COUNT" -gt 0 ]; then + # Ensure the admin-bypass label exists (idempotent). `gh label create` + # exits non-zero when it ALREADY exists; treat "created" OR "already + # present" as OK, only a genuine absence sets LABEL_OK=0. + LABEL_OK=1 + gh label create "admin-bypass" \ + --color "d93f0b" \ + --description "Commit bypassed required status checks" \ + --repo "$REPO" 2>/dev/null \ + || gh api "repos/$REPO/labels/admin-bypass" --jq '.name' 2>/dev/null | grep -qx "admin-bypass" \ + || LABEL_OK=0 + + NOW=$(date -u +%Y-%m-%dT%H:%M:%SZ) + TITLE="Admin Bypass: ${BYPASS_COUNT} direct-push commit(s) by @$ACTOR" + BODY_FILE=$(mktemp) + { + printf '%s\n\n' "**Direct Push Bypass Detected**" + printf 'The following %s commit(s) were pushed directly to `main` by @%s without an associated pull request, bypassing required status checks and review policy.\n\n' "$BYPASS_COUNT" "$ACTOR" + printf '| Commit | Subject |\n' + printf '|--------|---------|\n' + printf '%s' "$BYPASS_ROWS" + printf '\n' + printf '| Field | Value |\n' + printf '|-------|-------|\n' + printf '| **Pushed by** | @%s |\n' "$ACTOR" + printf '| **Workflow run** | [%s](https://github.com/%s/actions/runs/%s) |\n' "$RUN_ID" "$REPO" "$RUN_ID" + printf '| **Time (UTC)** | `%s` |\n\n' "$NOW" + printf '**Action required:** Review whether these pushes were intentional. If so, document the rationale and close. If not, investigate how branch protection was circumvented.\n' + } > "$BODY_FILE" - # No associated PR → direct push = bypass - echo "::warning::Admin bypass: direct push to main by $ACTOR (commit ${COMMIT_SHA})" - - SHORT_SHA=$(printf '%s' "$COMMIT_SHA" | cut -c1-7) - - # Ensure the admin-bypass label exists (idempotent; track success for log clarity). - # `gh label create` exits non-zero when the label ALREADY exists, so a bare - # `|| LABEL_OK=0` would wrongly mark the label unavailable on every run after - # the first. Treat "create succeeded" OR "label already present" as OK; only a - # genuine absence (create failed AND label not found) sets LABEL_OK=0. - LABEL_OK=1 - gh label create "admin-bypass" \ - --color "d93f0b" \ - --description "Commit bypassed required status checks" \ - --repo "$REPO" 2>/dev/null \ - || gh api "repos/$REPO/labels/admin-bypass" --jq '.name' 2>/dev/null | grep -qx "admin-bypass" \ - || LABEL_OK=0 - - # Compose issue body in a file — values from env vars, properly quoted. - # Trap ensures cleanup even if issue creation fails and set -e aborts. - BODY_FILE=$(mktemp) - trap 'rm -f "$BODY_FILE"' EXIT - - TITLE="Admin Bypass: ${SHORT_SHA} by @$ACTOR" - MSG_PREVIEW=$(printf '%s' "$COMMIT_MSG" | head -3) - NOW=$(date -u +%Y-%m-%dT%H:%M:%SZ) - - { - printf '%s\n\n' "**Direct Push Bypass Detected**" - printf '%s\n\n' "This commit was pushed directly to \`main\` without a pull request, bypassing required status checks and review policy." - printf '| Field | Value |\n' - printf '|-------|-------|\n' - printf '| **Commit** | [\`%s\`](https://github.com/%s/commit/%s) |\n' "$COMMIT_SHA" "$REPO" "$COMMIT_SHA" - printf '| **Actor** | @%s |\n' "$ACTOR" - printf '| **Workflow run** | [%s](https://github.com/%s/actions/runs/%s) |\n' "$RUN_ID" "$REPO" "$RUN_ID" - printf '| **Time (UTC)** | `%s` |\n\n' "$NOW" - printf '**Commit message (first 3 lines):**\n\n' - printf '```\n%s\n```\n\n' "$MSG_PREVIEW" - printf '**Action required:** Review whether this bypass was intentional. If so, document the rationale in a comment and close. If not, investigate how branch protection was circumvented.\n' - } > "$BODY_FILE" - - # Try with label first; fall back to no-label if label creation failed earlier - # or if labeling itself fails. If even the unlabeled create fails, fail the - # run (red X) so the bypass is not silently left unrecorded. - if [ "$LABEL_OK" = "1" ]; then - if ! gh issue create --repo "$REPO" --title "$TITLE" --body-file "$BODY_FILE" --label "admin-bypass" 2>&1; then - echo "::warning::Failed to create labeled issue — retrying without label" + # Try with label first; fall back to no-label. If even the unlabeled + # create fails, flag the run so the bypass is not silently unrecorded. + if [ "$LABEL_OK" = "1" ]; then + if ! gh issue create --repo "$REPO" --title "$TITLE" --body-file "$BODY_FILE" --label "admin-bypass" 2>&1; then + echo "::warning::Failed to create labeled issue — retrying without label" + gh issue create --repo "$REPO" --title "$TITLE" --body-file "$BODY_FILE" \ + || { echo "::error::Failed to create the admin-bypass summary issue"; AUDIT_FAILED=1; } + fi + else + echo "::warning::admin-bypass label unavailable — creating issue without label" gh issue create --repo "$REPO" --title "$TITLE" --body-file "$BODY_FILE" \ - || { echo "::error::Failed to create audit issue for $COMMIT_SHA"; exit 1; } + || { echo "::error::Failed to create the admin-bypass summary issue"; AUDIT_FAILED=1; } fi - else - echo "::warning::admin-bypass label unavailable — creating issue without label" - gh issue create --repo "$REPO" --title "$TITLE" --body-file "$BODY_FILE" \ - || { echo "::error::Failed to create audit issue for $COMMIT_SHA"; exit 1; } + rm -f "$BODY_FILE" + fi + + if [ "$AUDIT_FAILED" = "1" ]; then + echo "::error::One or more pushed commits could not be audited or filed — failing the run." + exit 1 fi