ci: vendor governance-enforce — this repo was never in the A_BLOCK ruleset - #42
ci: vendor governance-enforce — this repo was never in the A_BLOCK ruleset#42yakimoto wants to merge 7 commits into
Conversation
…never in the ruleset
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b3d25476-41cf-41c6-8a71-ba1dd92af023) |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 44 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Comment |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review Unable to check for correctness in 6cf0c37. New CI workflow adding a secrets/governance scanning gate with non-trivial edge-case handling logic. While the author owns this file, the security implications of correctly handling all push/PR scenarios (force-push, branch creation, unreachable commits) warrant human verification that the fallback behavior is correct. You can customize Macroscope's approvability policy. Learn more. |
PR Summary by QodoAdd governance-enforce A_BLOCK diff-scoped secrets/path gate to CI
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…push) Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Qodo FixerNo findings are within the configured fix scope. To change which findings are fixed, adjust the setting on your Qodo configuration page. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…EAD has no parent Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…othing Five defects, none of them cosmetic. Refs wave-av/claude-workstation#1747. 1. FAIL-OPEN DIFF BASE. `BASE=$(git rev-parse HEAD~1 2>/dev/null || git rev-parse HEAD)` — on a root commit `git rev-parse HEAD~1` prints its unresolved argument to stdout AND fails, so the `||` branch appends and BASE becomes a two-line string. `git diff` then exits 128, and the pinned enforcer turned that into zero files and a green check. Now: a reachability-checked base (a force-push can leave `github.event.before` pointing at a commit this checkout does not have), and with no resolvable base at all it diffs against the EMPTY TREE so the whole repo is scanned rather than nothing. 2. THE PINNED ENFORCER ITSELF FAILED OPEN. `^0.4.4` resolved to 0.4.4, whose file lister is `catch { return []; }` — any git error became zero files and rendered as `OK[enforce]: 0 changed file(s) scanned — 0 A_BLOCK violations`. A git error and a clean diff were byte-identical in the output. The fix had sat unreleased on claude-workstation main since 2026-07-29 because no `governance-v*` tag was ever pushed. Released now as 0.4.6 and pinned exactly here. 3. TOKEN IN SCOPE FOR THE WRONG STEPS. `NODE_AUTH_TOKEN` was job-level, so it was also in the environment of the step that executes the downloaded package. Now step-scoped, and the .npmrc holding it is removed on exit. 4. INSTALL SCRIPTS RAN WITH THAT TOKEN. `npm install` runs preinstall/postinstall by default. Added `--ignore-scripts`. 5. CANCELLED PUSH RUNS WERE SCANNED BY NOBODY. `cancel-in-progress: true` applied to push runs, and each push run only diffs its own before..HEAD range — so a cancelled run's commits were never examined by anything. Now PR-only. Also: `timeout-minutes: 10` and `set -euo pipefail`. Receipt, against a scratch repo whose root commit carries a no-hardcoded-paths violation, simulating a branch-creation push (`before` = all zeros): old logic -> malformed base -> caught error -> [] -> OK, 0 files scanned, PASS new logic -> "no diff base resolved ... scanning the whole tree" -> BLOCK, exit 1 Credit where it is due: several of these were found by the review bots on the sibling vendoring PRs and are folded in here — the step-scoped token, the .npmrc cleanup, the exact pin, `--ignore-scripts`, the force-push reachability check, `timeout-minutes`, and the concurrency hole (5), which was crest-console#7's catch and which I had missed entirely.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_131e5d02-41c0-4caa-a7e5-5508dc37d3b4) |
| if [ -z "$BASE" ] || [ "$BASE" = "0000000000000000000000000000000000000000" ] \ | ||
| || ! git cat-file -e "$BASE^{commit}" 2>/dev/null; then | ||
| BASE="$(git rev-parse --verify --quiet 'HEAD~1' || true)" |
There was a problem hiding this comment.
🟠 High workflows/governance-enforce.yml:78
When a force-push makes github.event.before unreachable in the checkout, the fallback at line 80 narrows the scan to HEAD~1, so only the final commit's diff is examined. Any earlier commits introduced by that force-push are skipped entirely, and a secret or hardcoded path pushed in one of those earlier commits reaches main without this gate examining it. Instead of silently narrowing the diff to HEAD~1, the fallback should fail closed or compute a reachable merge-base.
if [ -z "$BASE" ] || [ "$BASE" = "0000000000000000000000000000000000000000" ] \
|| ! git cat-file -e "$BASE^{commit}" 2>/dev/null; then
- BASE="$(git rev-parse --verify --quiet 'HEAD~1' || true)"
+ BASE="$(git merge-base --octopus HEAD 2>/dev/null || true)"
fi🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @.github/workflows/governance-enforce.yml around lines 78-80:
When a force-push makes `github.event.before` unreachable in the checkout, the fallback at line 80 narrows the scan to `HEAD~1`, so only the final commit's diff is examined. Any earlier commits introduced by that force-push are skipped entirely, and a secret or hardcoded path pushed in one of those earlier commits reaches `main` without this gate examining it. Instead of silently narrowing the diff to `HEAD~1`, the fallback should fail closed or compute a reachable merge-base.
Evidence trail:
.github/workflows/governance-enforce.yml:74-93 @ 67d046c0; `git blame REVIEWED_COMMIT -L 74,91 -- .github/workflows/governance-enforce.yml`
| if [ -z "$BASE" ]; then | ||
| BASE="$(git hash-object -t tree /dev/null)" | ||
| echo "::notice::no diff base resolved (root commit or unreachable before-sha) — scanning the whole tree against the empty tree" | ||
| fi | ||
| echo "diffing against $BASE" | ||
| exec node "$ENFORCE" --changed "$BASE" |
There was a problem hiding this comment.
🔍 Empty-tree fallback depends on the enforcer accepting a tree-ish base
The fail-closed path substitutes the empty tree object (git hash-object -t tree /dev/null) as the --changed argument. This only produces a whole-tree scan if @wave-av/governance@0.4.6's enforcer runs a two-dot diff (git diff <base> HEAD) or git diff --name-only <base>. If it uses three-dot (<base>...HEAD) or resolves the argument via rev-parse <base>^{commit}, the empty tree hash is not a commit and the command will error — which, given 0.4.6 is described as failing closed on git errors, would turn every unresolvable-base run (root commit, unreachable force-push before-sha) into a hard red job rather than a full scan. Worth confirming against the package's bin/enforce.mjs behavior once observed in a real run.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Verified against the 0.4.6 source: changedArgs uses the two-argument git diff <base> HEAD form, which accepts trees by explicit design (its doc comment cites this exact empty-tree CI fallback), and running the enforcer with the empty-tree base against this repo scans all tracked files and exits 0. Both enforce.mjs and the spawned enforce-ramp.mjs share that code path, and neither resolves the base via ^{commit} or three-dot merge-base.
… the scan to HEAD~1 Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…l a false pass Correction to the previous commit on this branch. Refs wave-av/claude-workstation#1747. That commit replaced the fail-open `BASE=HEAD` with a fallback to `HEAD~1`. `HEAD~1` is also wrong: it scans exactly ONE commit, so a five-commit push whose base is indeterminate (branch creation, force-push, unreachable `github.event.before`) examines the last commit and reports a confident pass on the other four. A narrowed scan reported as a full pass is the same defect in a quieter costume. Receipt — scratch repo, five-commit push, violation planted in commit 1: HEAD~1 base -> OK[enforce]: 1 changed file(s) scanned -> PASS (never saw it) empty-tree base -> 5 changed file(s) scanned -> BLOCK[enforce]: no-hardcoded-paths, exit 1 Now: with no resolvable base of any kind, diff against git's empty-tree object so every tracked file reads as added and the whole repo is scanned. Loud, never partial, never empty. Credit: wave-av/wave-rig's copy on main already had this right, with the reasoning in a comment ("HEAD~1 would skip earlier commits in a multi-commit push and let a violation through"). The fan-out copied the broken shape from elsewhere and I did not check the one repo that had already solved it. Also from wave-rig: `merge_group` is now a declared trigger and `github.event.merge_group. base_sha` joins the base chain. None of these repos runs a merge queue today, so the trigger is inert — but a required check that never reports on an event the repo actually uses is a permanent deadlock, and this closes that in advance rather than after someone hits it.
| # So: no resolvable base of any kind → diff against the EMPTY TREE, which makes every | ||
| # tracked file read as added and scans the whole repo. Loud, never partial, never empty. | ||
| # (`--all` also exists in 0.4.6 and would do most of this, but it is documented as NOT | ||
| # covering the diff-scoped over-grant detectors. Routing through the diff path with an | ||
| # empty base keeps every detector in play.) | ||
| if [ -z "$BASE" ] || [ "$BASE" = "0000000000000000000000000000000000000000" ] \ | ||
| || ! git cat-file -e "$BASE^{commit}" 2>/dev/null; then | ||
| BASE="$(git hash-object -t tree /dev/null)" | ||
| echo "::warning::indeterminate diff base (root commit, branch creation, or unreachable before-sha) — scanning the full tree against the empty-tree object so no commit is skipped" | ||
| fi |
There was a problem hiding this comment.
🔍 Empty-tree fallback turns the diff-scoped gate into a full-repo scan that will trip on legacy debt
The header states the gate is diff-scoped precisely so it "blocks NEW violations without failing on legacy debt" (lines 4-5). The new fallback makes every tracked file read as added, so any pre-existing hardcoded path or secret-shaped string anywhere in the repo will fail the job. On pull_request this cannot happen (base.sha is always present and fetched), but on a force-pushed branch's push to main, or a root commit, the job would fail for reasons unrelated to the pushed change. That is the intended "loud" behavior, but on push to main it produces a red default-branch build that no one can fix without whitelisting legacy debt — consider whether the fallback should warn-and-scan-full instead of blocking on non-PR events.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Adds the
governance-enforceA_BLOCK gate (secrets / hardcoded-paths, diff-scoped) to this repo. Part of claude-workstation#1624 E4 T4.9a, following thewave-av/cli#20pilot.Why this repo had no secrets scan
The org ruleset
governance-a-block-enforce(17901847) requires anenforcecheck across the fleet. Its scope is an explicit include list of 112 hand-maintained repository names — and every one of them matcheswave-*.The 16 public repos absent from that list are exactly the 16 not named
wave-*:.github,adk,api-spec,cli,companion-module-wave,create-wave-app,crest-console,dispatch-edge,examples,mcp-server,obs-wave-plugin,sdk,sdk-python,sdks,vmix-wave-integration,workflow-sdk.Read the intersection: the repos that publish our npm packages are precisely the repos running with no A_BLOCK secrets scan. Nobody excluded them. A naming convention silently became a security boundary, and it drew the line in the worst possible place.
Why the workflow lands before the ruleset entry
Adding a repo to a
required_status_checksruleset before it emits that check is a permanent deadlock — a required check that never reports can never go green, and every PR on the repo becomes unmergeable. So the order is: vendor the workflow, observe it green, then extend the list. Doing it the intuitive way round would have bricked all sixteen.This PR is also its own liveness drill. The workflow triggers on
pull_request, so it runs on the PR that adds it. Ifenforcereports green here, the vendored shape works in this repo. If it does not, nothing was required and nothing is blocked — which is the point of this ordering.Proven before fan-out, not assumed
@wave-av/governanceis aninternal-visibility package owned byclaude-workstation, so whether a public repo'sGITHUB_TOKENcan read it was the one real assumption. Rather than fan out on the inference, it was piloted on a single repo first:That is the receipt this PR rides on. The shape is copied verbatim from
wave-av/wave-moq-edge(public, 12/12 green), which matters becauseauto-approve.ymlfails silently on every public repo — it calls a reusable workflow in the privatewave-foundation, and a public repo cannot do that (parse-time failure, zero jobs, no annotation). This workflow calls nothing cross-repo, so that trap does not apply.Security properties, unchanged from the source:
actions/checkout@df4cb1c,actions/setup-node@48b55a0)persist-credentials: falseon checkoutcontents: read+packages: readRUNNER_TEMP,--no-save, so nothing touches this repo's dependency tree.npmrcis written with a literal${NODE_AUTH_TOKEN}(single-quotedprintf) which npm expands at run time — no secret value is ever written to disk or a log${{ }}inputs (base.sha,event.before) are routed throughenv:and read as"$VAR", never interpolated into the script bodyDiff-scoped by design: it blocks new violations without failing on legacy debt.
Refs wave-av/claude-workstation#1624.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.Note
Low Risk
CI-only addition with read-only permissions and isolated temp install; no application runtime or merge-blocking ruleset change until the workflow is verified green.
Overview
Adds
.github/workflows/governance-enforce.yml, bringing this repo into the org A_BLOCK fan-out (secrets-in-git, Doppler expectations, hardcoded paths) via@wave-av/governance@0.4.6. The workflow runs on PRs and pushes tomain/master, emits theenforcejob check, and is meant to go green here before the repo is added togovernance-a-block-enforce(required check with no reporter = deadlock).The vendored shape is hardened: step-scoped
GITHUB_TOKENfor npm,--ignore-scriptson install, exact package pin, push concurrency that does not cancel in-progress push runs (so commits aren’t left un-scanned), and fail-closed diff-base resolution (empty tree vsHEADself-diff). Enforcement is diff-scoped so new violations block without failing on legacy debt.Reviewed by Cursor Bugbot for commit 67d046c. Configure here.
Note
Add governance enforcement workflow to scan diffs on PRs and pushes to main
Adds governance-enforce.yml, which runs
@wave-av/governance@0.4.6on pull requests, merge groups, and pushes to main/master. The enforcer scans only changed files using a computed diff base; if no valid base is found, it falls back to the empty tree and scans the full repository. Push and merge group runs are not cancelable to ensure no committed ranges are skipped.Macroscope summarized 6cf0c37.