Skip to content

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

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#26
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
Adds CI-only workflow with read-only permissions and isolated package install; no runtime or application code changes.

Overview
Adds .github/workflows/governance-enforce.yml, a new CI job that runs on pull requests and pushes to main/master. It installs @wave-av/governance@0.4.6 in an isolated temp directory and runs enforce.mjs --changed on the PR/push diff for A_BLOCK rules (secrets in git, hardcoded paths). The workflow is vendored ahead of adding this repo to the org governance-a-block-enforce ruleset so the required enforce check can go green before merge gates depend on it.

Hardening is baked into the workflow: registry token only on the npm install step, --ignore-scripts, exact package pin (avoids a version that could report pass with zero files scanned), push concurrency that does not cancel in-progress runs (so push diffs are not skipped), and a fail-closed base resolution—when no valid diff base exists, it diffs against the empty tree so the whole tree is scanned instead of an empty self-diff.

Reviewed by Cursor Bugbot for commit a683bae. 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 5886e81.

@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_fff9e72a-acfe-4cae-a084-73e303ced86a)

@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: 8939d0e1-576b-4cd4-a679-c84ed66b09cf

📥 Commits

Reviewing files that changed from the base of the PR and between 7767662 and 5886e81.

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

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

@macroscopeapp

macroscopeapp Bot commented Aug 5, 2026

Copy link
Copy Markdown

Approvability

Verdict: Needs human review

Unable to check for correctness in 5886e81. Adds a new required security enforcement gate (A_BLOCK secrets scan). New security infrastructure and CI enforcement mechanisms warrant human review, even when well-documented. Unresolved concerns about fork PR access should also be addressed.

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

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Aug 5, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add diff-scoped governance-enforce A_BLOCK gate to CI

⚙️ Configuration changes ✨ Enhancement 🕐 10-20 Minutes

Grey Divider

AI Description

• Add GitHub Actions workflow to run A_BLOCK governance enforcement on PR diffs.
• Fetch @wave-av/governance from GitHub Packages with least-privilege permissions.
• Ensure required enforce status check exists before ruleset inclusion to avoid deadlocks.
Diagram

graph TD
  A(("PR / Push")) --> B["Workflow: governance-enforce"] --> C["Checkout (full history)"] --> D["Setup Node 22"] --> E{{"GitHub Packages"}} --> F["Install @wave-av/governance"] --> G["Run diff-scoped enforce"]
  subgraph Legend
    direction LR
    _trig(("Trigger")) ~~~ _step["Job/Step"] ~~~ _ext{{"External"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Central reusable workflow (org-level)
  • ➕ Single source of truth for pins/permissions across repos
  • ➕ Easier fleet-wide updates and auditing
  • ➖ Cross-repo reusable workflow can be unreadable/unusable from public repos if hosted privately
  • ➖ Still requires careful rollout ordering to avoid required-check deadlocks
2. Publish a dedicated pinned GitHub Action for enforcement
  • ➕ Cleaner workflow YAML (uses an action instead of inline npm install)
  • ➕ Action versioning can be controlled via tags/SHAs
  • ➖ New distribution surface to maintain (action repo + releases)
  • ➖ Still needs package/token access if the logic remains in a private/internal package
3. Rely on GitHub Advanced Security secret scanning (where available)
  • ➕ Native integration and reporting UX
  • ➕ No custom install/token flow
  • ➖ May not cover organization-specific rules like hardcoded-paths or Doppler-specific constraints
  • ➖ Licensing/enablement may vary across repos/org settings

Recommendation: The vendored workflow is the right rollout strategy here because it avoids cross-repo reusable workflow access pitfalls for public repos and prevents required-check deadlocks by ensuring the enforce check exists first. Consider converging later on a centrally maintained action/workflow once public/private visibility constraints are fully resolved.

Files changed (1) +59 / -0

Other (1) +59 / -0
governance-enforce.ymlAdd A_BLOCK diff-scoped governance enforcement workflow +59/-0

Add A_BLOCK diff-scoped governance enforcement workflow

• Introduces a new GitHub Actions workflow that runs on pull requests and pushes to main/master. It installs @wave-av/governance from GitHub Packages with read-only permissions and executes the enforcer against a computed base SHA to block only new violations.

.github/workflows/governance-enforce.yml

devin-ai-integration[bot]

This comment was marked as resolved.

@qodo-code-review

qodo-code-review Bot commented Aug 5, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Fork PR gate may fail 🐞 Bug ☼ Reliability
Description
governance-enforce runs on pull_request and requires authenticated access to
npm.pkg.github.com via GITHUB_TOKEN to install the enforcer package; if fork-origin PR runs
don’t have package access, the enforce check can fail consistently and block those PRs. This is
especially risky if/when the check becomes required via rulesets, since it can make fork-based
contributions unmergeable.
Code

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

+    env:
+      NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+    steps:
Evidence
The workflow is triggered for all pull requests, grants packages: read, and then attempts to
install @wave-av/governance from npm.pkg.github.com using GITHUB_TOKEN. This directly couples
PR check success to package registry access in the pull_request execution context.

.github/workflows/governance-enforce.yml[18-26]
.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
The workflow runs on `pull_request` and installs an org-scoped package from GitHub Packages using `GITHUB_TOKEN`. If fork PR tokens cannot read that package, the job will fail early during install and the required `enforce` check will not go green for fork PRs.

### Issue Context
The workflow is intended to be required by an org ruleset (`enforce` status check). Required checks that systematically fail for a PR class (e.g., forks) can deadlock contributions.

### Fix Focus Areas
- .github/workflows/governance-enforce.yml[18-26]
- .github/workflows/governance-enforce.yml[34-48]

### Suggested change options
Pick one that matches your policy:
1) **Make the package readable in fork PR context** (preferred if you need scanning on forks): adjust package visibility/access so the workflow token can fetch it during `pull_request` runs.
2) **Add a fork-safe fallback**: detect lack of package access and run a vendored or otherwise locally-available scanner path so the job still reports a meaningful result.
3) **Skip on forks (only if acceptable)**: add a job-level `if:` gate to run only when the PR head repo is the same as the base repo, avoiding hard failures—verify that your required-check policy treats the resulting status as acceptable before enabling the ruleset requirement.

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


