ci: vendor governance-enforce — this repo was never in the A_BLOCK ruleset - #27
ci: vendor governance-enforce — this repo was never in the A_BLOCK ruleset#27yakimoto wants to merge 4 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_acffa972-9cef-43ca-8492-c256cba1317d) |
|
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: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Comment |
ApprovabilityVerdict: Approved d0277f0 New CI workflow adding governance/secrets enforcement, authored by the designated CODEOWNER. Unresolved review comments are verification suggestions or appear stale rather than substantive bugs. Standard CI configuration addition. You can customize Macroscope's approvability policy. Learn more. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
PR Summary by QodoCI: add governance-enforce A_BLOCK diff-scoped gate
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}
| - name: fetch governance enforcer (isolated install) | ||
| run: | | ||
| mkdir -p "$RUNNER_TEMP/gov" && cd "$RUNNER_TEMP/gov" | ||
| printf '@wave-av:registry=https://npm.pkg.github.com\n//npm.pkg.github.com/:_authToken=${NODE_AUTH_TOKEN}\n' > .npmrc | ||
| npm install @wave-av/governance@^0.4.4 --no-save --no-audit --no-fund |
There was a problem hiding this comment.
🔍 Fork PRs will run this gate but may not be able to read the private org package
This repo is PUBLIC (see the rationale comments in .github/workflows/foundation-gate.yml:1-10 and .github/workflows/public-repo-guard.yml), so pull_request runs from forks are expected. Those runs get a read-only GITHUB_TOKEN scoped to the base repo; if @wave-av/governance on npm.pkg.github.com is not granted org/internal read visibility, the isolated npm install at line 48 returns 401 and the enforce job fails for every external contributor's PR — indistinguishable from a real A_BLOCK violation. The header comment cites wave-av/cli#20 as proof, but a same-repo branch PR does not exercise the fork token path. Worth verifying with one fork PR before this check is added to governance-a-block-enforce.
Was this helpful? React with 👍 or 👎 to provide feedback.
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. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 1 file
Confidence score: 3/5
- In
.github/workflows/governance-enforce.yml, using a caret-pinned governance package with default npm lifecycle execution makes the security gate non-deterministic, so a future upstream release could silently change enforcement behavior and create unexpected pass/fail outcomes — pin an exact immutable version (and consider lockfile/npm cistyle installs) to stabilize checks. - In
.github/workflows/governance-enforce.yml,pull_requestruns from forks may not be able to read@wave-av/governancefromnpm.pkg.github.comwith the default read-onlyGITHUB_TOKEN, which can cause governance checks to fail for external contributors and block PR flow — add a fork-safe auth strategy or a fallback path for forked PRs.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/governance-enforce.yml">
<violation number="1" location=".github/workflows/governance-enforce.yml:48">
P2: The security gate is not deterministic: the caret range and default npm lifecycle execution allow a later package release or dependency to alter enforcement without a workflow change. Pin an immutable package version with dependency integrity and install with lifecycle scripts disabled where the enforcer does not require them.</violation>
<violation number="2" location=".github/workflows/governance-enforce.yml:48">
P2: This is a public repo, so `pull_request` runs from forks are expected, and fork runs get a read-only `GITHUB_TOKEN` scoped to the base repo. If `@wave-av/governance` on `npm.pkg.github.com` isn't readable with that token for external contributors, this install step will 401 and fail the required `enforce` check for every fork PR, indistinguishable from a real A_BLOCK violation. Worth verifying with an actual fork PR before adding this check to the `governance-a-block-enforce` required-check ruleset.</violation>
</file>
Architecture diagram
sequenceDiagram
participant GH as GitHub Actions
participant Runner as CI Runner (Transient)
participant Registry as GitHub Packages (@wave-av)
participant Git as Git History
participant Status as PR Status Checks
Note over GH,Status: NEW: governance-enforce Workflow
GH->>Runner: pull_request event
Runner->>Git: checkout (fetch-depth: 0)
Note over Runner,Registry: Isolated Tooling Installation
Runner->>Runner: NEW: Write .npmrc to RUNNER_TEMP
Note right of Runner: Uses ephemeral GITHUB_TOKEN<br/>for internal package access
Runner->>Registry: NEW: npm install @wave-av/governance
Registry-->>Runner: Return enforcer package
Note over Runner,Git: Diff-Scoped Detection
alt pull_request
Runner->>Runner: Identify PR_BASE_SHA
else push
Runner->>Runner: Identify PUSH_BEFORE_SHA
end
Runner->>Git: git diff [BASE]...HEAD
Runner->>Runner: NEW: Exec enforce.mjs (bin)
alt Secrets or Hardcoded Paths Found in Diff
Runner-->>Status: NEW: Report 'enforce' check FAILURE
Note over Status: Blocks Merge (once added to Ruleset)
else No New Violations
Runner-->>Status: NEW: Report 'enforce' check SUCCESS
Note over Status: Liveness Drill: Verifies repo can run check
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| run: | | ||
| mkdir -p "$RUNNER_TEMP/gov" && cd "$RUNNER_TEMP/gov" | ||
| printf '@wave-av:registry=https://npm.pkg.github.com\n//npm.pkg.github.com/:_authToken=${NODE_AUTH_TOKEN}\n' > .npmrc | ||
| npm install @wave-av/governance@^0.4.4 --no-save --no-audit --no-fund |
There was a problem hiding this comment.
P2: The security gate is not deterministic: the caret range and default npm lifecycle execution allow a later package release or dependency to alter enforcement without a workflow change. Pin an immutable package version with dependency integrity and install with lifecycle scripts disabled where the enforcer does not require them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/governance-enforce.yml, line 48:
<comment>The security gate is not deterministic: the caret range and default npm lifecycle execution allow a later package release or dependency to alter enforcement without a workflow change. Pin an immutable package version with dependency integrity and install with lifecycle scripts disabled where the enforcer does not require them.</comment>
<file context>
@@ -0,0 +1,59 @@
+ run: |
+ mkdir -p "$RUNNER_TEMP/gov" && cd "$RUNNER_TEMP/gov"
+ printf '@wave-av:registry=https://npm.pkg.github.com\n//npm.pkg.github.com/:_authToken=${NODE_AUTH_TOKEN}\n' > .npmrc
+ npm install @wave-av/governance@^0.4.4 --no-save --no-audit --no-fund
+ - name: A_BLOCK enforce (secrets + hardcoded paths on the diff)
+ env:
</file context>
| run: | | ||
| mkdir -p "$RUNNER_TEMP/gov" && cd "$RUNNER_TEMP/gov" | ||
| printf '@wave-av:registry=https://npm.pkg.github.com\n//npm.pkg.github.com/:_authToken=${NODE_AUTH_TOKEN}\n' > .npmrc | ||
| npm install @wave-av/governance@^0.4.4 --no-save --no-audit --no-fund |
There was a problem hiding this comment.
P2: This is a public repo, so pull_request runs from forks are expected, and fork runs get a read-only GITHUB_TOKEN scoped to the base repo. If @wave-av/governance on npm.pkg.github.com isn't readable with that token for external contributors, this install step will 401 and fail the required enforce check for every fork PR, indistinguishable from a real A_BLOCK violation. Worth verifying with an actual fork PR before adding this check to the governance-a-block-enforce required-check ruleset.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/governance-enforce.yml, line 48:
<comment>This is a public repo, so `pull_request` runs from forks are expected, and fork runs get a read-only `GITHUB_TOKEN` scoped to the base repo. If `@wave-av/governance` on `npm.pkg.github.com` isn't readable with that token for external contributors, this install step will 401 and fail the required `enforce` check for every fork PR, indistinguishable from a real A_BLOCK violation. Worth verifying with an actual fork PR before adding this check to the `governance-a-block-enforce` required-check ruleset.</comment>
<file context>
@@ -0,0 +1,59 @@
+ run: |
+ mkdir -p "$RUNNER_TEMP/gov" && cd "$RUNNER_TEMP/gov"
+ printf '@wave-av:registry=https://npm.pkg.github.com\n//npm.pkg.github.com/:_authToken=${NODE_AUTH_TOKEN}\n' > .npmrc
+ npm install @wave-av/governance@^0.4.4 --no-save --no-audit --no-fund
+ - name: A_BLOCK enforce (secrets + hardcoded paths on the diff)
+ env:
</file context>
…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_154d443d-c097-4e68-8855-4535810380a8) |
| # pointing at a commit this checkout no longer contains. | ||
| 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:80
When a force-push makes github.event.before unreachable, the fallback selects HEAD~1 and the gate scans only the newest commit. A force-push that introduces multiple commits can hide secrets in the earlier commits — they never appear in the HEAD~1..HEAD diff, so the gate reports green without examining them. This contradicts the workflow's stated goal of failing closed on unverified input. Consider falling back to the empty tree (as the no-base branch already does) instead of HEAD~1, so the entire force-pushed history is scanned.
- BASE="$(git rev-parse --verify --quiet 'HEAD~1' || true)"🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @.github/workflows/governance-enforce.yml around line 80:
When a force-push makes `github.event.before` unreachable, the fallback selects `HEAD~1` and the gate scans only the newest commit. A force-push that introduces multiple commits can hide secrets in the earlier commits — they never appear in the `HEAD~1..HEAD` diff, so the gate reports green without examining them. This contradicts the workflow's stated goal of failing closed on unverified input. Consider falling back to the empty tree (as the no-base branch already does) instead of `HEAD~1`, so the entire force-pushed history is scanned.
Evidence trail:
.github/workflows/governance-enforce.yml:74-93 @ 31eeb1c7
| 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 SHA as --changed argument may not be accepted by the enforcer
The fail-closed path passes the empty tree object id to node enforce.mjs --changed "$BASE". This only works if the enforcer diffs with a two-dot form (git diff <base> HEAD) or git diff --name-only <base>. If it uses the three-dot form (git diff base...HEAD), git needs a merge base and will error out on a tree object, so the whole-tree fallback would abort rather than scan. The package is not vendored here, so this could not be verified from the repo. Worth confirming against @wave-av/governance@0.4.6 behaviour (ideally by exercising the root-commit/unreachable-base path once).
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Verified against the enforcer source: --changed uses the two-argument form git diff <base> HEAD (changedArgs() in governance/lib/git-files.mjs), explicitly documented to accept tree objects like the empty tree, and its file lister fails closed (non-zero exit) on any git error rather than reporting a pass.
…AD~1 Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
| if [ -z "$BASE" ] || [ "$BASE" = "0000000000000000000000000000000000000000" ] \ | ||
| || ! git cat-file -e "$BASE^{commit}" 2>/dev/null; then | ||
| BASE="$(git hash-object -t tree /dev/null)" | ||
| echo "::notice::no verifiable diff base (root commit, branch creation, or force-push with unreachable before-sha) — scanning the whole tree against the empty tree" | ||
| fi |
There was a problem hiding this comment.
🔍 Empty-tree fallback turns the gate into a whole-repo scan that can fail on legacy debt
The fail-closed path replaces an unverifiable base with the empty-tree hash, so every tracked file reads as added and the enforcer scans the entire repository. That contradicts the header claim that this gate is "diff-scoped: blocks NEW violations without failing on legacy debt" — on a force-push (or any push whose before sha is unreachable), the run will report every pre-existing A_BLOCK violation in the repo, potentially wedging pushes to main until legacy debt is cleaned or a suppression list exists. The trade-off is deliberate per the comment, but it's worth confirming this repo currently scans clean against the empty tree before the check is made required (which the header also warns about).
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
The trade-off is deliberate and documented at the fallback site; I ran the requested verification and this repo scans clean against the empty tree (enforce.mjs exit 0, 0 A_BLOCK violations across all 34 tracked files), so the whole-repo fallback cannot block on legacy debt.
…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.
| PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} | ||
| MERGE_BASE_SHA: ${{ github.event.merge_group.base_sha }} | ||
| PUSH_BEFORE_SHA: ${{ github.event.before }} | ||
| run: | | ||
| set -euo pipefail | ||
| ENFORCE="$RUNNER_TEMP/gov/node_modules/@wave-av/governance/bin/enforce.mjs" | ||
| BASE="${PR_BASE_SHA:-}" |
There was a problem hiding this comment.
🔍 pull_request base.sha vs merge-commit checkout can widen the scanned range
For pull_request, github.event.pull_request.base.sha is the base tip at event time while the checkout is refs/pull/N/merge, which merges in the current base tip. If the enforcer uses a two-dot diff, commits landed on the base branch after the event are attributed to this PR and can fail it on someone else's changes; a three-dot diff avoids this. Passing the merge base (or relying on the enforcer's three-dot semantics) is worth confirming.
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 change with no application runtime impact; it tightens supply-chain hygiene around the enforcer install and fail-closed scanning behavior.
Overview
Adds
governance-enforce, a new GitHub Actions workflow that runs the org A_BLOCK gate on PRs and pushes tomain/mastervia@wave-av/governance@0.4.6(secrets-in-git, Doppler expectations, hardcoded paths), diff-scoped so only new violations fail.The enforcer is installed in an isolated
$RUNNER_TEMPtree with--ignore-scripts, registry auth limited to the install step, SHA-pinned actions, andpackages: readonly. Diff base resolution is hardened: invalid or missing bases fall back toHEAD~1, then to the empty tree so the job cannot pass with zero files scanned; push runs useconcurrencyso superseded runs do not leave commits unexamined.The workflow is intentionally not wired into the org ruleset until this check is green here, avoiding a required check that never reports.
Reviewed by Cursor Bugbot for commit 31eeb1c. 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 d0277f0.