Conversation
…ment body Measured across all 28 public wave-av repos (claude-workstation#1747, #1794): TWO coverage shapes satisfy the one required check name `Secrets + content policy`. 27 repos triggers: pull_request, push, workflow_dispatch jobs: guard 1 repo triggers: + issues, issue_comment jobs: + body-guard This repo is in the 27. All 28 report the same green check. The outlier is wave-moq-edge, and its own comment says why it matters: "`edited` matters as much as `opened`: a body can be made to leak long after the PR is first raised, and until this workflow covered it, nothing ever re-scanned." A PR/issue/comment BODY is exactly as world-readable as the tree, and until now it was scanned by nothing server-side. That gap was not theoretical on wave-moq-edge: a PR was blocked for naming a private repo in wrangler.toml while the very same name, with more operational detail attached, sat unchallenged in its body. WHAT LANDS HERE — the bundle the workflow's own header names, minus what this repo already has (.gitleaks.toml and content-policy.sh are already vendored): .github/workflows/public-repo-guard.yml replaced (73 -> 163 lines) scripts/public-repo-guard/body-policy.sh new, mode 100755 scripts/public-repo-guard/tests/body-policy.test.sh new, mode 100755 Copied from wave-moq-edge, which has run this shape in production. Modes preserved via the git trees API — the contents API would have created both scripts 100644. HONEST ABOUT WHAT IT CAN DO. On a PR this PREVENTS the merge. On an issue or comment the text is already public the moment it posts, so this is DETECTION: it says go redact, fast. Only a client-side pre-write hook stops that class before publication. Also inherited from the reference: concurrency moves from workflow-level to PER JOB, because the two jobs want opposite behaviour. A workflow-level group forced one policy on both, and rapid body edits cancelled the tree job repeatedly — every cancelled check-run stays attached to the commit, so the PR reported UNSTABLE while the live runs were green. The body gate ships with its own fixtures and runs them in CI. Its NEGATIVE cases are the load-bearing half: a leak gate that blocks legitimate cross-repo references gets switched off, and then it protects nothing. Refs wave-av/claude-workstation#1747. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Excluded labels (none allowed) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
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_9af9e0c1-18c9-4915-a979-c562aab15a80) |
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR adds substantial security-sensitive CI behavior that scans public discussion content and changes merge-gating workflows. The unresolved concern that PR-controlled scanner code and fixtures can influence the gate's own verdict requires human review. Not approved because:
No code changes detected at Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more. |
|
Running ultrareview automatically — This adds a new merge-gating body-scanning job with regex-based allowlists and per-job concurrency; a subtle false-positive or fail-open bug would block or silently unguard PRs across every repo this guard is vendored to.. I'll post findings when complete. |
PR Summary by QodoCI: scan PR/issue/comment bodies in public-repo-guard
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1.
|
Qodo FixerNo findings are within the configured fix scope. To change which findings are fixed, adjust the setting on your Qodo configuration page. |
…ue exit 2 Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
9239ff5 to
178fd84
Compare
…d base ref Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…event a base change fires Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…ckout credentials Three review-driven fixes: - check()'s allowlist filters were '|| true'd: a filter dying with exit >= 2 emptied the match list and reported a DETECTED hit as clean. Each filter now captures its status and exits 2, same as the main scan. - The test fixtures hardcoded real private repo names and credential names; the file is gate-exempt by path, so it published exactly what the gate blocks. All fixture names are now obviously synthetic (acme-*). - All three checkout steps now set persist-credentials: false, matching every other workflow here: these jobs execute scripts from the PR's own tree, and that code must not run beside a token in .git/config. Also documents why a body-guard failure on an issue/comment lands on main's head commit and why the job must stay non-required. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…five The self-test step runs tests/body-policy.test.sh, so an install that copies only the four listed files fails on a step nobody read. List the fixture as part of the install unit. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
… allowlist to prose rules Two review findings on the body gate: - The bootstrap fallback ran the PR's own body-policy.sh whenever the base ref lacked a copy, which is not only the install PR: any PR opened against (or retargeted onto) a branch predating the guard would have its own code judge its own body. The fallback is now the default branch's copy (which a PR cannot control), and if no trusted copy exists anywhere the job fails closed instead of executing untrusted code. - The about-the-control allowlist was applied to every rule, so a line mentioning the gate by name was exempt even when it also carried a live credential. It now applies only to prose rules (internal-marker, private-repo-ops); credential- and infrastructure-format rules stay strict. Fixtures pin both halves. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…logy rule strict on gate-naming lines Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
| - name: body policy self-test (fixtures) | ||
| run: bash scripts/public-repo-guard/tests/body-policy.test.sh |
There was a problem hiding this comment.
🟨 Tree-scan job executes the pull request's own guard scripts as part of a required check
The required guard job checks out the PR's tree and runs scripts/public-repo-guard/tests/body-policy.test.sh (and, pre-existing, content-policy.sh) straight from that untrusted checkout, so a PR can rewrite the fixtures or the policy script it is being judged by and make the self-test pass unconditionally. The new body-guard job explicitly avoids exactly this (.github/workflows/public-repo-guard.yml:164-192 resolves the scanner from the trusted base/default ref), so the two halves of the same gate apply opposite trust models.
Was this helpful? React with 👍 or 👎 to provide feedback.
🌊 WAVE BugBot — 2 finding(s)🔴 1 · 🟠 1
severity: critical · major · minor · info — local review · $0 inference · wave-dispatch · react 👍/👎 to tune |
…dlock The body scan could never go green on the PR that introduces it. It refused to execute the PR's own copy of body-policy.sh and looked for a trusted copy on the base ref, then on the default branch; neither has one until this lands, so the job hard-failed with "No trusted copy of body-policy.sh ... Merge the guard bundle to the default branch to activate this check." That is a bootstrap deadlock, not a policy hit, and it is not fixable by weakening the gate. The trusted-copy dance is unnecessary here. This job triggers on `pull_request`, never `pull_request_target`, so a fork PR gets no write token and no repo secrets. With nothing to steal, running the repo's own copy directly means the worst a PR can do by editing the scanner is fail its own check. The three checkouts collapse to one, and the deadlock disappears. Second defect, visible on this PR right now: the required check "Secrets + content policy" reported `skipping`. Both jobs shared one `on:` block, so the tree job needed a job-level `if:` to sit out comment and body-edit events -- and GitHub still publishes a check-run under the required name with conclusion `skipped`, which branch protection reads as passing and which supersedes an earlier real failure. The mirror-image failure is a comment burst under `cancel-in-progress: true` piling `cancelled` runs onto the same required name, leaving the rollup FAILURE while the newest run is green. Both are the same structural problem: a required check-run name reachable from a trigger set that only the body scan needs. So this is a file-level split, not a job-level one. public-repo-guard.yml now lists only tree-changing events (pull_request opened/reopened/synchronize, push, workflow_dispatch, and the existing merge_group trigger the queue depends on), needs no `if:` at all, and can never publish a skipped or cancelled run for a comment. Coverage is unchanged. public-repo-guard-body.yml carries the non-required "Body content policy" check and gains two triggers the previous draft never had: pull_request_review_comment and pull_request_review, whose bodies are world-readable text nothing scanned. Also here, ported from the generation that already landed in a sibling repo: - ripgrep is installed from a pinned, checksum-verified upstream release with PCRE2 compiled in. `apt-get install ripgrep` has no PCRE2, so every `rg -P` rule exited 2 and the scanner fail-closed red on every run. - Both allowlists (`guard:allow <reason>` and the about-the-control list) are now scoped to the one prose rule that can self-trip, internal-marker. A body has no reviewable diff, so an author could previously neutralise any rule -- including the live-credential rules -- by appending the marker to the same line that leaked. Two regression fixtures cover it. - GUARD_PRIVATE_REPOS is normalised for newlines and CRs before splitting; a multi-line value used to configure only the first name and report a pass over the unscanned rest. - The internal-ip rule exempts the range's own all-zero designation (with or without a CIDR suffix) so a body quoting the gate's own documentation is not blocked with no remedy. A host one address past it still blocks. Kept from this branch, deliberately: the up-front PCRE2 probe; the CI fail-closed when GUARD_PRIVATE_REPOS is empty, with `none` as the explicit opt-out; synthetic fixture names throughout the test file, because it is public; and a STRICT private-repo-ops rule that takes no allowlist at all. Naming the gate must not sanitise wiring topology, and a documented safe example belongs in a file, where the marker lands in a reviewable diff. Verified locally: 36/36 fixtures pass; actionlint and shellcheck clean on all four files; content-policy.sh reports OK against a clean export of the tree; and this PR's own title and body scan clean under the real configuration.
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
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_e6f5f8bc-2545-4ebc-9f22-608de3323ef5) |
| # This job's check-run name ("Body content policy") is NOT a required status | ||
| # context in this repo's ruleset, so it can safely trigger on every comment/review | ||
| # event without any risk of masking or wedging the required tree-scan context — | ||
| # that is the entire reason it lives in a separate file from the tree scan. | ||
| # | ||
| # Honest about what it can and cannot do. On a PR this PREVENTS the merge. On an |
There was a problem hiding this comment.
Suggestion: This check is explicitly not required, so a failing body scan does not prevent a PR merge despite the workflow claiming that it does. [api mismatch]
Assessment: 🔴 Critical · 🔁 Occurrence: Sometimes
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** .github/workflows/public-repo-guard-body.yml
**Line:** 15:20
**Comment:**
*Api Mismatch: This check is explicitly not required, so a failing body scan does not prevent a PR merge despite the workflow claiming that it does.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| # a silent skip of the private-repo rule. A repo with deliberately nothing | ||
| # to guard sets the variable to the literal 'none'. | ||
| GUARD_PRIVATE_REPOS: ${{ vars.GUARD_PRIVATE_REPOS }} | ||
| run: bash scripts/public-repo-guard/body-policy.sh "$RUNNER_TEMP/bodyscan/body.txt" |
There was a problem hiding this comment.
Suggestion: The PR event checks out PR-controlled files, then runs the scanner from that checkout, allowing an author to modify the scanner and bypass body-policy enforcement. [security]
Assessment: 🔴 Critical · 🔁 Occurrence: Often
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** .github/workflows/public-repo-guard-body.yml
**Line:** 136:136
**Comment:**
*Security: The PR event checks out PR-controlled files, then runs the scanner from that checkout, allowing an author to modify the scanner and bypass body-policy enforcement.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
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_4a57d762-4238-4f60-b246-d96d3d171abb) |
User description
This repo's
public-repo-guardhas never scanned a single issue or comment body.Measured across all 28 public wave-av repos (
wave-av/claude-workstation#1747,#1794): two coverage shapes satisfy the one required check nameSecrets + content policy.pull_request, push, workflow_dispatchguardissues,issue_commentbody-guardThis repo is in the 27. All 28 report the same green check — because a required check asserts that something named X passed, never what X examined.
The outlier is
wave-moq-edge, and its own comment says why it matters:That gap was not theoretical there: a PR was blocked for naming a private repo in
wrangler.tomlwhile the very same name, with more operational detail attached, sat unchallenged in its body.What lands
Three files — the bundle the workflow's own header names, minus what this repo already has (
.gitleaks.tomlandcontent-policy.share already vendored, and are checked as prerequisites; a repo missing either is refused rather than half-installed):The workflow's header names four files as the install unit but executes a fifth —
tests/body-policy.test.sh, in its own self-test step. Omitting it installs a workflow that fails on a step nobody read, so the manifest ships it. Modes are preserved via the git trees API; the contents API creates100644regardless, which would silently break running these scripts as executables.Planned by
governance/lib/vendor-bundle.mjs(claude-workstation#1850) against a checked-in manifest, not by ad-hoc shell.One deliberate divergence from the reference, stated rather than silent
The shipped workflow is
wave-moq-edge's withactions/checkoutbumped from v5.0.1 to v7.0.1 (3d3c42e5aac5ba805825da76410c181273ba90b1), the pin already used byclaude-workstation's own gate.Copying verbatim was checked first and rejected on evidence: of the 18 target repos, 17 carry a byte-identical guard, and
wave-realtime-edgealready runs v7.0.0 — so a verbatim copy would have downgraded it, and shipped a stale pin to the other 17. A separate PR brings the reference itself up to the same pin.Honest about what this can and cannot do
On a PR this PREVENTS the merge. On an issue or comment the text is already public the moment it posts, so this is DETECTION: it says go redact, fast. Only a client-side pre-write hook stops that class before publication.
Also inherited from the reference: concurrency moves from workflow-level to per job, because the two jobs want opposite behaviour. A workflow-level group forced one policy on both, and rapid body edits cancelled the tree job repeatedly — every cancelled check-run stays attached to the commit, so the PR reported UNSTABLE while the live runs were green.
The body gate ships with its own fixtures and runs them in CI. Its negative cases are the load-bearing half: a leak gate that blocks legitimate cross-repo references gets switched off, and then it protects nothing.
Refs
wave-av/claude-workstation#1747.Note
Medium Risk
Touches the required CI security gate and new merge-blocking body policy; misconfiguration of
GUARD_PRIVATE_REPOSor overly broad rules could block PRs, though the workflow split specifically reduces false merge wedges.Overview
Adds server-side scanning for world-readable GitHub text (PR titles/bodies, issues, comments, inline review comments, and review summaries) that the existing tree gate never touched.
A new
public-repo-guard-body.ymlworkflow materializes event payload text viajq(no shell interpolation) and runsbody-policy.sh, which blocks credential-shaped strings, infra identifiers, unquoted internal markers, and private repo names only when paired with operational detail (secret names, bindings, secret counts)—while allowing bare cross-repo references. The check is named Body content policy and is intentionally not the required branch-protection context.public-repo-guard.ymlis refactored so the required Secrets + content policy job runs only on tree-changing events (pull_requestsynchronize/open/reopen, push, merge queue, dispatch), with per-job concurrency (cancel-in-progresson tree scans only). This split fixes production issues where comment-driven runs could mask failed tree scans or leave cancelled required checks that blocked merges. Both workflows pinactions/checkoutv7.0.1, setpersist-credentials: false, and install a checksum-pinned PCRE2 ripgrep so policy scripts fail loudly instead of on Ubuntu’s non-PCRE2rg.The tree job now runs
body-policy.test.shfixtures so precision regressions (e.g. legitimateacme-beta#260references) are caught in CI.GUARD_PRIVATE_REPOSmust be set in Actions (or explicitlynone) or body scans fail closed in CI.Reviewed by Cursor Bugbot for commit 614a255. Bugbot is set up for automated code reviews on this repo. Configure here.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.Note
Scan PR, issue, and comment bodies for credential and infrastructure leaks in CI
body-guardjob to public-repo-guard.yml that triggers on PR, issue, and issue comment events; materializes the title/body from the GitHub event payload and scans it with the new body-policy.sh scanner.rg -P(PCRE2 required) to block credential formats (Stripe, Anthropic, GitHub PATs, AWS AKIDs, embedded private keys), self-identified not-for-public markers, operator home paths, and private-repo names near operational detail patterns driven byGUARD_PRIVATE_REPOS.guardjob to validate scanner behavior before any real scan.GUARD_PRIVATE_REPOSmust be set in CI; an empty value exits 2 and fails the job.Macroscope summarized 614a255.
Review-driven hardening (post-review commits)
(?i)is now scoped to the private-repo name alternation ((?i:...)); a top-level flag leaked intoOPS_DETAILand made the SCREAMING_CASE credential rule match lowercase prose likecache_key. Two fixtures pin both halves (names stay case-insensitive, prose stays clean).body-guardnow executes the scanner from the trusted base ref (second sparse checkout atgithub.event.pull_request.base.sha), so a PR can no longer editbody-policy.shin the same push that leaks in its body and go green. When the base ref predates the guard (a long-lived branch, or a retargeted PR), the fallback is the default branch's copy, never the PR's own checkout; if no trusted copy exists anywhere the job fails closed rather than execute untrusted code.Both jobs probe for a PCRE2-capable ripgrep up front and fail with an error naming the problem instead of an opaque
exit 2.The about-the-control allowlist is scoped to prose rules only; a line naming the gate no longer exempts a credential-format hit on the same line.
Summary by Sourcery
Extend public-repo-guard to scan all public discussion text while preserving reliable required checks for published repository trees.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
CodeAnt-AI Description
Scan all public discussion text for secrets and internal information
What Changed
Impact
✅ Fewer secrets published in issue and review text✅ Re-scans edited public content✅ Reliable required security checks💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.