2. Unpinned governance package ✓ Resolved 🐞 Bug ⛨ Security
Description
The governance-enforce workflow installs @wave-av/governance with a caret range (^0.4.4), so
the required gate can change behavior (or break) without any repo change, leading to unpredictable
pass/fail results over time. This undermines determinism expectations already documented in this
repo’s other security gate workflows.
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 currently resolves @wave-av/governance using a semver range at run time. Another
workflow in this repo explicitly calls out determinism and pins + verifies its downloaded scanner
tooling, demonstrating that non-deterministic security gates are an operational risk here.

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

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

### Issue description
`governance-enforce` installs `@wave-av/governance@^0.4.4`, which makes the enforcement gate non-deterministic across time because npm may resolve different versions without any code change.

### Issue Context
This repository already documents a preference for deterministic, pinned security tooling in `public-repo-guard`.

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

### Suggested change
- Replace `@wave-av/governance@^0.4.4` with an exact version (e.g. `@wave-av/governance@0.4.4`).
- If you want stronger integrity guarantees, add an integrity verification step (e.g., pin to a known artifact/checksum) consistent with the repo’s existing "pinned + verified" security gate approach.

ⓘ 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-enforcing CI workflow with package authentication, diff-base logic, and a future required-check contract; it is localized but not low-risk enough for lite, while one edit site does not justify extended.

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

Qodo Logo

Comment thread .github/workflows/governance-enforce.yml Outdated
Comment on lines +34 to +36
env:
NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
steps:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Fork pr gate may fail 🐞 Bug ☼ Reliability

governance-enforce runs on pull_request and requires authenticated access to
npm.pkg.github.com via GITHUB_TOKEN to install the enforcer package; if fork-origin PR runs
don’t have package access, the enforce check can fail consistently and block those PRs. This is
especially risky if/when the check becomes required via rulesets, since it can make fork-based
contributions unmergeable.
Agent Prompt
### Issue description
The workflow runs on `pull_request` and installs an org-scoped package from GitHub Packages using `GITHUB_TOKEN`. If fork PR tokens cannot read that package, the job will fail early during install and the required `enforce` check will not go green for fork PRs.

### Issue Context
The workflow is intended to be required by an org ruleset (`enforce` status check). Required checks that systematically fail for a PR class (e.g., forks) can deadlock contributions.

### Fix Focus Areas
- .github/workflows/governance-enforce.yml[18-26]
- .github/workflows/governance-enforce.yml[34-48]

### Suggested change options
Pick one that matches your policy:
1) **Make the package readable in fork PR context** (preferred if you need scanning on forks): adjust package visibility/access so the workflow token can fetch it during `pull_request` runs.
2) **Add a fork-safe fallback**: detect lack of package access and run a vendored or otherwise locally-available scanner path so the job still reports a meaningful result.
3) **Skip on forks (only if acceptable)**: add a job-level `if:` gate to run only when the PR head repo is the same as the base repo, avoiding hard failures—verify that your required-check policy treats the resulting status as acceptable before enabling the ruleset requirement.

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

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-triggered pull_request runs still authenticate as the base repository's GITHUB_TOKEN with packages:read, matching the principal that succeeded on the public-repo pilot; the file is also vendored verbatim from the fleet template, so any fork gating belongs upstream.

