Skip to content

feat(ci): adopt canonical no-dedup bypass-audit standard#181

Merged
chris-yyau merged 8 commits into
mainfrom
ci/converge-bypass-audit-canonical
Jun 5, 2026
Merged

feat(ci): adopt canonical no-dedup bypass-audit standard#181
chris-yyau merged 8 commits into
mainfrom
ci/converge-bypass-audit-canonical

Conversation

@chris-yyau

@chris-yyau chris-yyau commented Jun 5, 2026

Copy link
Copy Markdown
Owner

Converges busdriver's admin-bypass audit onto the canonical no-dedup standard (helmet ADR-0001, pipeline v1.21.1), superseding the closed dedup PR #178.

What changes

  • No dedup — dedup keyed on issue title/body is an insider-editable suppression vector (mutable metadata; an author filter doesn't fix it). For org-owned repos the org audit log is the authoritative trail; a duplicate issue on a rare re-run is harmless.
  • Identity-based skip only — skips automated actors via github.actor (unforgeable); no commit-message [skip ci]/chore(release) skip (attacker-forgeable).
  • Fail-on-indeterminate — a failed/garbled gh api PR lookup fails the run (durable red-X) rather than emitting a false-positive issue or silently passing.
  • Hardening beyond merged helmet v1.21.0: exact-label existence check; stderr separated from the PR-lookup JSON (a successful call's stderr warning no longer manufactures a false red-X); contents: read added for /commits/{sha}/pulls on private repos; audit-trail comments made org/user-owned accurate.
  • Stamped # helmet-pipeline: v1.21.1 for drift detection.

Validation

litmus PASS — 5 codex iterations hardened the canonical template itself (this PR carries those fixes; a helmet backport follows).

🤖 Generated with Claude Code


Summary by cubic

Adopts the canonical no-dedup admin-bypass audit: audits every commit in a push, flags those not merged via a PR into main, and files one summary issue. Adds fail-closed behavior and hardens the workflow (helmet-pipeline: v1.21.2).

  • Refactors

    • Remove dedup via issue metadata; skip only by github.actor (no message-based skips).
    • Read push commit SHAs from $GITHUB_EVENT_PATH (fallback github.sha); cap at 256, fail the run if payload is indeterminate or push exceeds cap; continue auditing remaining commits but fail the run on any indeterminate lookup or issue-creation failure.
    • File one summary issue listing all bypassing commits; ensure admin-bypass label exists with unlabeled fallback; sanitize commit subjects and render as inline code; separate gh api stderr from JSON; make pipes pipefail-safe.
    • Harden runtime: add contents: read, timeout 10m, set concurrency to bypass-audit-${{ github.sha }}, stamp helmet-pipeline: v1.21.2; clarify org audit-log access (Enterprise API or export) and note the squash-merge original-commit residual and CI-skip residual for user-owned repos.
  • Bug Fixes

    • Audit every pushed commit, not just HEAD, to catch earlier unaudited commits in multi-commit pushes.
    • Only treat PRs merged into main as non-bypass to avoid missing direct pushes that also appear in open/other-branch PRs.
    • Prevent Markdown injection in the summary by sanitizing commit subjects and rendering them as inline code.

Written for commit 4aed129. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved audit reliability: all pushed commits are now audited, API failures are treated as indeterminate/failing, only automated actors are exempted, and commit-message skips no longer bypass audits.
  • Chores

    • Hardened workflow behavior: longer timeout, per-run concurrency, bash as default shell, expanded read permissions, bounded audit work with safe limits, and single-summary issue creation with labeled-first fallback.

Replace the dedup-based admin-bypass audit with the canonical no-dedup
design (helmet ADR-0001, pipeline v1.21.1): identity-based skip only,
fail-on-indeterminate, exact-label existence check, separated stderr on
the PR lookup, and contents:read for private-repo PR association.

Dedup via mutable issue metadata is an insider-editable suppression
vector; for org repos the org audit log is the authoritative trail.

Stamped # helmet-pipeline: v1.21.1 for drift detection.

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The workflow's documentation, concurrency model, and core audit detection logic are refactored. The header now explicitly describes the push-time audit design and its accepted limitations. Identity-based actor skipping replaces commit-message checks. PR API failures and summary issue creation errors set AUDIT_FAILED and cause a non-zero exit. A single labeled-first summary issue consolidates detected bypasses.

Changes

Bypass Audit Workflow Refactor

Layer / File(s) Summary
Documentation and workflow configuration
.github/workflows/bypass-audit.yml
Workflow documentation header rewritten to describe the push-time audit design and limitations; concurrency group key changed to bypass-audit-${{ github.sha }}; defaults.run.shell set to bash; audit job timeout increased to 10 minutes; contents: read added to job permissions.
Actor skip logic
.github/workflows/bypass-audit.yml
Simplifies skip criteria to only bypass automated actors based on github.actor (bot suffix or github-actions); removes commit-message and release-based skip checks.
Commit selection and audit bounding
.github/workflows/bypass-audit.yml
Reads commit SHAs from the push payload via jq, falls back to github.sha when empty, and enforces MAX_AUDIT with AUDIT_FAILED when exceeded.
Per-commit PR association checks
.github/workflows/bypass-audit.yml
For each audited SHA calls GET /repos/:owner/:repo/commits/{sha}/pulls; treats API failures or unexpected/non-array responses as indeterminate (AUDIT_FAILED=1); classifies a bypass only when zero PRs merged into main; accumulates bypass rows for reporting.
Label existence handling
.github/workflows/bypass-audit.yml
Attempts idempotent create/lookup of admin-bypass label and sets LABEL_OK to determine whether to create the summary issue with the label.
Consolidated summary issue creation
.github/workflows/bypass-audit.yml
Creates a single summary issue containing actor, run, time, and a per-commit table; prefers labeled creation when LABEL_OK=1 and retries without label on failure; sets AUDIT_FAILED=1 if final creation fails.
Final failure control flow
.github/workflows/bypass-audit.yml
Exits non-zero (exit 1) when AUDIT_FAILED=1, covering indeterminate PR lookups or summary issue filing failures.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • chris-yyau/busdriver#177: Both PRs modify the same bypass-audit.yml issue-creation flow—one adds SHA-based deduplication while this one removes it—making the changes directly related.

Poem

🐰 I watched the pushes hop at night,
I parsed each SHA beneath moonlight,
Labels tried first, then fallback play,
No silent skips — the audit keeps sway. 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the primary change: adopting a canonical no-dedup bypass-audit standard as part of a workflow convergence, which matches the main objective of replacing the prior dedup approach with the helmet v1.21.1 standard.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

codescene-delta-analysis[bot]

This comment was marked as outdated.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Re-trigger cubic

The attacker-controlled commit message was rendered inside a fenced code
block in the audit issue; a fence marker (or a carriage return) in the
message could break out and inject Markdown. Render it as an indented
code block with CR stripped — which cannot be broken out of.

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/bypass-audit.yml (1)

92-97: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Persist the push pre-SHA and compare range in the audit issue.

The workflow issue body currently records only the head COMMIT_SHA (commit link) and the first 3 lines of the head commit message; it omits github.event.before, so reviewers can’t reconstruct the full before…after range for multi-commit pushes.

🧾 Proposed fix
       env:
         GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        BEFORE_SHA: ${{ github.event.before }}
         COMMIT_SHA: ${{ github.sha }}
         ACTOR: ${{ github.actor }}
         REPO: ${{ github.repository }}
         COMMIT_MSG: ${{ github.event.head_commit.message }}
         RUN_ID: ${{ github.run_id }}
@@
           printf '| Field | Value |\n'
           printf '|-------|-------|\n'
           printf '| **Commit** | [\`%s\`](https://github.com/%s/commit/%s) |\n' "$COMMIT_SHA" "$REPO" "$COMMIT_SHA"
+          if [ "$BEFORE_SHA" != "0000000000000000000000000000000000000000" ]; then
+            printf '| **Range** | [\`%s...%s\`](https://github.com/%s/compare/%s...%s) |\n' \
+              "$BEFORE_SHA" "$COMMIT_SHA" "$REPO" "$BEFORE_SHA" "$COMMIT_SHA"
+          fi
           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"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/bypass-audit.yml around lines 92 - 97, The audit issue
body currently only records COMMIT_SHA and COMMIT_MSG; update the workflow to
also capture and persist the pre-push SHA (github.event.before) and a full
compare range string (e.g., "{github.event.before}..{github.sha}") so reviewers
can reconstruct the exact before…after range; add environment variables like
PRE_COMMIT_SHA: ${{ github.event.before }} and COMPARE_RANGE: ${{
github.event.before }}..${{ github.sha }} alongside COMMIT_SHA, and use those
variables when building the issue body or links in the job that creates the
audit issue.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/bypass-audit.yml:
- Around line 104-108: The current identity check using ACTOR with the wildcard
'"$ACTOR" == *"[bot]"*' and the exact '"$ACTOR" == "github-actions"' is too
broad; replace it with an explicit allowlist of trusted automation actor names
(e.g., canonical GitHub Actions/service accounts and dependabot identifiers) or
normalize ACTOR by stripping a trailing "[bot]" suffix and then perform exact
comparisons against a small trusted set, and update the if condition that
currently evaluates ACTOR to use that allowlist/normalized comparison instead of
the wildcard match.

---

Outside diff comments:
In @.github/workflows/bypass-audit.yml:
- Around line 92-97: The audit issue body currently only records COMMIT_SHA and
COMMIT_MSG; update the workflow to also capture and persist the pre-push SHA
(github.event.before) and a full compare range string (e.g.,
"{github.event.before}..{github.sha}") so reviewers can reconstruct the exact
before…after range; add environment variables like PRE_COMMIT_SHA: ${{
github.event.before }} and COMPARE_RANGE: ${{ github.event.before }}..${{
github.sha }} alongside COMMIT_SHA, and use those variables when building the
issue body or links in the job that creates the audit issue.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 33c58693-eb4c-45c4-9fdc-f9224db4d1fa

📥 Commits

Reviewing files that changed from the base of the PR and between d372fec and dd258b9.

📒 Files selected for processing (1)
  • .github/workflows/bypass-audit.yml

Comment thread .github/workflows/bypass-audit.yml
codescene-delta-analysis[bot]

This comment was marked as outdated.

A multi-commit direct push whose HEAD is PR-associated (e.g. a re-pushed
already-merged commit) could hide earlier unaudited commits. Loop over
github.event.commits and check each commit's PR association, filing one
issue per un-PR'd commit and failing the run on any indeterminate lookup.

GitHub caps the push payload commit list at 20; the remainder is an
accepted residual (the org audit log records every commit).

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
codescene-delta-analysis[bot]

This comment was marked as outdated.

Replace per-commit issue creation with a single summary issue listing all
bypassing commits in a table — avoids GitHub's content-creation rate limit
on a multi-commit bypass and never false-positives a large legitimate
merge. Commit subjects are sanitized and inline-code-wrapped (no Markdown
injection). Extract the first line via bash parameter expansion, not head,
so the GitHub `-eo pipefail` shell can't SIGPIPE-abort on a huge message.

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
codescene-delta-analysis[bot]

This comment was marked as outdated.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread .github/workflows/bypass-audit.yml
The audit-log API requires GitHub Enterprise Cloud; non-Enterprise orgs
use the audit-log export. Drop the contested web-UI claim.

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
codescene-delta-analysis[bot]

This comment was marked as outdated.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c4157f2539

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/bypass-audit.yml Outdated
COMMIT_MSG: ${{ github.event.head_commit.message }}
RUN_ID: ${{ github.run_id }}
HEAD_SHA: ${{ github.sha }}
COMMITS_JSON: ${{ toJSON(github.event.commits) }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Read the push payload from the event file

When a push contains many commits or large commit messages, toJSON(github.event.commits) can exceed the OS per-environment-string limit before the step's shell starts, so the audit never reaches the new MAX_AUDIT handling or creates the summary issue. Since this workflow now depends on the full commit list, read commits from $GITHUB_EVENT_PATH inside the script instead of serializing the whole array through env.

Useful? React with 👍 / 👎.

`/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 read as "no bypass" — an undetected
bypass. Filter to PRs with merged_at != null AND base.ref == main
(matches seatbelt's sweep design).

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

GitHub's commit->PR association marks a SHA as belonging to a merged PR
even for the original commits of a squash-merged PR (which never landed on
main). A direct push of one of those exact SHAs is not flagged — an
accepted, irreducible residual of the association API; the org audit log
records the actual push.

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@chris-yyau chris-yyau force-pushed the ci/converge-bypass-audit-canonical branch from 1e932a3 to 9d00994 Compare June 5, 2026 13:15
codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

toJSON(github.event.commits) materialized the whole push payload (up to
2048 commits, each with message + file lists + URLs) into a single env
var. With a large push that can exceed the runner's env/ARG_MAX limit
(~2 MB), making the audit step fail to exec before any logic runs —
silently suppressing the very bypass this workflow exists to detect.

Read commit ids and messages from $GITHUB_EVENT_PATH with jq instead
(file data, never shell-interpolated), and fail the run when the payload
is unreadable, not JSON, or .commits is not an array (indeterminate), so
a multi-commit bypass cannot slip through a silent HEAD-only fallback. A
valid but empty commit list still legitimately falls back to HEAD.

Bumps the helmet-pipeline stamp v1.21.1 -> v1.21.2. Flagged independently
by codex and cubic on the converged fleet PRs.

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@chris-yyau chris-yyau force-pushed the ci/converge-bypass-audit-canonical branch from 4829f5a to 4aed129 Compare June 5, 2026 14:15
@chris-yyau chris-yyau merged commit ba03bec into main Jun 5, 2026
21 checks passed
@chris-yyau chris-yyau deleted the ci/converge-bypass-audit-canonical branch June 5, 2026 14:35
chris-yyau pushed a commit that referenced this pull request Jun 5, 2026
# [1.56.0](v1.55.0...v1.56.0) (2026-06-05)

### Features

* **ci:** adopt canonical no-dedup bypass-audit standard ([#181](#181)) ([ba03bec](ba03bec))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant