ci: scan issue and comment bodies — this repo has never scanned one - #64
ci: scan issue and comment bodies — this repo has never scanned one#64yakimoto wants to merge 11 commits into
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>
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_263f3bd0-3b45-4774-bc37-17387c394f59) |
|
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 |
|
Running ultrareview automatically — Adding a repo-wide body-scanning security gate touches CI/CD behavior on every PR/issue/comment; a subtle regex or allowlist bug could block all merges or let a leak through, so this warrants a slower multi-pass review.. I'll post findings when complete. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review Unable to check for correctness in 2a09911. This PR introduces a significant new security scanning feature with unresolved review comments identifying a potential detection bug (word boundary issue in proximity rule) and a security gap (.guardignore from untrusted tree can neuter scans in privileged runs). These substantive concerns warrant human review. You can customize Macroscope's approvability policy. Learn more. |
PR Summary by QodoCI: scan PR/issue/comment bodies with public-repo-guard body-guard job
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| raw="$(rg -nP --no-filename -- "$re" "$FILE" 2>/dev/null)"; rc=$? | ||
| if (( rc >= 2 )); then | ||
| echo "::error title=public-repo-guard ($name)::ripgrep failed (exit $rc) scanning rule '$name' — failing closed." | ||
| exit 2 | ||
| fi | ||
| # Filter with rg, not grep: BSD/macOS grep has no -P, so a `grep -P` allowlist | ||
| # silently errors out locally while working on GNU/CI — the gate would then | ||
| # disagree with itself depending on where it ran. rg is already required above. | ||
| local matches | ||
| matches="$(printf '%s' "$raw" \ | ||
| | rg -vN -- 'guard:allow[[:space:]]+[^[:space:]]' \ | ||
| | rg -vNiP -- "$ABOUT_THE_CONTROL" || true)" |
There was a problem hiding this comment.
🔍 apt-installed ripgrep on older Ubuntu has no PCRE2, which would fail the whole gate closed
Both new/existing Install ripgrep steps fall back to apt-get install ripgrep. I verified locally on Ubuntu 22.04 that the distro package (13.0.0-2ubuntu0.1) is built without PCRE2: rg -P exits 2 with "PCRE2 is not available in this build of ripgrep". Every rule in scripts/public-repo-guard/body-policy.sh uses rg -nP, and check() treats exit >= 2 as a scanner error and exits 2 — so on such a runner the body gate (and content-policy.sh, which has the same dependency) would fail closed on every event rather than scan anything. Running the fixture suite here reproduces exactly that: 25/29 fixtures fail with "ripgrep failed (exit 2)"; with a PCRE2-capable engine all 29 pass.
This is fine as long as ubuntu-latest keeps providing a PCRE2-enabled ripgrep (24.04 ships 14.x, which does). It is worth noting because the failure mode is a hard red on every PR/issue/comment, and because it makes the fixture suite unrunnable for contributors on older distros.
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}
| AKID_FIXTURE="AKI""A1234567890ABCDEF" | ||
| expect 1 'AWS access key id' \ | ||
| "The failing job had ${AKID_FIXTURE} configured." |
There was a problem hiding this comment.
1. Aws key fixture in tests 📘 Rule violation ⛨ Security
The new fixture test constructs an AKIA... AWS access key ID value, which is a credential-like secret pattern committed in source. Even if intended as a dummy, it risks violating the repository policy against hard-coded secrets and can trigger secret scanners unnecessarily.
Agent Prompt
## Issue description
`scripts/public-repo-guard/tests/body-policy.test.sh` constructs an AWS access key ID fixture (an `AKIA...`-shaped credential) from string literals. The compliance rule disallows hard-coded secrets/credential-like values in committed source, even in tests.
## Issue Context
The test still needs a string that matches the `aws-akid` regex in `body-policy.sh`, but it should avoid embedding a credential-shaped full value directly in source.
## Fix Focus Areas
- scripts/public-repo-guard/tests/body-policy.test.sh[57-59]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Qodo Fixer🍒 Ready to be cherry-picked — ✅ Merged (0) · ☑ Fixed (1) 🔗 Fix PR: #65 This fix PR was closed automatically. Its branch is preserved so you can cherry pick the changes into the original PR. Prompt for coding agent Process — 1 fixed
|
…ted PRs Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…he test file 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}
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>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…y from the default branch Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
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>
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_name == 'issues' | ||
| || github.event_name == 'issue_comment' | ||
| || github.event_name == 'pull_request_review' | ||
| || github.event_name == 'pull_request_review_comment' |
There was a problem hiding this comment.
🔍 Issue/issue-comment failures attach a red check-run to the default branch
body-guard runs on issues and issue_comment, whose GITHUB_REF/GITHUB_SHA point at the default branch. A leaking issue comment therefore posts a FAILED Body content policy check-run against the latest commit on main. That is the intended detection signal, but it also means any tooling that reads the combined commit status of main (release automation, dashboards, deploy gates) will see main as failing because of text in an unrelated issue. Worth confirming nothing downstream keys off the default branch's commit status.
Was this helpful? React with 👍 or 👎 to provide feedback.
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…mption-filter errors Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
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_33ecabf1-82a1-462a-80a3-388d038acf9b) |
| elif [[ "${GITHUB_ACTIONS:-}" == "true" ]]; then | ||
| echo "::error title=public-repo-guard (private-repo-ops)::GUARD_PRIVATE_REPOS resolved empty in CI — the private-repo proximity rule would silently not run. Set the org/repo Actions variable vars.GUARD_PRIVATE_REPOS; failing closed rather than reporting a pass that enforces nothing." | ||
| exit 2 |
There was a problem hiding this comment.
🔍 Missing GUARD_PRIVATE_REPOS now hard-fails, diverging from the tree gate's silent skip
content-policy.sh:101-110 silently skips the private-repo rule when GUARD_PRIVATE_REPOS is empty, including in CI. The new body gate instead exits 2 whenever GITHUB_ACTIONS=true and the variable resolves empty. The divergence is deliberate and documented, but it means the Body content policy job goes red on every PR, issue and comment in this repo unless vars.GUARD_PRIVATE_REPOS is actually set at org or repo level here — something the currently-green tree gate would not have revealed, since it just skips. Worth confirming the variable is configured for wave-av/sdk (and that org variables are exposed to fork-PR runs, otherwise fork PRs will always fail this job) before or immediately after merge.
Was this helpful? React with 👍 or 👎 to provide feedback.
…dy-guard Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
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_63dc2fa0-88fb-4f5f-9e17-61b7606bdc55) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… name-first proximity rule Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
| - name: content policy (WAVE trade-secret / internal-leak gate) | ||
| env: | ||
| GUARD_PRIVATE_REPOS: ${{ vars.GUARD_PRIVATE_REPOS }} | ||
| run: bash scripts/public-repo-guard/content-policy.sh . | ||
| GATE_DIR: ${{ steps.gate.outputs.dir }} | ||
| run: bash "$GATE_DIR/scripts/public-repo-guard/content-policy.sh" . |
There was a problem hiding this comment.
🟨 Privileged review-event scan still honours scan-scope files taken from the untrusted pull request tree
On pull_request_review / pull_request_review_comment the guard job pins only the gate's executables (.gitleaks.toml, content-policy.sh) to the default branch, but the scan itself still runs against the checked-out PR tree and both scanners read scope-control files from that same untrusted tree: content-policy.sh appends every glob from a repo-root .guardignore to its exclude list (scripts/public-repo-guard/content-policy.sh:34-39), and gitleaks honours a .gitleaksignore at --source .. A pull request can therefore add .guardignore containing ** and make the pinned, trusted scanners report a clean tree.
(Refers to lines 161-172)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Honoring .guardignore/.gitleaksignore from the scanned tree is the gate's documented, diff-visible allowlist mechanism: any suppression file a PR adds is visible in the public diff under review, and the same suppression applies identically in the unprivileged pull_request run, so there is no privilege escalation specific to review events. This finding was reviewed and deliberately skipped in four prior passes for the same reason.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| - name: body policy (PR / issue / comment / review text) | ||
| env: | ||
| GUARD_PRIVATE_REPOS: ${{ vars.GUARD_PRIVATE_REPOS }} | ||
| run: | | ||
| set -euo pipefail | ||
| script="trusted/scripts/public-repo-guard/body-policy.sh" | ||
| if [ ! -f "$script" ]; then | ||
| if [ -f "bootstrap/scripts/public-repo-guard/body-policy.sh" ]; then | ||
| echo "::notice title=public-repo-guard (body-guard)::body-policy.sh is not on the default branch yet; bootstrap run using this same-repo PR's own copy." | ||
| script="bootstrap/scripts/public-repo-guard/body-policy.sh" | ||
| else | ||
| # No trusted copy and no same-repo bootstrap copy: this is a fork | ||
| # PR (or a non-PR event) during the bootstrap window. Refuse to | ||
| # grade a body with a scanner the untrusted tree supplies. | ||
| echo "::error title=public-repo-guard (body-guard)::body-policy.sh is not on the default branch and this event's tree is untrusted; refusing to execute the PR's own scanner. Merge the guard bundle to the default branch first." | ||
| exit 1 | ||
| fi | ||
| fi | ||
| bash "$script" "$RUNNER_TEMP/bodyscan/body.txt" |
There was a problem hiding this comment.
False positive: on issues/issue_comment events the bootstrap checkout (the only non-default-branch ref) is statically disabled by its step-level if (event must be pull_request/pull_request_review/pull_request_review_comment and same-repo), so line 306 only ever executes the default branch's copy or fails closed. The job also never writes to the Actions cache, so no poisoning vector exists.
2 participants
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
Changes security-gate CI behavior and trusted-script sourcing on privileged review events; misconfiguration of vars.GUARD_PRIVATE_REPOS is designed to fail closed, but the new surface area warrants careful branch-protection alignment for the new check job.
Overview
Extends public-repo-guard so GitHub title/body text (PRs, issues, comments, and review summaries) is scanned server-side, not only the published tree. A new
body-guardjob materializes untrusted text from the event payload viajqand runsbody-policy.sh, which flags credential-shaped strings, infra markers, and private-repo names only when paired with operational detail (~140 chars)—so bare cross-repo references stay allowed unlikecontent-policy.shon the tree.The
guardjob gains review-event hardening: onpull_request_review/pull_request_review_comment, gate executables (gitleaks.toml, policy scripts) are pinned from the default branch so fork PRs cannot supply the scanner. Triggers now includeeditedon PRs and bodies; per-job concurrency replaces workflow-level grouping (tree scans cancel in-flight on the same SHA; body scans do not).actions/checkoutis bumped to v7.0.1;body-policy.test.shruns in CI as a self-test.Reviewed by Cursor Bugbot for commit a5711c9. Configure here.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.Note
Scan PR/issue/comment/review bodies for credentials and internal markers in public-repo-guard
body-guardjob triggered by issues, issue comments, PR reviews, and review comments; materializes event body/title text viajqand runsbody-policy.shagainst it.Macroscope summarized 2a09911.