@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.

1 issue found across 1 file

Confidence score: 3/5

  • In .github/workflows/governance-enforce.yml, using ^0.4.4 with no lockfile or integrity pin means the gate can pull different package code over time, creating a supply-chain path for CI behavior changes or compromise; this is the highest-risk area because it can affect every run. Pin an exact version and add an integrity-verified install path (or committed lockfile) so the workflow is reproducible.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".github/workflows/governance-enforce.yml">

<violation number="1" location=".github/workflows/governance-enforce.yml:48">
P1: The gate executes a moving package version on every run: `^0.4.4` allows later 0.4.x releases, and this isolated install has no committed lockfile or integrity pin. A compromised or behavior-changing release can therefore change or bypass the required A_BLOCK check without a workflow change. A reviewed, integrity-pinned artifact (or a committed lockfile installed with `npm ci`) would make the enforcer reproducible.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant GH as GitHub
    participant WF as governance-enforce workflow
    participant Checkout as actions/checkout
    participant Node as actions/setup-node
    participant Runner as $RUNNER_TEMP/gov
    participant Gov as @wave-av/governance

    Note over GH,Gov: CI-only governance scan, no app runtime changes

    alt pull_request event
        GH->>WF: trigger on PR
        WF->>WF: set PR_BASE_SHA = github.event.pull_request.base.sha
    else push to main/master
        GH->>WF: trigger on push
        WF->>WF: set PUSH_BEFORE_SHA = github.event.before
    end

    WF->>WF: set permissions: contents:read, packages:read
    WF->>WF: set NODE_AUTH_TOKEN = ${{ secrets.GITHUB_TOKEN }}

    WF->>Checkout: checkout with fetch-depth:0, persist-credentials:false
    Checkout-->>WF: full git history (needed for diff)

    WF->>Node: setup Node 22
    Node-->>WF: node available

    WF->>Runner: mkdir -p $RUNNER_TEMP/gov && cd $RUNNER_TEMP/gov
    Runner->>Runner: write .npmrc with literal ${NODE_AUTH_TOKEN}
    Runner->>GH: npm install @wave-av/governance@^0.4.4 --no-save
    Note over Runner,GH: Authenticated via GITHUB_TOKEN to GitHub Packages
    GH-->>Runner: package installed in isolated temp dir

    WF->>WF: resolve BASE commit
    alt PR_BASE_SHA set
        WF->>WF: BASE = PR_BASE_SHA
    else PUSH_BEFORE_SHA set and not all-zeroes
        WF->>WF: BASE = PUSH_BEFORE_SHA
    else fallback
        WF->>WF: BASE = HEAD~1 (or HEAD if no parent)
    end

    WF->>Gov: node enforce.mjs --changed "$BASE"
    Note over WF,Gov: Diff-scoped: only checks changes against BASE
    Gov->>Gov: scan diff for secrets-in-git, Doppler, hardcoded-paths
    alt violations found in diff
        Gov-->>WF: exit non-zero, list violations
        WF-->>GH: ❌ enforce check fails
    else no new violations
        Gov-->>WF: exit zero
        WF-->>GH: ✅ enforce check passes (unblocks PR merge)
    end

    Note over WF,GH: This PR is the liveness drill — enforce runs on itself
    Note over GH: Org ruleset governance-a-block-enforce<br/>requires this check (after PR merges)
Loading

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

Re-trigger cubic

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

P1: The gate executes a moving package version on every run: ^0.4.4 allows later 0.4.x releases, and this isolated install has no committed lockfile or integrity pin. A compromised or behavior-changing release can therefore change or bypass the required A_BLOCK check without a workflow change. A reviewed, integrity-pinned artifact (or a committed lockfile installed with npm ci) would make the enforcer reproducible.

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 gate executes a moving package version on every run: `^0.4.4` allows later 0.4.x releases, and this isolated install has no committed lockfile or integrity pin. A compromised or behavior-changing release can therefore change or bypass the required A_BLOCK check without a workflow change. A reviewed, integrity-pinned artifact (or a committed lockfile installed with `npm ci`) would make the enforcer reproducible.</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 Outdated
…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_ea9be8af-8513-4ffa-94a3-60eff666a699)

Comment on lines +80 to +81
BASE="$(git rev-parse --verify --quiet 'HEAD~1' || true)"
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.

🔴 Critical workflows/governance-enforce.yml:80

