Skip to content

ci: vendor governance-enforce — this repo was never in the A_BLOCK ruleset - #25

Open
yakimoto wants to merge 4 commits into
mainfrom
ci/vendor-governance-enforce
Open

ci: vendor governance-enforce — this repo was never in the A_BLOCK ruleset#25
yakimoto wants to merge 4 commits into
mainfrom
ci/vendor-governance-enforce

Conversation

@yakimoto

@yakimoto yakimoto commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Adds the governance-enforce A_BLOCK gate (secrets / hardcoded-paths, diff-scoped) to this repo. Part of claude-workstation#1624 E4 T4.9a, following the wave-av/cli#20 pilot.

Why this repo had no secrets scan

The org ruleset governance-a-block-enforce (17901847) requires an enforce check across the fleet. Its scope is an explicit include list of 112 hand-maintained repository names — and every one of them matches wave-*.

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_checks ruleset 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. If enforce reports 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/governance is an internal-visibility package owned by claude-workstation, so whether a public repo's GITHUB_TOKEN can read it was the one real assumption. Rather than fan out on the inference, it was piloted on a single repo first:

wave-av/cli#20 — workflow run 31011943790, conclusion success.

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 because auto-approve.yml fails silently on every public repo — it calls a reusable workflow in the private wave-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 SHA-pinned (actions/checkout@df4cb1c, actions/setup-node@48b55a0)
  • persist-credentials: false on checkout
  • least privilege: contents: read + packages: read
  • isolated install into RUNNER_TEMP, --no-save, so nothing touches this repo's dependency tree
  • the .npmrc is written with a literal ${NODE_AUTH_TOKEN} (single-quoted printf) which npm expands at run time — no secret value is ever written to disk or a log
  • both ${{ }} inputs (base.sha, event.before) are routed through env: and read as "$VAR", never interpolated into the script body

Diff-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 no application or runtime code changes; risk is limited to workflow misconfiguration (e.g. false greens) rather than production behavior.

Overview
Adds .github/workflows/governance-enforce.yml, introducing an org A_BLOCK gate that scans PR/push diffs for secrets-in-git, Doppler policy, and hardcoded paths via an isolated install of @wave-av/governance@0.4.6.

The workflow runs on pull_request and push to main/master, reports an enforce job, and is meant to go green here before this repo is added to the governance-a-block-enforce ruleset (required check with no reporter would deadlock merges).

Hardening baked into the vendored copy: SHA-pinned Actions, packages: read, registry token only on the npm install step with --ignore-scripts, push concurrency that does not cancel in-progress push runs (so each push range stays scanned), and fail-closed diff-base resolution (empty tree fallback instead of HEAD vs HEAD).

Reviewed by Cursor Bugbot for commit 320e1b4. Configure here.


View with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is enabled.

Review in cubic

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.6 on 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 2d08570.

@yakimoto yakimoto added the rr:skip-cubic RF.P1 reviewer routing (#1039) label Aug 5, 2026
@cursor

cursor Bot commented Aug 5, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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_e755fd0d-902b-41d6-b7cc-0f3c1e1173d4)

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1733078a-e4d7-4689-b04e-581181643348

📥 Commits

Reviewing files that changed from the base of the PR and between 0e79241 and 2d08570.

📒 Files selected for processing (1)
  • .github/workflows/governance-enforce.yml

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

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Aug 5, 2026
@macroscopeapp

macroscopeapp Bot commented Aug 5, 2026

Copy link
Copy Markdown

Approvability

Verdict: Needs human review

Unable to check for correctness in 2d08570. Multiple unresolved review comments raise operational concerns about this security workflow: fork PR authentication, potential content-policy scan failures from internal repo name references, and fallback behavior. These should be addressed before merging.

You can customize Macroscope's approvability policy. Learn more.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

CI: add governance-enforce A_BLOCK diff-scoped secrets/path gate

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Add a GitHub Actions enforce status check for A_BLOCK governance scanning.
• Run diff-scoped secrets and hardcoded-path detection via @wave-av/governance.
• Ensure the check exists before adding this repo to the org required-check ruleset.
Diagram

graph TD
  PR["PR / Push"] --> GA["GitHub Actions"] --> CHK["Checkout (full history)"] --> NODE["Setup Node 22"] --> NPM["Install @wave-av/governance"] --> DIFF{"Resolve base SHA"} --> RUN["Run enforce.mjs (--changed)"] --> STATUS["Status check: enforce"]

  subgraph Legend
    direction LR
    _proc["Step"] ~~~ _dec{"Decision"}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Central reusable workflow (org-owned)
  • ➕ Single source of truth across repositories
  • ➕ Easier global rollout and updates
  • ➖ Public repos can’t always call private reusable workflows (parse-time failure risk)
  • ➖ Still requires careful sequencing with required-check rulesets
