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: Pro Plus 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_408266c9-b601-43bd-9d43-71fbbc3d4fa7) |
|
Running ultrareview automatically — This PR changes the public-repo-guard CI itself — adding a body-scanning job with regex/allowlist logic and new issue/comment triggers — so a missed bug could either leak secrets through the gate or break the required check on every PR.. I'll post findings when complete. |
PR Summary by QodoCI: add issue/comment body scanning to public-repo-guard
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review Unable to check for correctness in 95db039. New CI security gate feature with significant logic. An unresolved review comment identifies a valid bypass vulnerability: the body-guard job runs the policy script from the PR's own checkout, allowing malicious PRs to modify the script and bypass the gate. Security infrastructure changes with open security concerns warrant human review. You can customize Macroscope's approvability policy. Learn more. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| expect() { | ||
| local want="$1" name="$2" body="$3" out rc | ||
| printf '%s\n' "$body" > "$TMP/body.txt" | ||
| out="$(bash "$SCRIPT" "$TMP/body.txt" 2>&1)"; rc=$? |
There was a problem hiding this comment.
🔍 The apt fallback can install a ripgrep without PCRE2, turning the new probe into a permanent red job
Both the tree gate and the new body gate use rg -P. The install step is command -v rg >/dev/null || apt-get install -y ripgrep. On Ubuntu 22.04 the distro package (ripgrep 13.0.0-2ubuntu0.1) is built WITHOUT PCRE2 — verified locally: printf 'probe' | rg -P '^probe$' exits 2. If the hosted runner image does not already ship a PCRE2-enabled ripgrep, the new probe at scripts/public-repo-guard/body-policy.sh:34-35 makes body-guard fail with exit 2 on every PR/issue/comment (and content-policy.sh would already be failing closed on every rule). Worth confirming against an actual green run of this workflow; if the runner's rg is the apt build, the install step should download a PCRE2-enabled ripgrep release instead of relying on apt.
Was this helpful? React with 👍 or 👎 to provide feedback.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Qodo FixerNo findings are within the configured fix scope. To change which findings are fixed, adjust the setting on your Qodo configuration page. |
…list from hard hits, probe pcre2, rerun tree scan on base retarget Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
4044e64 to
c21e064
Compare
…hard rules from all line-level allowlists Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ultrareview completed in 11m 14s
1 issue found and verified against the latest diff
Confidence score: 2/5
- In
.github/workflows/public-repo-guard.yml, the workflow runsscripts/public-repo-guard/body-policy.shfrom PR-controlled contents, so a contributor can alter the script in the same PR to bypass the body scan and merge policy-violating changes unnoticed—run the policy from a trusted ref (for example, base/default branch) or inline/pin the check logic in the workflow.
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/public-repo-guard.yml">
<violation number="1" location=".github/workflows/public-repo-guard.yml:136">
P0: The new body scan can be bypassed on PRs because it executes `scripts/public-repo-guard/body-policy.sh` from the checked-out PR contents. That lets a contributor modify the policy script in the same PR and make the check pass even when the body contains content the gate is meant to block. Using a trusted ref for the policy checkout (for PRs, the base SHA) would keep the scanner logic immutable from the untrusted change being evaluated.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| with: | ||
| # Only the gate's own scripts are needed — no reason to pay for the whole | ||
| # tree on every comment. | ||
| sparse-checkout: scripts/public-repo-guard |
There was a problem hiding this comment.
P0: The new body scan can be bypassed on PRs because it executes scripts/public-repo-guard/body-policy.sh from the checked-out PR contents. That lets a contributor modify the policy script in the same PR and make the check pass even when the body contains content the gate is meant to block. Using a trusted ref for the policy checkout (for PRs, the base SHA) would keep the scanner logic immutable from the untrusted change being evaluated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/public-repo-guard.yml, line 132:
<comment>The new body scan can be bypassed on PRs because it executes `scripts/public-repo-guard/body-policy.sh` from the checked-out PR contents. That lets a contributor modify the policy script in the same PR and make the check pass even when the body contains content the gate is meant to block. Using a trusted ref for the policy checkout (for PRs, the base SHA) would keep the scanner logic immutable from the untrusted change being evaluated.</comment>
<file context>
@@ -71,3 +92,72 @@ jobs:
+ with:
+ # Only the gate's own scripts are needed — no reason to pay for the whole
+ # tree on every comment.
+ sparse-checkout: scripts/public-repo-guard
+ sparse-checkout-cone-mode: false
+
</file context>
| sparse-checkout: scripts/public-repo-guard | |
| ref: ${{ github.event.pull_request.base.sha || github.sha }} | |
| sparse-checkout: scripts/public-repo-guard |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
| if: >- | ||
| (github.event_name == 'pull_request' | ||
| && (github.event.action != 'edited' || github.event.changes.base != null)) | ||
| || github.event_name == 'push' | ||
| || github.event_name == 'workflow_dispatch' |
There was a problem hiding this comment.
🔍 Tree job now reports 'skipped' for the required check on body-only edits
With the new job-level if, a pull_request: edited event (title/body edit without a base change) still starts the workflow but skips guard, producing a check run named Secrets + content policy with conclusion skipped on the head commit. GitHub treats a skipped job as passing for required status checks, so this should not block or falsely green anything, but it does mean the most recent instance of the required check on a PR can be a skip rather than a real scan. Worth confirming against this repo's branch-protection configuration if it uses any tooling that distinguishes skipped from success.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| # Only the gate's own scripts are needed — no reason to pay for the whole | ||
| # tree on every comment. | ||
| sparse-checkout: scripts/public-repo-guard | ||
| sparse-checkout-cone-mode: false |
There was a problem hiding this comment.
🟨 Body-scanning gate executes the gate script from the pull request's own branch
The new body-guard job checks out the PR's merge ref by default and then runs scripts/public-repo-guard/body-policy.sh from that checkout, so a pull request (including one from a fork) can modify or neuter the very script that is supposed to block leaks in its title/body. The tree guard job has the same property for content-policy.sh, but the new job extends the pattern to the body gate as well. Impact is limited to gate evasion (the job runs with a read-only token and no secrets other than the GUARD_PRIVATE_REPOS variable), not code execution against protected resources.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
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, comment, or review 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.
Review-surface text (submitted review bodies and inline review comments) arrives as
pull_request_review/pull_request_review_comment, notissue_comment; both triggers were added during review so no world-readable body class is left unscanned.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.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.Note
Medium Risk
Changes security gate behavior and CI triggers for all PR/issue/comment activity; mis-tuned body rules could block legitimate merges or miss leaks, but scope is vendored scripts and read-only workflow permissions.
Overview
Extends public-repo-guard so world-readable PR, issue, and comment text is scanned server-side, not only the published tree. The workflow now triggers on
editedevents (bodies can leak after first open) and adds abody-guardjob that materializes untrusted title/body from the event payload into a file and runsbody-policy.sh—without shell interpolation of hostile body content.The existing
guardjob is narrowed so it skips issue/comment events and PRedited(tree unchanged), and concurrency is per job: tree scans can cancel in progress; body scans do not, so cancelled checks do not leave PRs looking unstable.body-policy.shmirrors many tree rules (credentials, infra IDs, internal markers) but diverges on private repos: bare cross-repo mentions are allowed; blocks apply when a configured private name appears within ~140 characters of operational detail (credential names, secret bindings, secret counts). Violation output redacts matched text in annotations.body-policy.test.shexercises block/pass precision and fail-closed paths; the tree job runs those fixtures in CI.actions/checkoutis bumped to v7.0.1 in both jobs;body-guarduses sparse checkout ofscripts/public-repo-guardonly.Reviewed by Cursor Bugbot for commit 9457c13. Configure here.
Note
Scan PR/issue/comment bodies for policy violations in public-repo-guard CI
body-guardjob in public-repo-guard.yml that runs on PR, issue, comment, and review events, materializing untrusted title/body text from$GITHUB_EVENT_PATHand scanning it via the new body-policy.sh.guardjob gains aniffilter to skip non-tree-changing events (issue/comment edits), reducing unnecessary runs.guardjob.Macroscope summarized 95db039.