ci: vendor governance-enforce — this repo was never in the A_BLOCK ruleset - #76
ci: vendor governance-enforce — this repo was never in the A_BLOCK ruleset#76yakimoto wants to merge 3 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_0ce30bf3-926e-4aee-8026-e1a3b22fcb0c) |
|
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: Needs human review Unable to check for correctness in 04ec38d. Multiple reviewers have identified that fork PRs may fail to install the private @wave-av/governance package, potentially blocking all external contributions once this check is required. This design issue warrants human review to determine the intended fork PR handling. You can customize Macroscope's approvability policy. Learn more. |
PR Summary by QodoVendor A_BLOCK governance-enforce CI gate (diff-scoped secrets/path checks)
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
| - 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 may not be able to install the internal governance package
CONTRIBUTING.md instructs external contributors to fork this public repo. For pull_request events from forks, secrets.GITHUB_TOKEN is downgraded to read-only and its ability to read an internal-visibility package owned by another org (claude-workstation) is not established by the pilot cited in the description (wave-av/cli#20 was a same-repo branch PR). If the install step 401s on fork PRs, this job fails on every fork contribution, and would become an unmergeable required check once the repo is added to governance-a-block-enforce. Worth verifying with a fork PR before extending the ruleset.
Was this helpful? React with 👍 or 👎 to provide feedback.
Code Review by Qodo
1.
|
| on: | ||
| pull_request: | ||
| push: | ||
| branches: [main, master] | ||
|
|
||
| permissions: | ||
| contents: read | ||
| packages: read | ||
|
|
There was a problem hiding this comment.
2. Fork pr package access risk 🐞 Bug ☼ Reliability
The workflow always runs on pull_request and unconditionally attempts an authenticated install from npm.pkg.github.com; if @wave-av/governance is not publicly readable to the PR token (common for internal/private packages), the job will fail for fork PRs and can block external contributions once this check is required. There is no conditional handling or fallback path for that scenario.
Agent Prompt
## Issue description
The workflow runs on `pull_request` and always does `npm install` from GitHub Packages using `GITHUB_TOKEN`. If the package isn’t readable under the token context used for fork PRs, the job fails and (if required) blocks merges.
## Issue Context
This repo already documents that public repos can have structural access limits to private org resources in Actions; similar constraints can apply to GitHub Packages depending on package visibility/access settings.
## Fix Focus Areas
- .github/workflows/governance-enforce.yml[18-26]
- .github/workflows/governance-enforce.yml[44-48]
- .github/workflows/foundation-gate.yml[3-8]
## Suggested change
- Decide the intended behavior for fork PRs and encode it explicitly, for example:
- If forks are supported: ensure `@wave-av/governance` is publicly readable (or otherwise readable by the `pull_request` token), or distribute the scanner in a way accessible to fork workflows.
- If forks are not supported: add an `if:` guard to skip or soft-fail with a clear annotation when `github.event.pull_request.head.repo.fork == true`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Qodo Fixer✅ Merged (0) · ☑ Fixed (0) Process
|
There was a problem hiding this comment.
2 issues found across 1 file
Confidence score: 3/5
- In
.github/workflows/governance-enforce.yml, installing private@wave-av/governancecan fail before policy checks run if this repo has no explicit package access grant, so enforcement may silently never execute on normal runs — grant this repository read access to the package (or move to a package visibility/auth model the repo can reliably consume). - In
.github/workflows/governance-enforce.yml, usingsecrets.GITHUB_TOKENonpull_requestis likely to fail for forked PRs because that token usually cannot read org-private packages, which can block external contribution validation — switch to a trigger/token strategy that can safely access the package for forked PRs (or gate the step with a fork-aware fallback).
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:25">
P1: This check can fail before enforcement for every run if the private `@wave-av/governance` package is owned by another repository without an explicit package access grant to this repo: `packages: read` alone is not a cross-repository package-access grant for `GITHUB_TOKEN`. Configuring the package's repository access for this repo and verifying fork-PR runs (or using an approved `read:packages` credential path) would prevent a permanently failing/unreported gate.</violation>
<violation number="2" location=".github/workflows/governance-enforce.yml:47">
P2: The job authenticates to GitHub Packages using `secrets.GITHUB_TOKEN` while triggered on `pull_request`. For PRs opened from forks, this token is scoped to the fork and typically cannot read the org's private `@wave-av/governance` package, so the install step (and thus the whole check) would fail for every external contribution — the previous proof-of-concept run was presumably a same-repo PR, not a fork PR. Consider adding an explicit fallback or `if: github.event.pull_request.head.repo.fork == false` guard, or otherwise ensure the package is readable from fork-scoped tokens, before this check is made required.</violation>
</file>
Architecture diagram
sequenceDiagram
participant GitHub as GitHub Actions
participant Runner as GitHub Runner
participant Checkout as actions/checkout
participant Setup as actions/setup-node
participant NPM as npm Registry (GitHub Packages)
participant Gov as @wave-av/governance
participant Git as Git History
Note over GitHub,Git: governance-enforce CI Workflow (A_BLOCK gate)
GitHub->>Runner: trigger: pull_request or push to main/master
Runner->>Checkout: checkout repo (fetch-depth: 0, persist-credentials: false)
Checkout-->>Runner: source with full history
Runner->>Setup: setup Node.js 22
Setup-->>Runner: node ready
Note over Runner,NPM: Isolated install into RUNNER_TEMP
Runner->>Runner: create $RUNNER_TEMP/gov dir
Runner->>Runner: write .npmrc (literal token var)
Runner->>NPM: npm install @wave-av/governance@^0.4.4 --no-save
NPM-->>Runner: package installed (isolated, no repo dependency tree impact)
Note over Runner,Git: Determine diff base SHA
alt PR event
Runner->>Runner: use github.event.pull_request.base.sha
else Push event
Runner->>Runner: use github.event.before
else Fallback (manual / first push)
Runner->>Git: git rev-parse HEAD~1
Git-->>Runner: parent SHA
end
Note over Runner,Gov: Diff-scoped enforcement
Runner->>Gov: enforce.mjs --changed <BASE>
Gov->>Git: read diff against base SHA
Git-->>Gov: changed files / lines
alt Violation found (new secrets, hardcoded paths)
Gov-->>Runner: exit non-zero, list violations
Runner-->>GitHub: ❌ enforce check fails (blocks PR merge)
else No new violations
Gov-->>Runner: exit 0
Runner-->>GitHub: ✅ enforce check passes
end
Note over Runner,GitHub: Check reported before adding to ruleset<br/>(prevents deadlock on required check)
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| permissions: | ||
| contents: read | ||
| packages: read |
There was a problem hiding this comment.
P1: This check can fail before enforcement for every run if the private @wave-av/governance package is owned by another repository without an explicit package access grant to this repo: packages: read alone is not a cross-repository package-access grant for GITHUB_TOKEN. Configuring the package's repository access for this repo and verifying fork-PR runs (or using an approved read:packages credential path) would prevent a permanently failing/unreported gate.
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 25:
<comment>This check can fail before enforcement for every run if the private `@wave-av/governance` package is owned by another repository without an explicit package access grant to this repo: `packages: read` alone is not a cross-repository package-access grant for `GITHUB_TOKEN`. Configuring the package's repository access for this repo and verifying fork-PR runs (or using an approved `read:packages` credential path) would prevent a permanently failing/unreported gate.</comment>
<file context>
@@ -0,0 +1,59 @@
+
+permissions:
+ contents: read
+ packages: read
+
+concurrency:
</file context>
| - 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 |
There was a problem hiding this comment.
P2: The job authenticates to GitHub Packages using secrets.GITHUB_TOKEN while triggered on pull_request. For PRs opened from forks, this token is scoped to the fork and typically cannot read the org's private @wave-av/governance package, so the install step (and thus the whole check) would fail for every external contribution — the previous proof-of-concept run was presumably a same-repo PR, not a fork PR. Consider adding an explicit fallback or if: github.event.pull_request.head.repo.fork == false guard, or otherwise ensure the package is readable from fork-scoped tokens, before this check is made required.
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 47:
<comment>The job authenticates to GitHub Packages using `secrets.GITHUB_TOKEN` while triggered on `pull_request`. For PRs opened from forks, this token is scoped to the fork and typically cannot read the org's private `@wave-av/governance` package, so the install step (and thus the whole check) would fail for every external contribution — the previous proof-of-concept run was presumably a same-repo PR, not a fork PR. Consider adding an explicit fallback or `if: github.event.pull_request.head.repo.fork == false` guard, or otherwise ensure the package is readable from fork-scoped tokens, before this check is made required.</comment>
<file context>
@@ -0,0 +1,59 @@
+ - 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
+ - name: A_BLOCK enforce (secrets + hardcoded paths on the diff)
</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_431fe70b-0b30-4b2c-bc18-021c1029c1f4) |
| # 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
On a force-push to main, github.event.before points at a commit the checkout no longer contains, so the code falls back to HEAD~1 — diffing only the tip commit against its parent. A force-push that introduces a secret in an earlier commit (not the tip) passes the gate without examining that commit, defeating the security check. Consider falling back to the empty-tree base (as the code already does when HEAD~1 is unresolvable) so the entire tree is scanned instead of just the last commit.
- BASE="$(git rev-parse --verify --quiet 'HEAD~1' || true)"
+ BASE=""
+ echo "::notice::force-push detected (before-sha unreachable) — scanning the whole tree against the empty tree"🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @.github/workflows/governance-enforce.yml around line 80:
On a force-push to `main`, `github.event.before` points at a commit the checkout no longer contains, so the code falls back to `HEAD~1` — diffing only the tip commit against its parent. A force-push that introduces a secret in an earlier commit (not the tip) passes the gate without examining that commit, defeating the security check. Consider falling back to the empty-tree base (as the code already does when `HEAD~1` is unresolvable) so the entire tree is scanned instead of just the last commit.
Evidence trail:
.github/workflows/governance-enforce.yml:74-93 at e8b58bc; https://docs.github.com/en/rest/using-the-rest-api/github-event-types (PushEvent `before`: SHA of most recent commit on ref before push)
| # FIX 2 — --ignore-scripts. npm runs preinstall/install/postinstall by default, so this | ||
| # step would execute dependency-authored code with the registry token in its environment. | ||
| # FIX 3 — exact pin, and 0.4.6 specifically. `^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.4.6 fails closed on a git error instead. | ||
| # A caret is also a standing authorization for whatever is published next; a bump is now | ||
| # a visible commit in this file. | ||
| npm install @wave-av/governance@0.4.6 --no-save --no-audit --no-fund --ignore-scripts |
There was a problem hiding this comment.
🔍 Workflow pins 0.4.6 while the PR description says ^0.4.4
The description states the workflow installs @wave-av/governance@^0.4.4, but the committed file installs an exact 0.4.6 with --ignore-scripts (FIX 2/FIX 3). The file is the stricter, intended state; the description is stale. Also worth confirming @wave-av/governance@0.4.6 does not rely on lifecycle scripts to produce bin/enforce.mjs, since --ignore-scripts would silently leave the path missing and the last step would fail with a bare node error rather than a policy verdict.
Was this helpful? React with 👍 or 👎 to provide feedback.
…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.
| 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 | ||
| 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 object as a diff base
The indeterminate-base fallback substitutes the empty-tree object id from git hash-object -t tree /dev/null and passes it to --changed. This only produces the intended "scan the whole tree" behavior if @wave-av/governance@0.4.6 diffs with a two-dot range (git diff BASE HEAD). If it uses a three-dot range (BASE...HEAD) or resolves the argument via rev-parse BASE^{commit}, git errors on a tree object and, per the comment at .github/workflows/governance-enforce.yml:66-67, 0.4.6 fails closed — meaning the fallback path always red-fails rather than scanning. Worth confirming against the package before this becomes a required check, since the fallback is exactly the path exercised on force-pushes and branch creation.
Was this helpful? React with 👍 or 👎 to provide feedback.
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.
Note
Low Risk
CI-only addition with read-only permissions and an isolated npm install; no application runtime or deployment paths change. Main operational risk is workflow misconfiguration (false greens or install failures), not product security regressions.
Overview
Adds
.github/workflows/governance-enforce.yml, bringing this repo under the same A_BLOCK gate (secrets in git, hardcoded paths) that org rulesets already require onwave-*repos—this public npm publisher was missing from that hand-maintained list.On pull requests and pushes to
main/master, the job checks out with full history, installs@wave-av/governance@0.4.6into$RUNNER_TEMP(isolated from the repo’s deps), and runsenforce.mjs --changed <base>on the diff so new violations fail without scanning legacy debt.The workflow copy includes fail-closed behavior from claude-workstation#1747: registry token only on the install step,
--ignore-scripts, exact package pin, push concurrency that does not cancel in-flight push scans, and base resolution that falls back to the empty tree instead ofHEAD(which would scan zero files and pass).Do not add this repo to
governance-a-block-enforceuntilenforceis green here—required checks that never report deadlock merges.Reviewed by Cursor Bugbot for commit e8b58bc. 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 04ec38d.