2. Publish a dedicated GitHub Action wrapper for governance enforcement
  • ➕ Cleaner workflow YAML; standard action inputs/outputs
  • ➕ Potential for caching and versioning conventions
  • ➖ Additional maintenance surface area
  • ➖ Still needs package access/token handling for GitHub Packages
3. Run full-repo scans instead of diff-scoped enforcement
  • ➕ Finds legacy issues immediately; simpler base-SHA logic
  • ➖ Likely noisy and disruptive due to existing debt
  • ➖ Higher runtime cost; risks blocking on pre-existing violations

Recommendation: Keep the current vendored, diff-scoped workflow. It avoids cross-repo reusable-workflow visibility pitfalls for public repos, minimizes disruption by only blocking new violations, and provides the required enforce check so the repo can be safely added to the org ruleset afterward.

Files changed (1) +59 / -0

Other (1) +59 / -0
governance-enforce.ymlAdd A_BLOCK governance-enforce workflow and 'enforce' status check +59/-0

Add A_BLOCK governance-enforce workflow and 'enforce' status check

• Introduces a new workflow that runs on pull requests and pushes to main/master. The job checks out full history, installs '@wave-av/governance' in an isolated temp directory using GitHub Packages, determines a suitable base SHA, and runs diff-scoped enforcement to produce the 'enforce' check.

.github/workflows/governance-enforce.yml

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 3 potential issues.

Open in Devin Review

Comment on lines +8 to +13
# VENDORED 2026-08-05 (claude-workstation#1624, E4 T4.9a). This repo was never in that ruleset's
# include list, because the list is 112 hand-maintained names and every one of them matches
# `wave-*`. A naming convention had silently become a security boundary: the repos that publish
# our npm packages — cli, sdk, adk, mcp-server, workflow-sdk — were the ones running with no
# A_BLOCK secrets scan at all. Copied verbatim from wave-moq-edge, and proven on a PUBLIC repo
# before fan-out: wave-av/cli#20, run 31011943790, conclusion success.

@devin-ai-integration devin-ai-integration Bot Aug 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Header comment names potentially private repos, which the repo's own content gate can block

The header references claude-workstation#1624 and wave-moq-edge. This repo runs public-repo-guard which blocks any occurrence of names supplied via the org variable GUARD_PRIVATE_REPOS (scripts/public-repo-guard/content-policy.sh:100-114), matched as whole words across the whole tree including .github/** (the scan uses --hidden --no-ignore-vcs). If either name is in that variable, this PR will fail the existing secrets/content gate. Worth checking the variable's contents, or annotating the lines with # guard:allow <reason>.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +44 to +48
- 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Fork PRs will attempt an authenticated GitHub Packages install

On pull_request from a fork of this public repo, NODE_AUTH_TOKEN is the base repo's read-only GITHUB_TOKEN. If the @wave-av/governance package's repo access list does not grant this repo read access, npm install fails and the job errors rather than skipping — which matters because the comment intends this check to eventually become required. Verify a fork PR run succeeds before adding the check to the ruleset.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread .github/workflows/governance-enforce.yml Outdated
@qodo-code-review

qodo-code-review Bot commented Aug 5, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Floating enforcer version ✓ Resolved 🐞 Bug ⛨ Security
Description
The workflow installs @wave-av/governance with a caret range (^0.4.4), so the required enforce
gate can change behavior on later runs without any change/approval in this repo. That weakens
reproducibility and can cause surprise pass/fail drift (or worse, supply-chain exposure) in a
security gate.
Code

.github/workflows/governance-enforce.yml[R47-48]

+          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
Evidence
The workflow installs the enforcer at runtime with a semver range, which permits behavior drift
without repo changes. This repo’s other security gating workflow emphasizes pinned/verified tooling
to keep gates deterministic and reviewable.

.github/workflows/governance-enforce.yml[44-49]
.github/workflows/public-repo-guard.yml[9-15]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The workflow installs `@wave-av/governance@^0.4.4`, which allows npm to resolve newer compatible versions on future runs. This means the repo’s required security gate can change without a PR in this repo.

### Issue Context
This workflow runs as a required status check (`enforce`), so reproducibility and reviewability matter.

