From 0216cc1d0c0715b2cf4e1eafeb6918e18e7ef058 Mon Sep 17 00:00:00 2001 From: Chris Yau Date: Fri, 5 Jun 2026 04:20:26 +0800 Subject: [PATCH 1/3] feat(ci): adopt canonical bypass-audit standard + pipeline drift detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Supersedes the dedup approach. Two things: 1. Canonical standard — adopt diveanddev.com's design as helmet's template: push-only, identity-skip ONLY (no message-based skip a human could forge), fail-the-run on an indeterminate PR-lookup (never silent-skip, never false-positive), and NO dedup. Issue metadata is mutable, so a title/SHA dedup is an insider-editable suppression primitive (even an author filter is defeated by editing a bot-authored issue); the org audit log is the authoritative trail and a duplicate on a rare re-run is harmless. 2. Drift detection — vendored boilerplate was silently freezing at its onboarding generation (chrisyau.me/jikdak sat at v1.12 while helmet hit v1.21). Now: stamp `# helmet-pipeline: vX.Y.Z` into the template, and scripts/check-pipeline-drift.sh compares each repo's stamp to helmet's own template stamp (not plugin.json — avoids false drift on unrelated bumps), failing on any behind/unstamped/unreadable repo. Update SKILL.md template excerpt + detection-logic to match. ADR-0001 records the design, the vendored-not-reusable choice (keep repos self-contained), and the seatbelt sweep exception. .helmet-fleet.example documents the scan list. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy --- .github/workflows/bypass-audit.yml | 123 +++++++++++++----- .gitignore | 3 + .helmet-fleet.example | 19 +++ ...pass-audit-standard-and-drift-detection.md | 75 +++++++++++ scripts/check-pipeline-drift.sh | 102 +++++++++++++++ skills/helmet/SKILL.md | 62 +++++---- 6 files changed, 316 insertions(+), 68 deletions(-) create mode 100644 .gitignore create mode 100644 .helmet-fleet.example create mode 100644 docs/adr/0001-bypass-audit-standard-and-drift-detection.md create mode 100755 scripts/check-pipeline-drift.sh diff --git a/.github/workflows/bypass-audit.yml b/.github/workflows/bypass-audit.yml index 4cabeb6..4da3ddf 100644 --- a/.github/workflows/bypass-audit.yml +++ b/.github/workflows/bypass-audit.yml @@ -1,21 +1,54 @@ name: Admin Bypass Audit -# Detects commits pushed directly to main without a PR — i.e., admin bypasses -# of the required-status-checks gate. Creates a GitHub Issue with label -# `admin-bypass` and emits a workflow warning annotation. +# helmet-pipeline: v1.21.0 +# +# 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 +# `admin-bypass` (plus a workflow warning annotation). # # With enforce_admins: false (solo-dev default), repo admins can bypass branch # protection via direct push or --admin merge. This workflow makes those # bypasses visible and auditable after the fact. # -# Known limitations: -# - `gh pr merge --admin` DOES associate the merge commit with a PR, so this -# workflow won't detect that vector. For that, add check-runs inspection. -# - Automated actors matching `[bot]` suffix (github-actions[bot], -# dependabot[bot], etc.) or actor equal to `github-actions` are skipped -# to avoid noise on release commits. -# - Release commits (`chore(release)` as first line of commit message, or -# `[skip ci]` anywhere in the full message) are skipped. +# Design: a single push-time `audit` job. It keys on github.actor — the +# UNFORGEABLE identity of whoever pushed, evaluated AT push time. A human direct +# push with no PR is flagged; pushes by automation (github-actions, dependabot, +# etc.) are skipped. This is robust because github.actor cannot be spoofed, and +# it never false-positives on a repo's own automation: pinact's GITHUB_TOKEN +# pushes don't trigger workflows at all, so the audit simply never runs for them. +# +# AUTHORITATIVE TRAIL: for ORG-OWNED repos, the canonical tamper-proof record of +# 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: +# 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 +# layer — a low-latency GitHub Issue at push time so routine direct-push bypasses +# are visible without polling the audit log. +# +# Known limitations (accepted): +# - `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 +# 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; +# and GITHUB_TOKEN automation pushes create no workflow run, indistinguishable +# from a suppressed one). That structural limitation is exactly why GitHub's +# server-side audit log, not a workflow, is the real answer. +# - No dedup on purpose. A title/SHA existing-issue check is both pre-creatable +# (anyone with issues:write — incl. a bypasser who knows their own SHA — can +# pre-open or edit a matching issue to suppress the audit; issue metadata is +# 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. # # SECURITY: All user-controlled inputs (commit message, actor name) are passed # via env: block and quoted in shell. Never interpolate github.event.* directly @@ -36,6 +69,7 @@ defaults: shell: bash jobs: + # Push-time audit: immediate detection of a direct push to main with no PR. audit: runs-on: ubuntu-latest timeout-minutes: 3 @@ -57,39 +91,45 @@ jobs: COMMIT_MSG: ${{ github.event.head_commit.message }} RUN_ID: ${{ github.run_id }} 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. set -e - # Skip automated actors (bots run via GITHUB_TOKEN, semantic-release, dependabot, etc.) + # 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. if [[ "$ACTOR" == *"[bot]"* ]] || [[ "$ACTOR" == "github-actions" ]]; then echo "Skipping audit — automated actor: $ACTOR" exit 0 fi - # Skip release commits. - # `chore(release)` only matches as first-line prefix (conventional commit). - # `[skip ci]` is searched in the FULL commit message (header, body, or trailer). - FIRST_LINE=$(printf '%s' "$COMMIT_MSG" | head -1) - if [[ "$FIRST_LINE" == "chore(release)"* ]]; then - echo "Skipping audit — release commit: $FIRST_LINE" - exit 0 - fi - if printf '%s' "$COMMIT_MSG" | grep -qF '[skip ci]'; then - echo "Skipping audit — CI-skip marker in commit message" - 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). The latter = transient error (warn + skip, - # do NOT create false-positive issue). + # 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 "::warning::gh api failed to list PRs for $COMMIT_SHA — skipping audit (cannot determine bypass status)" + 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" - exit 0 + exit 1 fi - # Parse once, reuse - PR_COUNT=$(printf '%s' "$PRS_JSON" | jq 'length' 2>/dev/null || echo "0") + # 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 + 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" @@ -99,19 +139,27 @@ jobs: # No associated PR → direct push = bypass echo "::warning::Admin bypass: direct push to main by $ACTOR (commit ${COMMIT_SHA})" - # Ensure the admin-bypass label exists (idempotent; track success for log clarity) + 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 || LABEL_OK=0 + --repo "$REPO" 2>/dev/null \ + || gh label list --repo "$REPO" --json name --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: $(printf '%s' "$COMMIT_SHA" | cut -c1-7) by @$ACTOR" + TITLE="Admin Bypass: ${SHORT_SHA} by @$ACTOR" MSG_PREVIEW=$(printf '%s' "$COMMIT_MSG" | head -3) NOW=$(date -u +%Y-%m-%dT%H:%M:%SZ) @@ -130,13 +178,16 @@ jobs: } > "$BODY_FILE" # Try with label first; fall back to no-label if label creation failed earlier - # or if labeling itself fails. Log the fallback so audit is not silent. + # 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" - gh issue create --repo "$REPO" --title "$TITLE" --body-file "$BODY_FILE" + gh issue create --repo "$REPO" --title "$TITLE" --body-file "$BODY_FILE" \ + || { echo "::error::Failed to create audit issue for $COMMIT_SHA"; exit 1; } fi else echo "::warning::admin-bypass label unavailable — creating issue without label" - gh issue create --repo "$REPO" --title "$TITLE" --body-file "$BODY_FILE" + gh issue create --repo "$REPO" --title "$TITLE" --body-file "$BODY_FILE" \ + || { echo "::error::Failed to create audit issue for $COMMIT_SHA"; exit 1; } fi diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..e838c9c --- /dev/null +++ b/.gitignore @@ -0,0 +1,3 @@ + +# helmet drift-scan fleet list (operator-specific) +.helmet-fleet diff --git a/.helmet-fleet.example b/.helmet-fleet.example new file mode 100644 index 0000000..e3a5b0b --- /dev/null +++ b/.helmet-fleet.example @@ -0,0 +1,19 @@ +# helmet fleet list — repos to scan for pipeline drift. +# Copy to `.helmet-fleet` (gitignored) and edit. One `owner/repo` per line; # comments OK. +# Used by: scripts/check-pipeline-drift.sh --fleet +# +# List ONLY push-time repos that carry the canonical `# helmet-pipeline: vX.Y.Z` stamp. +# The drift check exits non-zero on any unstamped/behind repo, so do NOT list repos that +# are intentionally off the push-time standard — they would false-positive forever: +# • Dive-And-Dev/diveanddev.com — the hand-authored REFERENCE design; carries no helmet +# stamp by design (ADR-0001 — it needs no change). It IS the standard, not measured against it. +# • chris-yyau/seatbelt — a scheduled-SWEEP variant (cron + dedup), a documented exception +# (ADR-0001); structurally divergent from the push-time standard, so it is not scanned here. +# +# Example (stamp-bearing push-time repos): +# chris-yyau/busdriver +# chris-yyau/helmet +# Dive-And-Dev/perch +# Dive-And-Dev/chrisyau.me +# Dive-And-Dev/jikdak +# Dive-And-Dev/growth-engine diff --git a/docs/adr/0001-bypass-audit-standard-and-drift-detection.md b/docs/adr/0001-bypass-audit-standard-and-drift-detection.md new file mode 100644 index 0000000..fbda1d8 --- /dev/null +++ b/docs/adr/0001-bypass-audit-standard-and-drift-detection.md @@ -0,0 +1,75 @@ +# ADR-0001: Single bypass-audit standard + pipeline drift detection + +- **Status:** Accepted +- **Date:** 2026-06-05 + +## Context + +helmet generates a `bypass-audit.yml` workflow into each repo it onboards (detects +direct pushes to `main` that bypassed required checks, opens an `admin-bypass` issue). +Because helmet **vendors** (copies) the file at onboarding, every repo froze a snapshot +of whatever helmet generation it adopted. Nothing re-synced them — dependabot bumps +action SHAs but never the workflow *logic* — so the fleet drifted badly: `chrisyau.me` +and `jikdak` sat at the **v1.12** generation (April) while a freshly-authored +`diveanddev.com` (June, via its PR #30) had a materially more secure design. The +neglected repo was *ahead* of the actively-developed ones, purely because its file was +written last. + +Three design divergences had accumulated across generations: (a) dedup present/absent +and, where present, gameable; (b) commit-message-based skip (`[skip ci]`/`chore(release)`) +that a human bypasser can forge to evade; (c) silent skip vs. fail-the-run on an +indeterminate API response. + +A code review (codex) also surfaced that any **dedup keyed on issue title/body is an +insider-editable suppression primitive**: GitHub issue metadata is mutable, and +`.author.login` stays `github-actions[bot]` even after a human edits the body — so an +author-filtered dedup can still be defeated by editing a bot-authored issue to pre-load +a future bypass SHA. `diveanddev.com` had independently reasoned to *no dedup* for +exactly this reason. + +## Decision + +1. **One standard = `diveanddev.com`'s design** for all push-time repos: push-only, + **identity-based skip only** (no commit-message skip), **fail-the-run** on an + indeterminate PR-lookup (never silent-skip, never false-positive), and **no dedup** + (the org audit log is the authoritative trail; a duplicate issue on a rare manual + re-run is harmless and far safer than a mutable-metadata suppression vector). + helmet's own `bypass-audit.yml` is the canonical template. +2. **Distribution stays vendored (self-contained), not centralized.** Each repo keeps + its own copy; we do **not** convert to a reusable workflow. Rationale: reusable + workflows would couple every production app repo to helmet at runtime (and make the + repo that *authored* the design depend on a copy of itself) — unacceptable for + self-contained production repos. +3. **Prevent future drift with detection, not coupling.** Every generated workflow + carries a `# helmet-pipeline: vX.Y.Z` stamp; `scripts/check-pipeline-drift.sh` + compares each repo's stamp to the canonical version and reports repos that are + behind. Drift becomes visible instead of silent. +4. **`seatbelt` is a documented exception.** It is a daily *sweep* (cron) design, which + structurally requires dedup; it is not converged to the push-only standard. + +## Alternatives considered + +- **Reusable workflow (centralize):** eliminates drift structurally, but couples every + repo to helmet at runtime and makes self-contained production repos non-self-contained. + Rejected — the coupling cost outweighs the "byte-identical forever" guarantee. +- **Keep author-filtered dedup as the standard:** rejected — codex showed it remains an + insider-editable suppression primitive; for an audit workflow, no-dedup is safer. +- **Drop seatbelt's sweep too (full uniformity):** rejected — would delete a deliberate, + more-thorough capability; a sweep genuinely needs dedup. + +## Consequences + +- The six in-flight "hardened dedup" PRs are **superseded** (to be closed) — the standard is + no-dedup. +- All push-time repos converge on one design; new onboards are born on it and stamped. +- Drift is now detectable on demand (and via a scheduled scan); re-sync is a manual + re-onboard when the check flags a repo (acceptable for a vendored model). +- `diveanddev.com` needs no change — it *is* the reference. + +## Revisit trigger + +- If manual re-syncs become frequent/annoying, add an auto-re-adoption PR bot. +- If a repo gains multiple `issues:write` collaborators AND a no-dedup duplicate-issue + rate becomes a real nuisance, reconsider a non-metadata dedup (e.g. a committed ledger), + not a metadata one. +- If GitHub ships first-class org-wide required workflows that fit, reconsider centralizing. diff --git a/scripts/check-pipeline-drift.sh b/scripts/check-pipeline-drift.sh new file mode 100755 index 0000000..ef86fcd --- /dev/null +++ b/scripts/check-pipeline-drift.sh @@ -0,0 +1,102 @@ +#!/usr/bin/env bash +# check-pipeline-drift.sh — surface repos whose helmet-generated CI boilerplate has +# silently drifted behind the current helmet pipeline version. +# +# WHY: helmet *vendors* (copies) its workflows into each repo at onboarding, so every +# repo holds a frozen snapshot. Nothing re-syncs it — dependabot bumps action SHAs but +# never the workflow logic — so a repo can sit generations behind without anyone +# noticing (e.g. a repo onboarded at v1.12 while helmet is at v1.21). Each generated +# workflow carries a `# helmet-pipeline: vX.Y.Z` stamp; this script reads that stamp +# from each repo and compares it to the canonical version, turning silent drift loud. +# +# This is the *self-contained* prevention model (repos keep their own copy; we detect +# drift) rather than reusable-workflows (central copy; repos depend on it). See +# docs/adr/0001-bypass-audit-standard-and-drift-detection.md. +# +# USAGE: +# scripts/check-pipeline-drift.sh [ ...] +# scripts/check-pipeline-drift.sh --fleet # read repos from .helmet-fleet +# +# EXIT: 0 = no drift; 1 = at least one repo behind/unstamped/ahead; 2 = usage/setup error. +# REQUIRES: gh (authenticated), jq. +set -euo pipefail + +SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +REPO_ROOT=$(cd "$SCRIPT_DIR/.." && pwd) +WORKFLOW_PATH=".github/workflows/bypass-audit.yml" +FLEET_FILE="$REPO_ROOT/.helmet-fleet" + +# Canonical version = the `# helmet-pipeline:` stamp in helmet's OWN bypass-audit.yml +# (the template itself), NOT the plugin version. The stamp is bumped only when the +# template changes, so an unrelated plugin version bump can't manufacture false drift. +CANON_FILE="$REPO_ROOT/$WORKFLOW_PATH" +CANON=$(sed -n 's/^# helmet-pipeline: v\([0-9][0-9.]*\).*/\1/p' "$CANON_FILE" 2>/dev/null | head -1) +if [[ -z "$CANON" ]]; then + echo "ERROR: no '# helmet-pipeline: vX.Y.Z' stamp found in $CANON_FILE (the canonical template)" >&2 + exit 2 +fi + +# Resolve the repo list (explicit args, or --fleet from .helmet-fleet). +repos=() +if [[ "${1:-}" == "--fleet" ]]; then + if [[ ! -f "$FLEET_FILE" ]]; then + echo "ERROR: --fleet given but $FLEET_FILE not found (see .helmet-fleet.example)" >&2 + exit 2 + fi + while IFS= read -r line; do + line="${line%%#*}" # strip comments + line="$(printf '%s' "$line" | tr -d '[:space:]')" + [[ -n "$line" ]] && repos+=("$line") + done < "$FLEET_FILE" +elif [[ "$#" -gt 0 ]]; then + repos=("$@") +fi +if [[ "${#repos[@]}" -eq 0 ]]; then + echo "Usage: $0 [ ...] | --fleet" >&2 + exit 2 +fi + +printf 'Canonical helmet-pipeline version: v%s\n\n' "$CANON" +printf '%-34s %-12s %s\n' "REPO" "STAMP" "STATUS" + +current=0; behind=0; ahead=0; missing=0 +for repo in "${repos[@]}"; do + raw=$(gh api "repos/$repo/contents/$WORKFLOW_PATH" -H "Accept: application/vnd.github.raw" 2>/dev/null || true) + if [[ -z "$raw" ]]; then + printf '%-34s %-12s %s\n' "$repo" "-" "not found / no access (verify)" + missing=$((missing + 1)) + continue + fi + stamp=$(printf '%s\n' "$raw" | sed -n 's/^# helmet-pipeline: v\([0-9][0-9.]*\).*/\1/p' | head -1) + if [[ -z "$stamp" ]]; then + printf '%-34s %-12s %s\n' "$repo" "unstamped" "drift: no version stamp (pre-detection)" + behind=$((behind + 1)) + continue + fi + if [[ "$stamp" == "$CANON" ]]; then + printf '%-34s %-12s %s\n' "$repo" "v$stamp" "current" + current=$((current + 1)) + continue + fi + # Not equal: decide behind vs ahead via version sort (separate command — SC2312). + oldest=$(printf '%s\n%s\n' "$stamp" "$CANON" | sort -V | head -1) + if [[ "$oldest" == "$stamp" ]]; then + printf '%-34s %-12s %s\n' "$repo" "v$stamp" "BEHIND (canonical v$CANON)" + behind=$((behind + 1)) + else + printf '%-34s %-12s %s\n' "$repo" "v$stamp" "ahead of canonical (investigate)" + ahead=$((ahead + 1)) + fi +done + +printf '\n%d current, %d behind/unstamped, %d ahead, %d not-found.\n' \ + "$current" "$behind" "$ahead" "$missing" +if [[ "$behind" -gt 0 || "$ahead" -gt 0 ]]; then + echo "Action needed: re-run helmet onboarding on behind/unstamped repos; investigate any 'ahead'." + exit 1 +fi +if [[ "$missing" -gt 0 ]]; then + echo "INCOMPLETE: $missing repo(s) could not be read (not found / no access) — cannot certify them as current. Verify access and re-run." + exit 1 +fi +echo "No drift detected." diff --git a/skills/helmet/SKILL.md b/skills/helmet/SKILL.md index 9ba235d..aec198c 100644 --- a/skills/helmet/SKILL.md +++ b/skills/helmet/SKILL.md @@ -3460,6 +3460,9 @@ Detects commits pushed directly to `main` without a PR — i.e., admin bypasses ```yaml name: Admin Bypass Audit +# helmet-pipeline: v # ← STAMP the helmet version that generated this + # file (drift-detection reads it; see B4) + on: push: branches: [main] @@ -3483,7 +3486,7 @@ jobs: pull-requests: read # look up PR for commit SHA steps: - name: Harden Runner - uses: step-security/harden-runner@ab7a9404c0f3da075243ca237b5fac12c98deaa5 # v2.19.3 + uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4 with: egress-policy: audit - name: Detect direct-push bypass @@ -3495,52 +3498,47 @@ jobs: COMMIT_MSG: ${{ github.event.head_commit.message }} RUN_ID: ${{ github.run_id }} run: | - set -e - # Skip automated actors + set -e # NOT -euo pipefail: pipefail SIGPIPE-fails the head -3 pipe mid-run + # IDENTITY-based skip ONLY — automated actors. Do NOT skip on commit-message + # content ([skip ci]/chore(release)): that text is attacker-controllable, so a + # human bypasser could use it to evade. Bot release commits are skipped here. if [[ "$ACTOR" == *"[bot]"* ]] || [[ "$ACTOR" == "github-actions" ]]; then echo "Skipping audit — automated actor: $ACTOR"; exit 0 fi - # Skip release commits — `chore(release)` as first-line prefix, - # `[skip ci]` anywhere in full commit message. - FIRST_LINE=$(printf '%s' "$COMMIT_MSG" | head -1) - if [[ "$FIRST_LINE" == "chore(release)"* ]]; then - echo "Skipping audit — release commit"; exit 0 - fi - if printf '%s' "$COMMIT_MSG" | grep -qF '[skip ci]'; then - echo "Skipping audit — CI-skip marker"; exit 0 - fi - # Look up PRs — distinguish "API OK, no PR" (bypass) from "API failed" (skip). - # Never treat gh api failure as bypass — that creates false-positive issues. + # Look up PRs. Distinguish "API OK, no PR" (bypass) from "API failed/garbled" + # (INDETERMINATE → FAIL the run; never silently skip, never false-positive). if ! PRS_JSON=$(gh api "repos/$REPO/commits/$COMMIT_SHA/pulls" 2>&1); then - echo "::warning::gh api failed — skipping audit"; exit 0 + echo "::error::gh api failed for $COMMIT_SHA — failing run (status indeterminate)"; exit 1 fi - PR_COUNT=$(printf '%s' "$PRS_JSON" | jq 'length' 2>/dev/null || echo "0") - if [ "$PR_COUNT" != "0" ]; then - echo "No bypass — commit came from PR"; exit 0 + if ! PR_COUNT=$(printf '%s' "$PRS_JSON" | jq -e 'if type=="array" then length else error("not array") end' 2>/dev/null); then + echo "::error::unexpected PR-list response — failing run"; exit 1 fi + if [ "$PR_COUNT" != "0" ]; then echo "No bypass — came from PR"; exit 0; fi # No PR → direct push = bypass echo "::warning::Admin bypass: direct push to main by $ACTOR ($COMMIT_SHA)" + # NO dedup on purpose: issue title/body are MUTABLE (anyone with issues:write, + # incl. via editing a bot-authored issue) → unsafe as a suppression primitive; + # a duplicate on a rare manual re-run is harmless. See ADR-0001. gh label create "admin-bypass" --color "d93f0b" \ - --description "Commit bypassed required status checks" \ - --repo "$REPO" 2>/dev/null || true + --description "Commit bypassed required status checks" --repo "$REPO" 2>/dev/null || true # ... (body composition + gh issue create, see full template) ``` See `.github/workflows/bypass-audit.yml` in the helmet repo for the full template with issue body composition. **Detection logic:** -1. If actor is a bot (contains `[bot]` or equals `github-actions`) → skip -2. If commit message first line starts with `chore(release)` → skip (conventional commit) -3. If full commit message contains `[skip ci]` anywhere → skip -4. Look up PRs associated with commit SHA via `/commits/{sha}/pulls` API -5. **If `gh api` fails (rate limit, transient error) → log warning, skip** (do NOT create false-positive issue) -6. If API succeeded and zero associated PRs → direct-push bypass → warn + create issue -7. Otherwise → commit came from PR → no alert - -**Known limitations:** -- `gh pr merge --admin` DOES associate the merge commit with a PR, so this workflow won't detect that vector. Adding check-runs inspection would catch it but requires `administration: read` which is an elevated permission. -- Timing: this workflow runs AFTER the push lands, so it's detective control, not preventive. The gate already allowed the push; the audit surfaces it. -- Automated actors are globally skipped. If a compromised bot were to push directly to main, this workflow would not alert. Acceptable for solo dev; not acceptable for team repos. +1. If actor is a bot (`[bot]` suffix or `github-actions`) → skip. **IDENTITY-based skip only.** +2. **Do NOT skip on commit-message content** (`[skip ci]` / `chore(release)`): that text is attacker-controllable, so a human bypasser could use it to evade. Legitimate release commits are authored by a bot actor and are already skipped by rule 1. +3. Look up PRs associated with the commit SHA via `/commits/{sha}/pulls`. +4. **If `gh api` fails OR returns a non-array → FAIL the run** (`exit 1`, durable red-X in Actions history). Never silently skip, never coerce a garbled response into a false-positive bypass issue. +5. If API succeeded and zero associated PRs → direct-push bypass → warn + create issue. **No dedup** (see below / ADR-0001). +6. Otherwise → commit came from PR → no alert. + +**Known limitations (accepted):** +- `gh pr merge --admin` associates the merge commit with a PR, so this won't flag admin-merges — that is pr-grind's authorized path, logged to `.claude/bypass-log.jsonl`. +- A native CI-skip marker (`[skip ci]`, `[skip actions]`, …) in the head commit suppresses the whole workflow run. **Not an audit gap:** for org-owned repos the **GitHub org audit log** (`protected_branch.policy_override` events) is the authoritative, tamper-proof trail — this workflow is a convenience layer. A post-hoc "sweep" was evaluated and rejected (a workflow can't distinguish legitimate automation from a forged bypass after the fact). +- Detective, not preventive: runs after the push lands. +- **No dedup.** Issue title/body are mutable (anyone with `issues:write` can pre-create or edit a matching issue — even an author filter is defeated by editing a bot-authored issue), so a title/SHA dedup is an insider-editable suppression primitive. A duplicate issue on a rare manual re-run is the safer trade. See ADR-0001. **Key points:** - **Creates a permanent audit trail** — GitHub Issues are durable, searchable, and trigger notifications. Step-summary-only alerts get forgotten. From 904eedc24e60473c66bb5b71e3bc304cf7ba6615 Mon Sep 17 00:00:00 2001 From: Chris Yau Date: Fri, 5 Jun 2026 04:32:32 +0800 Subject: [PATCH 2/3] docs(helmet): align bypass-audit stamp note with drift-check source check-pipeline-drift.sh derives the canonical version from helmet's own bypass-audit.yml `# helmet-pipeline:` stamp (not plugin.json), so the onboarding instruction must tell the generator to copy that pipeline-template stamp verbatim and bump it only when the template changes. Stamping the plugin version (the old wording) would make every repo show false "ahead" drift on any unrelated plugin release. Also fix a dangling "see B4" pointer (B4 is multi-repo deployment, not drift detection) to reference ADR-0001, the authoritative decision record. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy --- skills/helmet/SKILL.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/skills/helmet/SKILL.md b/skills/helmet/SKILL.md index aec198c..694cb75 100644 --- a/skills/helmet/SKILL.md +++ b/skills/helmet/SKILL.md @@ -3460,8 +3460,12 @@ Detects commits pushed directly to `main` without a PR — i.e., admin bypasses ```yaml name: Admin Bypass Audit -# helmet-pipeline: v # ← STAMP the helmet version that generated this - # file (drift-detection reads it; see B4) +# helmet-pipeline: v # ← COPY helmet's OWN canonical bypass-audit.yml + # stamp verbatim. This is the pipeline-template + # version, bumped ONLY when the template changes — + # NOT the plugin version, NOT every release. + # check-pipeline-drift.sh reads helmet's canonical + # stamp as the source of truth; see ADR-0001. on: push: From 94188c7dedbe23283ac6aa9bf9abf784c8e48b1b Mon Sep 17 00:00:00 2001 From: Chris Yau Date: Fri, 5 Jun 2026 15:01:30 +0800 Subject: [PATCH 3/3] fix(ci): harden admin-bypass label-exists fallback Query the exact label endpoint instead of a page-capped label-list scan, so the existence check can't false-negative on a repo with many labels (codex P2 on PR #51). Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy --- .github/workflows/bypass-audit.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bypass-audit.yml b/.github/workflows/bypass-audit.yml index 4da3ddf..f76c6b9 100644 --- a/.github/workflows/bypass-audit.yml +++ b/.github/workflows/bypass-audit.yml @@ -151,7 +151,7 @@ jobs: --color "d93f0b" \ --description "Commit bypassed required status checks" \ --repo "$REPO" 2>/dev/null \ - || gh label list --repo "$REPO" --json name --jq '.[].name' 2>/dev/null | grep -qx "admin-bypass" \ + || 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.