When a push is force-updated and github.event.before points to a commit no longer in the checkout, the fallback resolves BASE to HEAD~1. This diffs only the newest commit, so any earlier commits in the force-push are never scanned by the A_BLOCK gate — a secret or hardcoded path introduced in those skipped commits reaches main/master unexamined. Consider falling back to the empty tree (as the code already does when BASE is empty) instead of HEAD~1, so the entire tree is scanned when the original base is unreachable.

          if [ -z "$BASE" ] || [ "$BASE" = "0000000000000000000000000000000000000000" ] \
             || ! git cat-file -e "$BASE^{commit}" 2>/dev/null; then
-            BASE="$(git rev-parse --verify --quiet 'HEAD~1' || true)"
+            BASE=""
          fi
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @.github/workflows/governance-enforce.yml around lines 80-81:

When a push is force-updated and `github.event.before` points to a commit no longer in the checkout, the fallback resolves `BASE` to `HEAD~1`. This diffs only the newest commit, so any earlier commits in the force-push are never scanned by the A_BLOCK gate — a secret or hardcoded path introduced in those skipped commits reaches `main`/`master` unexamined. Consider falling back to the empty tree (as the code already does when `BASE` is empty) instead of `HEAD~1`, so the entire tree is scanned when the original base is unreachable.

Evidence trail:
.github/workflows/governance-enforce.yml:67-93 @ a683baec; .github/workflows/governance-enforce.yml:76-90 @ a683baec; git diff MERGE_BASE..REVIEWED_COMMIT -- .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 4 new potential issues.

Open in Devin Review

Comment on lines +49 to +66
- 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

@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.

🔍 Fork PRs may not be able to read the private @wave-av package

The install step authenticates to npm.pkg.github.com with secrets.GITHUB_TOKEN and packages: read. Reading a package published from a different repository requires that package to have explicitly granted read access to this repo (or be internal/public with org-wide access). If that grant is missing — and notably for pull_request runs originating from forks, where the token is more restricted — npm install fails and the job goes red for reasons unrelated to any A_BLOCK violation. Since the workflow is meant to become a required check across the fan-out, confirm the package access grant exists for each vendored repo before enabling the 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.

Fork-triggered pull_request runs authenticate as the base repo's GITHUB_TOKEN with the same packages:read grant that passed the public-repo pilot, so the principal is identical; the suggested fork-PR drill is a rollout process step, not a workflow defect.

Comment on lines +59 to +66
# 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

@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.

🔍 Run-time-fetched gate contradicts the repo's stated vendoring philosophy

The existing gate in this repo documents an explicit design rule: the scanner config and policy script are "VENDORED into the repo ... they are NOT fetched at run time" so the gate "cannot be reprogrammed out-of-band" (.github/workflows/public-repo-guard.yml:9-15). This new workflow downloads and executes @wave-av/governance from a registry at run time. The exact version pin (0.4.6) mitigates the immediate reprogramming risk, but there is no integrity check (no lockfile, no npm ci, no digest verification) and the behavior of a required security gate now depends on an external registry. Given this repo's own precedent, the deviation is worth an explicit rationale.

Open in Devin Review

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

Comment on lines +82 to +91
# FIX 5 — fail CLOSED. This previously fell back to `BASE=HEAD`, and `--changed HEAD`
# diffs HEAD against itself: an empty diff, zero files scanned, job green. With no
# resolvable base, diff against the EMPTY TREE so every tracked file reads as added and
# the whole repo is scanned. (`--all` also exists in 0.4.6 and would do most of this, but
# it is documented as NOT covering the diff-scoped over-grant detectors. Routing through
# the diff path with an empty base keeps every detector in play.)
if [ -z "$BASE" ]; 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

@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 depends on the enforcer using two-dot diff semantics

The fallback sets BASE to the empty-tree object (git hash-object -t tree /dev/null) and passes it to enforce.mjs --changed. This only produces "every tracked file is added" if the enforcer internally runs a two-dot diff (git diff <base> HEAD). If it uses three-dot (git diff <base>...HEAD), git must compute a merge-base, which fails for a tree object, and since 0.4.6 is documented here as failing closed on git errors, the fallback path would hard-fail every run that hits it (root commit, branch creation, force-push with unreachable before-sha) rather than performing the intended full scan. Worth verifying against @wave-av/governance@0.4.6's implementation before relying on this path.

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.

Verified against the 0.4.6 source: changedArgs uses the two-argument form git diff <base> HEAD, documented as accepting tree objects precisely for this CI empty-tree fallback, and gitFiles fails closed (exit 1) on any git error instead of scanning nothing.

Comment thread .github/workflows/governance-enforce.yml
yakimoto and others added 2 commits August 6, 2026 00:19
… just HEAD~1

Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…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.
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