### Fix Focus Areas
- .github/workflows/governance-enforce.yml[44-49]

### Suggested fix
- Replace `@wave-av/governance@^0.4.4` with an exact version (e.g. `@wave-av/governance@0.4.4`).
- If you intentionally want controlled roll-forward, do it via explicit PR bumps (and optionally add a comment documenting the update process).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Token scoped too broadly ✓ Resolved 🐞 Bug ⛨ Security
Description
NODE_AUTH_TOKEN is set at the job level, so it is available to every step (including the
enforcement run) instead of only the npm install step that needs it. This unnecessarily increases
the exposure window/blast radius for a credential-bearing environment variable.
Code

.github/workflows/governance-enforce.yml[R34-36]

+    env:
+      NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+    steps:
Evidence
The workflow sets NODE_AUTH_TOKEN for the entire job even though only the install step uses it to
authenticate to GitHub Packages via .npmrc.

.github/workflows/governance-enforce.yml[34-48]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`NODE_AUTH_TOKEN` is defined at the job level, making it available to all steps. Only the step that writes `.npmrc` and runs `npm install` needs this variable.

### Issue Context
The workflow already uses read-only permissions, so this is defense-in-depth: reduce accidental exposure in unrelated steps and subprocesses.

### Fix Focus Areas
- .github/workflows/governance-enforce.yml[33-49]

### Suggested fix
- Remove the job-level `env: NODE_AUTH_TOKEN: ...`.
- Add `env: NODE_AUTH_TOKEN: ...` only to the "fetch governance enforcer" step.
- Optionally delete `$RUNNER_TEMP/gov/.npmrc` after install to reduce persistence on the runner workspace.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. Missing explicit timeout ✓ Resolved 🐞 Bug ☼ Reliability
Description
The enforce job does not set timeout-minutes, leaving it to the platform default which is
typically much longer than needed for this small gate. A stuck npm install or hung enforcement run
can therefore delay required-check completion and waste runner time longer than intended.
Code

.github/workflows/governance-enforce.yml[R31-35]

+jobs:
+  enforce:
+    runs-on: ubuntu-latest
+    env:
+      NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Evidence
governance-enforce defines runs-on but no job timeout, while similar CI gates in this repo do
specify explicit timeouts.

.github/workflows/governance-enforce.yml[31-36]
.github/workflows/python-lint.yml[18-22]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The `enforce` job omits `timeout-minutes`, so the effective timeout is an implicit platform default rather than a repo-controlled bound.

### Issue Context
Other CI gates in this repo set explicit timeouts.

### Fix Focus Areas
- .github/workflows/governance-enforce.yml[31-36]

### Suggested fix
- Add `timeout-minutes: 10` (or another value consistent with expected runtime) to `jobs.enforce` to bound hangs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
Review mode: ⚖️ Balanced: This adds a security-sensitive CI enforcement workflow that installs and executes a package, handles credentials and diff-base selection, and affects repository governance; one focused hunk is not enough to justify extended, nor is it low-risk enough for lite.

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

qodo-code-review[bot]

This comment was marked as resolved.

@qodo-code-review

Copy link
Copy Markdown

Qodo Fixer

No findings are within the configured fix scope. To change which findings are fixed, adjust the setting on your Qodo configuration page.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

2 issues found across 1 file

Confidence score: 3/5

  • In .github/workflows/governance-enforce.yml, forked pull_request runs may not be able to install @wave-av/governance because NODE_AUTH_TOKEN maps to the base repo’s read-only GITHUB_TOKEN, which can make governance checks fail for external contributors and block PR validation — add a fork-safe auth path or skip/private-package fallback for forked PRs.
  • In .github/workflows/governance-enforce.yml, the vendored header references internal repo names that can trip public-repo-guard if they match GUARD_PRIVATE_REPOS, causing policy-check failures unrelated to this repo’s code changes — remove/sanitize those identifiers or adjust guard matching for vendored headers.
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:12">
P2: The vendored header comment names internal repos (`claude-workstation#1624`, `wave-moq-edge`). If either name is present in the org's `GUARD_PRIVATE_REPOS` variable, this repo's own `public-repo-guard` content-policy scan (which matches whole words across the tree, including `.github/**`) will flag these lines and fail the PR. Worth checking the variable's contents or annotating with `# guard:allow <reason>` if intentional.</violation>

