ci: vendor governance-enforce — this repo was never in the A_BLOCK ruleset - #7
ci: vendor governance-enforce — this repo was never in the A_BLOCK ruleset#7yakimoto 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_09d5dbaf-9aaa-4ff0-b03a-b423c0cc7b53) |
📝 WalkthroughWalkthroughChangesGovernance enforcement
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant Git
participant GovernanceEnforcer
GitHubActions->>Git: Select pull-request base or push predecessor
GitHubActions->>Git: Fall back to HEAD~1 or HEAD when needed
GitHubActions->>GovernanceEnforcer: Run A_BLOCK checks against the selected diff
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 7a5d2da. This PR adds a security governance workflow with complex diff-base fallback logic. Multiple unresolved review comments raise substantive concerns about authentication failures and edge cases where the security scan may not examine all content as intended. You can customize Macroscope's approvability policy. Learn more. |
![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.
🔍 GITHUB_TOKEN authenticating to another repo's GitHub Packages scope
The isolated install authenticates to npm.pkg.github.com with the workflow's GITHUB_TOKEN. That token's package read scope is limited to packages linked to this repository (or packages whose visibility/access grants this repo). If @wave-av/governance is a private package not granted access to wave-av/crest-console, the npm install step will 401 and the gate will fail for every PR. The header notes the pattern was proven on wave-av/cli, but per-package repository access grants are configured per package, so worth confirming for this repo before adding the check to the required-status ruleset.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is a deployment verification concern, not a workflow defect: the PR is explicitly its own liveness drill, so the install step running on this PR's CI will prove whether @wave-av/governance grants this repo access before the check is added to the ruleset.
PR Summary by QodoVendor governance-enforce A_BLOCK diff-scoped gate for secrets/hardcoded paths
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
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. |
| - 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 fail this gate on package auth
For pull_request events from forks, secrets.GITHUB_TOKEN is issued with read-only scope for the base repo, but organizations frequently see failures reading internal-visibility GitHub Packages from fork-triggered runs. Since this repo is public, fork PRs are plausible, and a failing install step here would hard-fail the job (and, once the ruleset is extended, block those PRs entirely). Worth verifying the fork case explicitly before adding enforce to governance-a-block-enforce, since the wave-av/cli#20 pilot was a same-repo branch PR.
Was this helpful? React with 👍 or 👎 to provide feedback.
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/governance-enforce.yml:
- Around line 27-29: Update the concurrency configuration near the
workflow-level group so push-triggered scans are never canceled by later pushes,
while preserving cancellation for appropriate non-push runs. Use distinct
concurrency groups or disable cancellation for push events, and ensure pending
push runs are not allowed to replace one another in a way that skips changed
revisions.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7c76e8b3-56fe-46c9-85e7-2d685ae87e59
📒 Files selected for processing (1)
.github/workflows/governance-enforce.yml
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow =="
cat -n .github/workflows/governance-enforce.yml
echo
echo "== references to pushed_before_sha / changed / governance =="
rg -n "PUSH_BEFORE_SHA|push\.before|github\.push|changed|`@wave-av/governance`|governance-enforce" .github/workflows || true
echo
echo "== other checks mentioning diff-only coverage =="
rg -n "DIFF-SCOPED|full-tree|no path rule|SECRET|governance" .github/workflows/_checks.yml || trueRepository: wave-av/crest-console
Length of output: 7280
🌐 Web query:
GitHub Actions concurrency cancel-in-progress previous run in same group cancels
💡 Result:
In GitHub Actions, the behavior of the concurrency setting depends on the configuration of cancel-in-progress and whether you are using the default queuing behavior or the newer, expanded queuing feature [1][2]. At most, one job or workflow run can be in progress within a concurrency group at any given time [1][3]. When a new run is queued for that group, the following happens based on your configuration: 1. Default Behavior (cancel-in-progress: false or unset): If a run is already in progress, the new run enters a pending state [1][3]. If there was a previously pending run in that group, it is automatically canceled and replaced by the newest run [1][3]. This ensures that while only one run is active, there is at most one pending run waiting to take its place [3]. 2. Cancel-in-progress: true: If a run is already in progress, setting cancel-in-progress: true will cause the currently running job or workflow to be canceled immediately [1][3]. The new run then replaces it [1]. 3. Expanded Queuing (queue: max): As of May 2026, you can enable larger queues by setting queue: max [2]. This allows up to 100 pending runs to wait in line sequentially instead of canceling older pending runs [2]. This configuration is incompatible with cancel-in-progress: true and can only be used when cancel-in-progress is set to false (or left unset) [1][2]. Key Considerations: - Concurrency group names are scoped to the repository [1]. If multiple workflows use the same concurrency group name, they will compete for the same slot, potentially canceling each other's pending or in-progress runs [1][3]. To isolate concurrency to a specific workflow, include github.workflow in your group name (e.g., group: ${{ github.workflow }}-${{ github.ref }}) [1][3]. - You can use expressions with cancel-in-progress to dynamically decide whether to cancel runs, such as only canceling on specific branches while allowing others to run to completion [1][4].
Citations:
- 1: https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency
- 2: https://github.blog/changelog/2026-05-07-github-actions-concurrency-groups-now-allow-larger-queues/
- 3: https://docs.github.com/enterprise-cloud@latest/actions/using-jobs/using-concurrency
- 4: Clarify concurrency cancel-in-progress behaviour github/docs#30647
🌐 Web query:
GitHub Actions workflow push event before SHA documentation
💡 Result:
In a GitHub Actions workflow triggered by a push event, the commit SHA immediately preceding the push is available via the context variable ${{ github.event.before }} [1][2]. When a push event occurs, the GitHub webhook payload includes a before property, which represents the SHA of the most recent commit on the branch prior to the push [1]. You can access this value directly in your workflow file using the github context [3][2]. Key details: - Context Access: Use ${{ github.event.before }} to retrieve the previous SHA [2]. - Initial Pushes: Note that if the push involves a newly created branch or ref, the before SHA may be a default value, such as 0000000000000000000000000000000000000000 [2]. - Comparison: This value is distinct from ${{ github.sha }}, which represents the tip commit of the pushed ref (the "after" state) [3][4]. This functionality is commonly used in workflows that need to calculate the range of commits included in a push, such as for generating changelogs, identifying modified files, or determining the correct fetch-depth for the actions/checkout step [5][2].
Citations:
- 1: https://docs.github.com/en/webhooks/webhook-events-and-payloads?actionType=fixed
- 2: https://stackoverflow.com/questions/61860732/how-can-i-get-the-previous-commit-before-a-push-or-merge-in-github-action-workfl
- 3: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts
- 4: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows
- 5: https://stackoverflow.com/questions/77023798/github-actions-checkout-fetch-history-up-to-before-the-push-event
Security Misconfiguration (CWE-693)
Reachability: External
Keep push scans from being canceled.
If two pushes to main or master run in the same github.workflow-${ github.ref } concurrency group, a later push cancels the earlier run. The completed run then uses only its own PUSH_BEFORE_SHA, so changes from the canceled push can be skipped by --changed "$BASE".
Use a unique concurrency group for push runs, or do not let pushes cancel PR runs; canceling pending runs within the same group still allows one pending run to replace another.
Proposed fix
concurrency:
- group: ${{ github.workflow }}-${{ github.ref }}
- cancel-in-progress: true
+ group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.run_id }}
+ cancel-in-progress: ${{ github.event_name == 'pull_request' }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| concurrency: | |
| group: ${{ github.workflow }}-${{ github.ref }} | |
| cancel-in-progress: true | |
| concurrency: | |
| group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} | |
| cancel-in-progress: ${{ github.event_name == 'pull_request' }} |
🤖 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/governance-enforce.yml around lines 27 - 29, Update the
concurrency configuration near the workflow-level group so push-triggered scans
are never canceled by later pushes, while preserving cancellation for
appropriate non-push runs. Use distinct concurrency groups or disable
cancellation for push events, and ensure pending push runs are not allowed to
replace one another in a way that skips changed revisions.
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
![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 and verified against the latest diff
Confidence score: 2/5
- In
.github/workflows/governance-enforce.yml, the governance enforcer is effectively floating to the latest 0.4.x on fresh runners, so a future or compromised release could silently change a required security gate and weaken enforcement—pin an exact version (or commit digest) and vendor/lock it for reproducible runs. - In
.github/workflows/governance-enforce.yml, installing fromnpm.pkg.github.comwith the defaultGITHUB_TOKENcan fail when package read access is not explicitly granted, which can make this required check flaky or block PRs unexpectedly—grant package read permission for@wave-av/governance(or use a dedicated token with the needed scope).
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:47">
P2: The install step authenticates to npm.pkg.github.com using the workflow's GITHUB_TOKEN, whose GitHub Packages read scope is limited to packages explicitly granted access to this repository. If @wave-av/governance hasn't been granted per-repo access for wave-av/crest-console, the npm install will fail with a 401 for every run, permanently failing this check. This should be verified for this specific repo before wiring it into the required-status ruleset, since the referenced pilot (wave-av/cli#20) doesn't guarantee the same package access grant exists here.</violation>
<violation number="2" location=".github/workflows/governance-enforce.yml:48">
P1: The enforcer is not actually vendored: every fresh runner resolves the newest 0.4.x release, allowing a later or compromised package release to silently alter or weaken this required security check. Pin an exact package version and verify the dependency lockfile/integrity.</violation>
</file>
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.
P1: The enforcer is not actually vendored: every fresh runner resolves the newest 0.4.x release, allowing a later or compromised package release to silently alter or weaken this required security check. Pin an exact package version and verify the dependency lockfile/integrity.
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 enforcer is not actually vendored: every fresh runner resolves the newest 0.4.x release, allowing a later or compromised package release to silently alter or weaken this required security check. Pin an exact package version and verify the dependency lockfile/integrity.</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>
| NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| 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 install step authenticates to npm.pkg.github.com using the workflow's GITHUB_TOKEN, whose GitHub Packages read scope is limited to packages explicitly granted access to this repository. If @wave-av/governance hasn't been granted per-repo access for wave-av/crest-console, the npm install will fail with a 401 for every run, permanently failing this check. This should be verified for this specific repo before wiring it into the required-status ruleset, since the referenced pilot (wave-av/cli#20) doesn't guarantee the same package access grant exists here.
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 install step authenticates to npm.pkg.github.com using the workflow's GITHUB_TOKEN, whose GitHub Packages read scope is limited to packages explicitly granted access to this repository. If @wave-av/governance hasn't been granted per-repo access for wave-av/crest-console, the npm install will fail with a 401 for every run, permanently failing this check. This should be verified for this specific repo before wiring it into the required-status ruleset, since the referenced pilot (wave-av/cli#20) doesn't guarantee the same package access grant exists here.</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>
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_02b21b8d-8f26-47b2-a6a7-f3265d9877c6) |
| # 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 the supplied base SHA is unreachable (e.g., after a force-push), the fallback sets BASE to HEAD~1, which diffs only the tip commit. For a multi-commit push, earlier commits in the new history are silently skipped, so newly introduced secrets or hardcoded paths pass the gate despite the comment claiming the fallback fails closed. The fix at line 88 already handles an empty BASE by falling back to the empty tree, but this HEAD~1 branch at line 80 never reaches that logic — it sets BASE to a non-empty value and exits the fallback early. Consider resolving to the empty tree here as well, or determining a complete diff range, so no commits are skipped.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @.github/workflows/governance-enforce.yml around line 80:
When the supplied base SHA is unreachable (e.g., after a force-push), the fallback sets `BASE` to `HEAD~1`, which diffs only the tip commit. For a multi-commit push, earlier commits in the new history are silently skipped, so newly introduced secrets or hardcoded paths pass the gate despite the comment claiming the fallback fails closed. The fix at line 88 already handles an *empty* `BASE` by falling back to the empty tree, but this `HEAD~1` branch at line 80 never reaches that logic — it sets `BASE` to a non-empty value and exits the fallback early. Consider resolving to the empty tree here as well, or determining a complete diff range, so no commits are skipped.
Evidence trail:
.github/workflows/governance-enforce.yml:74-93 (f1ac62a); .github/workflows/_checks.yml:75-80,101-108 (f1ac62a); git diff MERGE_BASE REVIEWED_COMMIT -- .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" |
There was a problem hiding this comment.
🔍 Empty-tree base depends on how the enforcer diffs
git hash-object -t tree /dev/null yields the well-known empty-tree id, which git resolves in-memory even though the object is not written. That is fine for a two-dot git diff <empty-tree> HEAD, but a three-dot diff (A...HEAD) against a tree id fails because there is no merge base. Since the fail-closed path deliberately routes the whole-tree scan through the diff code path, this fallback only works if enforce.mjs uses a two-dot diff. Verifying this would be worthwhile, since the failure mode is exactly the "reports nothing examined" class the header comment is trying to eliminate.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Verified against the published 0.4.6 source: changedArgs uses the two-argument git diff <base> HEAD form, explicitly documented as chosen because A...HEAD rejects tree objects like the empty tree. Even on a git error the lister fails closed (exit 1), so the silent-pass failure mode cannot occur.
…not HEAD~1 Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
| permissions: | ||
| contents: read | ||
| packages: read | ||
|
|
||
| # FIX 4 — a cancelled push run's commits were scanned by NOBODY. Every push run diffs only its | ||
| # own before..HEAD range, so cancelling run N when run N+1 starts leaves N's commits permanently | ||
| # unexamined. PR runs are safe to supersede: each one re-diffs the whole branch against its base. | ||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} | ||
| cancel-in-progress: ${{ github.event_name == 'pull_request' }} | ||
|
|
||
| jobs: | ||
| enforce: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| steps: | ||
| - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 | ||
| with: | ||
| fetch-depth: 0 | ||
| persist-credentials: false | ||
| - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 | ||
| with: | ||
| node-version: "22" | ||
| - name: fetch governance enforcer (isolated install) | ||
| # FIX 1 — token scoped to THIS STEP. At job level it was also in scope for the step that | ||
| # executes the downloaded package, and for anything else the job ever grows. | ||
| env: | ||
| NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| set -euo pipefail | ||
| mkdir -p "$RUNNER_TEMP/gov" && cd "$RUNNER_TEMP/gov" | ||
| trap 'rm -f "$RUNNER_TEMP/gov/.npmrc"' EXIT | ||
| printf '@wave-av:registry=https://npm.pkg.github.com\n//npm.pkg.github.com/:_authToken=${NODE_AUTH_TOKEN}\n' > .npmrc | ||
| # 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.
🔍 Private-package install from a public repo / fork PRs may not authenticate
This repository is public (package.json sets private: false, and .github/workflows/public-repo-guard.yml exists as a publication gate). The install step authenticates to npm.pkg.github.com with secrets.GITHUB_TOKEN and only packages: read. A repo-scoped GITHUB_TOKEN can read a GitHub Packages artifact published by a different repo only if that package has explicitly granted read access to this repo (or is org-internal with the right visibility). For pull_request runs originating from forks the token is further restricted. Worth verifying that @wave-av/governance@0.4.6 is readable here before the org ruleset makes this a required check — otherwise the job fails at install on every PR, which is exactly the permanent-deadlock scenario the header comment warns about.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Pre-ruleset verification concern already covered by the rollout design: this PR is its own liveness drill, and the header plus PR description gate ruleset inclusion on first observing this check green here, so any package-access 401/403 surfaces in this PR's CI before the check can become required.
| 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 assumes the enforcer performs a two-dot diff
git hash-object -t tree /dev/null yields the well-known empty-tree oid, which git special-cases so it resolves even though it was never written to the object database — git diff <empty-tree> HEAD works. However, if enforce.mjs --changed <base> internally uses a three-dot/merge-base diff (git diff base...HEAD), the empty tree has no merge base with HEAD and git errors out. Since 0.4.6 is described as failing closed on git errors, the fallback would turn every unresolvable-base run into a hard failure rather than a whole-tree scan. Confirm which diff form the enforcer uses.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Verified against the published 0.4.6 source: changedArgs uses the two-argument git diff <base> HEAD form, documented as chosen specifically because A...HEAD requires a merge base and rejects tree objects like the empty tree. The fallback performs the intended whole-tree scan.
…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.
| - name: fetch governance enforcer (isolated install) | ||
| # FIX 1 — token scoped to THIS STEP. At job level it was also in scope for the step that | ||
| # executes the downloaded package, and for anything else the job ever grows. | ||
| env: | ||
| NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| set -euo pipefail | ||
| mkdir -p "$RUNNER_TEMP/gov" && cd "$RUNNER_TEMP/gov" | ||
| trap 'rm -f "$RUNNER_TEMP/gov/.npmrc"' EXIT | ||
| printf '@wave-av:registry=https://npm.pkg.github.com\n//npm.pkg.github.com/:_authToken=${NODE_AUTH_TOKEN}\n' > .npmrc | ||
| # 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.
🟡 Outside contributors' pull requests will always fail the new check on this public repository
The new gate downloads a private company package using the automatic pull-request token (npm install @wave-av/governance@0.4.6 at .github/workflows/governance-enforce.yml:70), which contributors from outside the organization do not have access to, so their pull requests fail the check for reasons they cannot fix.
Impact: Every pull request opened from a fork of this public, Apache-2.0 repository is blocked by a red required-looking check.
Why the fork token cannot read the org-scoped registry package
The .npmrc written at .github/workflows/governance-enforce.yml:62 points @wave-av at npm.pkg.github.com and authenticates with secrets.GITHUB_TOKEN (.github/workflows/governance-enforce.yml:57). For a pull request originating from a fork, that token is issued for the fork repository and is read-only; GitHub Packages access for an org-private package is granted per-repository, and forks are not on that list, so npm install gets 401/403 and the step exits non-zero under set -euo pipefail.
This repository is explicitly the public open-core client (see README.md and .github/workflows/public-repo-guard.yml, a "pre-publication content gate for WAVE public repos"), so fork pull requests are the expected contribution path. The sibling gates (_checks.yml, public-repo-guard.yml) are deliberately self-contained/vendored and need no private fetch, which is why they work for forks.
Options: gate the job with if: github.event.pull_request.head.repo.fork == false (accepting no scan for fork PRs, plus a push-side scan), or vendor the enforcement script into this repo the way scripts/public-repo-guard/content-policy.sh is vendored.
Prompt for agents
The governance-enforce workflow installs the org-private npm package @wave-av/governance from GitHub Packages using secrets.GITHUB_TOKEN. On this repository (public open-core, per README.md and public-repo-guard.yml) pull requests come from forks, where GITHUB_TOKEN is scoped to the fork and cannot read the wave-av org's private package, so the install step fails and the whole check goes red for external contributors with no way to fix it. Decide on an approach: either skip/soft-report the job for fork PRs (and rely on the push-to-main run for coverage), or vendor the enforcement logic into the repo the way scripts/public-repo-guard/content-policy.sh is vendored so no authenticated fetch is needed. Note the workflow header explicitly says not to make this a required check until it is observed green here.
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
Adds CI-only governance scanning with read-only permissions and hardened install/execution; no application runtime or data-path changes.
Overview
Adds
.github/workflows/governance-enforce.yml, bringing this repo onto the org A_BLOCK path (governance-a-block-enforceexpects anenforcejob) after it was missing from the hand-maintainedwave-*-only ruleset list.On **PRs and pushes to
main/master, the workflow checks out full history, installs@wave-av/governance@0.4.6into$RUNNER_TEMP(isolated--ignore-scriptsinstall with registry token scoped to that step), and runsenforce.mjs --changed <base>so only new secrets/hardcoded-path violations on the diff fail—not legacy debt.Hardening baked into the vendored copy: push runs use
concurrencykeyed byrun_idso superseding a push does not leave commits unscanned; diff base falls back through unreachablebefore/HEAD~1, then the empty tree (notHEAD) when no base exists; actions are SHA-pinned withpersist-credentials: falseandcontents/packages: read.Do not add this repo to the ruleset until
enforceis green here—required checks with no reporter deadlock merges.Reviewed by Cursor Bugbot for commit f1ac62a. 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 7a5d2da.
Summary by CodeRabbit