<violation number="2" location=".github/workflows/governance-enforce.yml:48">
P2: For pull_request events from a fork, `NODE_AUTH_TOKEN` resolves to the base repo's read-only `GITHUB_TOKEN`. If the `@wave-av/governance` package's repo access list doesn't include this repo, `npm install` will fail and the enforce job will error rather than gracefully skip — a real problem once this check becomes a required status check on fork PRs. Verify a fork PR run succeeds before adding this to the required ruleset.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant PR as Pull Request Event
    participant GH as GitHub Actions Runner
    participant Repo as Repo Checkout
    participant NPM as GitHub npm Registry
    participant Gov as @wave-av/governance
    participant Workflow as Workflow Run

    Note over PR,Workflow: NEW: governance-enforce workflow (A_BLOCK gate)

    PR->>GH: trigger on pull_request or push to main/master
    GH->>GH: evaluate concurrency group
    GH->>GH: allocate job with contents:read + packages:read
    
    GH->>Repo: actions/checkout@df4cb1c (persist-credentials:false)
    Repo-->>GH: full git history (fetch-depth:0)
    
    GH->>GH: actions/setup-node@48b55a0 (node-22)
    
    Note over GH,NPM: Isolated install into RUNNER_TEMP
    GH->>GH: create ~/gov dir + write .npmrc
    Note over GH: printf writes literal ${NODE_AUTH_TOKEN}<br/>npm expands at runtime (no secret written to disk)
    
    GH->>NPM: npm install @wave-av/governance@^0.4.4 --no-save
    NPM-->>GH: package + dependencies (internal visibility)
    
    Note over GH,Gov: Diff-scoped enforcement
    GH->>GH: resolve BASE SHA (PR base.sha or event.before)
    alt PR event with base.sha
        GH->>GH: BASE = github.event.pull_request.base.sha
    else push event
        GH->>GH: BASE = github.event.before
    else fallback (first commit)
        GH->>GH: BASE = HEAD~1 or HEAD
    end
    
    GH->>Gov: node enforce.mjs --changed $BASE
    Gov->>Repo: read git diff against BASE
    Gov->>Gov: scan diff for secrets (Doppler tokens, hardcoded credentials)
    Gov->>Gov: scan diff for hardcoded paths
    
    alt Diff has NEW violations
        Gov-->>GH: exit non-zero (fail)
        GH-->>PR: status: ❌ enforce (blocking)
    else Clean diff or only legacy debt
        Gov-->>GH: exit zero (pass)
        GH-->>PR: status: ✅ enforce (green)
    end
    
    Note over PR,Workflow: Governance ruleset (17901847) is NOT yet updated<br/>This check runs but is not required until ruleset is extended
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread .github/workflows/governance-enforce.yml Outdated
# include list, because the list is 112 hand-maintained names and every one of them matches
# `wave-*`. A naming convention had silently become a security boundary: the repos that publish
# our npm packages — cli, sdk, adk, mcp-server, workflow-sdk — were the ones running with no
# A_BLOCK secrets scan at all. Copied verbatim from wave-moq-edge, and proven on a PUBLIC repo

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The vendored header comment names internal repos (claude-workstation#1624, wave-moq-edge). If either name is present in the org's GUARD_PRIVATE_REPOS variable, this repo's own public-repo-guard content-policy scan (which matches whole words across the tree, including .github/**) will flag these lines and fail the PR. Worth checking the variable's contents or annotating with # guard:allow <reason> if intentional.

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 12:

<comment>The vendored header comment names internal repos (`claude-workstation#1624`, `wave-moq-edge`). If either name is present in the org's `GUARD_PRIVATE_REPOS` variable, this repo's own `public-repo-guard` content-policy scan (which matches whole words across the tree, including `.github/**`) will flag these lines and fail the PR. Worth checking the variable's contents or annotating with `# guard:allow <reason>` if intentional.</comment>

<file context>
@@ -0,0 +1,59 @@
+# include list, because the list is 112 hand-maintained names and every one of them matches
+# `wave-*`. A naming convention had silently become a security boundary: the repos that publish
+# our npm packages — cli, sdk, adk, mcp-server, workflow-sdk — were the ones running with no
+# A_BLOCK secrets scan at all. Copied verbatim from wave-moq-edge, and proven on a PUBLIC repo
+# before fan-out: wave-av/cli#20, run 31011943790, conclusion success.
+#
</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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: For pull_request events from a fork, NODE_AUTH_TOKEN resolves to the base repo's read-only GITHUB_TOKEN. If the @wave-av/governance package's repo access list doesn't include this repo, npm install will fail and the enforce job will error rather than gracefully skip — a real problem once this check becomes a required status check on fork PRs. Verify a fork PR run succeeds before adding this to the required 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>For pull_request events from a fork, `NODE_AUTH_TOKEN` resolves to the base repo's read-only `GITHUB_TOKEN`. If the `@wave-av/governance` package's repo access list doesn't include this repo, `npm install` will fail and the enforce job will error rather than gracefully skip — a real problem once this check becomes a required status check on fork PRs. Verify a fork PR run succeeds before adding this to the required 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>

Comment thread .github/workflows/governance-enforce.yml Outdated
Comment thread .github/workflows/governance-enforce.yml
…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.
@cursor

cursor Bot commented Aug 6, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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_8721ba0f-99dd-4b7b-8c30-170edc2c2dc8)

# 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)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Critical workflows/governance-enforce.yml:80

When a force-push makes github.event.before unreachable, the fallback resolves BASE to HEAD~1 whenever that commit exists. The scan then diffs only the newest commit against its immediate parent, so any secret or hardcoded-path violation in the other commits introduced by the rewritten push is silently unexamined while the gate can still pass. This defeats the fix's stated goal of scanning every commit the push introduced. HEAD~1 is a per-commit base, not a per-push base. Consider falling straight through to the empty-tree scan (or otherwise resolving the oldest commit in the push range) so the full set of introduced commits is examined.

-            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 resolves `BASE` to `HEAD~1` whenever that commit exists. The scan then diffs only the newest commit against its immediate parent, so any secret or hardcoded-path violation in the other commits introduced by the rewritten push is silently unexamined while the gate can still pass. This defeats the fix's stated goal of scanning every commit the push introduced. `HEAD~1` is a per-commit base, not a per-push base. Consider falling straight through to the empty-tree scan (or otherwise resolving the oldest commit in the push range) so the full set of introduced commits is examined.

Evidence trail:
https://github.com/wave-av/sdk-python/tree/320e1b4637df89fe3e155629c9c7f98871349d9a/.github/workflows/governance-enforce.yml#L30-L35
https://github.com/wave-av/sdk-python/tree/320e1b4637df89fe3e155629c9c7f98871349d9a/.github/workflows/governance-enforce.yml#L74-L93
git diff MERGE_BASE REVIEWED_COMMIT -- .github/workflows/governance-enforce.yml

…d of scanning only HEAD~1

Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

Open in Devin Review

Comment on lines +91 to +94
if [ -z "$BASE" ]; then
BASE="$(git hash-object -t tree /dev/null)"
echo "::notice::no usable diff base (branch creation, root commit, or force-push with unreachable before-sha) — scanning the whole tree against the empty tree"
fi

@devin-ai-integration devin-ai-integration Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Empty-tree fallback turns any missing base into a whole-repo scan that can block main

When no usable base is found (branch creation, root commit, force-push with unreachable before-sha) the workflow diffs against the empty tree, so every tracked file counts as added. This is intentional fail-closed behavior, but it contradicts the diff-scoped promise in the header comment (.github/workflows/governance-enforce.yml:4-5): any pre-existing legacy violation in the repo will fail that run, including on pushes to main after a force-push. Worth confirming the repo currently scans clean whole-tree before this check is added to the org ruleset.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The empty-tree fallback is the intentional FIX 5 fail-closed design for exceptional paths (root commit, force-push with unreachable before-sha), where failing loudly on legacy debt beats passing having scanned nothing. The rollout plan already gates the ruleset entry on observed green runs, which addresses the "confirm it scans clean" advisory.

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Aug 6, 2026
…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.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

Open in Devin Review

Comment on lines +101 to +105
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Empty-tree fallback assumes the enforcer's --changed accepts a bare tree object

The fallback sets BASE to the empty-tree object hash (git hash-object -t tree /dev/null) and passes it as --changed. This only works if @wave-av/governance builds its diff with a two-dot form (e.g. git diff --name-only <base> HEAD). If the enforcer internally uses the symmetric three-dot form (git diff <base>...HEAD) or ever resolves the argument with <base>^{commit}, the empty-tree hash is not a commit and git will abort — turning the "loud, never partial" path into a hard job failure on every branch-creation push or force-push with an unreachable before-sha. Worth verifying against 0.4.6's bin/enforce.mjs once, since this path is exactly the one that cannot be exercised in a normal PR.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rr:skip-cubic RF.P1 reviewer routing (#1039)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant