Skip to content

ci: scan issue and comment bodies — this repo has never scanned one #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yakimoto wants to merge 8 commits into
base: main
Choose a base branch
from
Open

ci: scan issue and comment bodies — this repo has never scanned one #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
864f4f5
ci: this repo's public-repo-guard never scanned a single issue or com…
yakimoto Aug 6, 2026
1cbbf9f
fix: scope the about-the-control allowlist to prose rules and (?i) to…
yakimoto Aug 6, 2026
40ff3ac
fix: fail closed when an allowlist filter errors instead of masking i…
yakimoto Aug 6, 2026
897b6cb
fix(guard): remove author-controlled allowlist escapes from body hard…
yakimoto Aug 6, 2026
141c057
fix(guard): close two body/tree scan gaps in the workflow triggers
yakimoto Aug 6, 2026
6435e79
ci: never publish a skippable verdict on a PR head; exempt the CGNAT …
yakimoto Aug 6, 2026
2410a12
ci: install list is five files — the workflow runs the fixture self-test
yakimoto Aug 6, 2026
743a99f
fix: strip carriage returns from GUARD_PRIVATE_REPOS in both policy s…
yakimoto Aug 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 160 additions & 7 deletions .github/workflows/public-repo-guard.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ name: public-repo-guard
# wave-av/.github must not be able to alter another repo's secret scanner). The
# gitleaks binary is version-pinned AND SHA-256-verified before it runs.
#
# To install on a new repo, copy all three files together:
# To install on a new repo, copy all five files together:
# .github/workflows/public-repo-guard.yml
# .gitleaks.toml
# scripts/public-repo-guard/content-policy.sh
# scripts/public-repo-guard/body-policy.sh
# scripts/public-repo-guard/tests/body-policy.test.sh
#
# Scan scope: the published working TREE (gitleaks --no-git), NOT git history. The
# goal is "what is public right now is clean", so a shallow checkout is sufficient.
Expand All @@ -25,24 +27,62 @@ name: public-repo-guard
# path glob to a repo-root `.guardignore`, or extend the repo-local `.gitleaks.toml`.

on:
# `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.
pull_request:
types: [opened, edited, reopened, synchronize]
issues:
types: [opened, edited]
issue_comment:
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
types: [created, edited]
# Inline review comments on a diff are a SEPARATE event from issue_comment —
# without this trigger they are world-readable text that no job ever scans.
pull_request_review_comment:
types: [created, edited]
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
# A submitted review's top-level body (the free-text field above any inline
# comments) is yet another world-readable payload, separate from BOTH comment
# events — without this trigger nothing ever scans it.
pull_request_review:
types: [submitted, edited]
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
push:
branches: [main, master]
workflow_dispatch:

# `pull_request`, deliberately NOT `pull_request_target`: a fork PR must never get
# a write token or repo secrets just because a gate wanted to read its body.
permissions:
contents: read

concurrency:
group: public-repo-guard-${{ github.ref }}
cancel-in-progress: true
# Concurrency is per JOB, not per workflow: the two jobs want opposite behaviour.
# A workflow-level group would force one policy on both, and it showed: rapid body
# edits cancelled the tree job over and over, and every cancelled check-run stays
# attached to the commit, so the PR reported UNSTABLE while the live runs were green.

jobs:
guard:
name: Secrets + content policy
# Skips ONLY issues/issue_comment events: the tree scan has nothing to say
# about a comment, and those events run against the DEFAULT branch, so their
# skipped check runs cannot attach to any PR head. Every event that runs in a
# PR's context (pull_request INCLUDING `edited`, pull_request_review,
# pull_request_review_comment) must run the scan for real: a job skipped by a
# job-level `if` still publishes a check run named "Secrets + content policy"
# with conclusion `skipped` on the PR head SHA, branch protection treats
# skipped as passing, and the newest check run for a name wins — so a mere
# title edit or review comment would flip an already-FAILED required tree
# scan green with nothing re-examining the tree. Re-scanning an unchanged
# tree costs minutes; a maskable required check costs the gate.
if: github.event_name != 'issues' && github.event_name != 'issue_comment'
Comment on lines +64 to +75

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

🔍 Rationale for running the tree scan on review events rests on an incorrect assumption

The comment argues the tree job must run (not be skipped) for pull_request_review and pull_request_review_comment because a skipped job "still publishes a check run ... on the PR head SHA" and would mask an earlier failure. For those two events GitHub runs the workflow in the DEFAULT-branch context (GITHUB_REF/GITHUB_SHA = last commit on the default branch), exactly like issues/issue_comment, which the same if excludes for that very reason. So the check runs from review events attach to the default-branch commit, not the PR head, and could not mask a failed PR check. The practical effect is a full clone + gitleaks install + tree scan of main on every review comment, and (see the reported concurrency finding) collateral cancellation of the PR's own scan. Worth re-deriving the intended trigger matrix.

Open in Devin Review

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

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

Incorrect premise: per GitHub's events reference, pull_request_review and pull_request_review_comment set GITHUB_REF to the PR merge branch (refs/pull/N/merge) and GITHUB_SHA to its last merge commit, not the default branch. Their check runs therefore attach to the PR context, which is precisely why the tree scan must run rather than publish a maskable skipped check.

concurrency:
group: public-repo-guard-tree-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true
Comment on lines +76 to +78

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

🔴 Posting a review comment can cancel a pull request's required secret scan, leaving it stuck

The tree scan for review-triggered events reuses the same cancellation group as the pull request's own scan (group: public-repo-guard-tree-${{ github.event.pull_request.number || github.ref }} with cancel-in-progress: true at .github/workflows/public-repo-guard.yml:76-78), so a review or review comment posted while the pull request's scan is running kills it.
Impact: The required secret/content check on the pull request ends as cancelled and never turns green, blocking the merge until someone manually re-runs it.

Why the review-triggered run cannot replace the cancelled one

The guard job now runs for pull_request_review and pull_request_review_comment (.github/workflows/public-repo-guard.yml:38-46, .github/workflows/public-repo-guard.yml:75). For those events GitHub sets GITHUB_REF/GITHUB_SHA to the default branch, so actions/checkout checks out main, the scan examines the default-branch tree rather than the PR, and the resulting check run attaches to the default-branch commit — not the PR head SHA.

But the payload of those events does contain pull_request.number, so the concurrency group evaluates to public-repo-guard-tree-<PR number>, identical to the group used by the real pull_request-event run for that PR. With cancel-in-progress: true, the review event's run cancels the PR's in-flight tree scan. The PR head SHA is then left with a cancelled check run for "Secrets + content policy", and the replacement run's verdict lands on a different commit — precisely the "cancelled check-run stays attached to the commit" failure mode the comment at .github/workflows/public-repo-guard.yml:56-59 says the per-job concurrency was introduced to avoid.

Possible fixes: key the tree job's group on github.event.pull_request.head.sha || github.ref, exclude review events from the group (or from the job entirely, since they scan the wrong tree), or set cancel-in-progress: false for the review-triggered path.

Prompt for agents 
In .github/workflows/public-repo-guard.yml the `guard` (tree scan) job runs for pull_request, pull_request_review and pull_request_review_comment events, and its job-level concurrency group is keyed on `github.event.pull_request.number || github.ref` with cancel-in-progress: true. For pull_request_review and pull_request_review_comment, GitHub runs the workflow against the DEFAULT branch (GITHUB_REF/GITHUB_SHA point at the default branch) but the payload still carries pull_request.number, so those runs land in the same concurrency group as the PR's real tree scan and cancel it. The cancelled check run stays attached to the PR head SHA while the replacement run scans main and reports on the default-branch commit, so the PR's required check never goes green. Decide whether the tree job should run at all for review events (it cannot scan the PR tree), and if it should, make its concurrency group distinguish the actual scanned ref/commit (e.g. key on github.event.pull_request.head.sha or github.sha) so review activity cannot cancel the PR's own scan.
Open in Devin Review

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

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

False positive built on the same incorrect context claim: review-triggered runs execute on the PR merge ref, so a run that cancels the PR's in-flight scan checks out the same merge tree and reports the same check name on the same head SHA; the newest completed run supplies the verdict and the PR is not left stuck. Sharing the concurrency group is the intended collapse of redundant scans of the same tree.

runs-on: ubuntu-latest
steps:
- uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1

@cubic-dev-ai cubic-dev-ai Bot Aug 6, 2026 •
edited
Loading

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: A fork PR can make the new body check pass without scanning its text. This pull_request job checks out the event's default ref, which is the PR merge commit, and line 163 then executes scripts/public-repo-guard/body-policy.sh from that checkout; the PR author can replace that script (or the workflow) with a no-op. The sparse checkout limits paths but does not make the scanner trusted. The body gate should execute an immutable base-branch copy from a trusted workflow/context, without executing files supplied by the PR.

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

<comment>A fork PR can make the new body check pass without scanning its text. This `pull_request` job checks out the event's default ref, which is the PR merge commit, and line 163 then executes `scripts/public-repo-guard/body-policy.sh` from that checkout; the PR author can replace that script (or the workflow) with a no-op. The sparse checkout limits paths but does not make the scanner trusted. The body gate should execute an immutable base-branch copy from a trusted workflow/context, without executing files supplied by the PR.</comment>

<file context>
@@ -25,24 +26,44 @@ name: public-repo-guard
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd  # v5.0.1
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
 
       # gitleaks' GitHub Action requires a paid license for organizations; the CLI
</file context>
Fix with cubic

with:
# This job only reads the tree — never leave the token sitting in
# .git/config while repo-supplied scripts execute in the workspace.
persist-credentials: false

# gitleaks' GitHub Action requires a paid license for organizations; the CLI
# itself is MIT-licensed and free. Pin the version AND verify the release
Expand All @@ -64,10 +104,123 @@ jobs:
- name: gitleaks (secret scan — published tree)
run: gitleaks detect --no-git --source . --config .gitleaks.toml --redact --no-banner --exit-code 1

- name: Install ripgrep
run: command -v rg >/dev/null || (sudo apt-get update -qq && sudo apt-get install -y -qq ripgrep)
# Both policy scripts use `rg -P` (PCRE2). Ubuntu's apt ripgrep is built
# WITHOUT PCRE2, so `rg -P` exits 2 there and the scripts fail closed —
# red on every run, which gets a gate switched off. Accept a preinstalled
# rg only if it actually has PCRE2; otherwise install the official release
# binary (PCRE2 compiled in), pinned and checksum-verified like gitleaks.
- name: Install ripgrep (pinned + checksum-verified, PCRE2 build)
env:
RIPGREP_VERSION: "14.1.1"
RIPGREP_SHA256: "4cf9f2741e6c465ffdb7c26f38056a59e2a2544b51f7cc128ef28337eeae4d8e"
run: |
if command -v rg >/dev/null && rg --pcre2-version >/dev/null 2>&1; then
echo "using preinstalled $(rg --version | head -n1) with PCRE2"; exit 0
fi
curl -fsSL --proto '=https' --tlsv1.2 -o ripgrep.tar.gz \
"https://github.com/BurntSushi/ripgrep/releases/download/${RIPGREP_VERSION}/ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl.tar.gz"
echo "${RIPGREP_SHA256} ripgrep.tar.gz" | sha256sum -c -
tar -xzf ripgrep.tar.gz --strip-components=1 "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl/rg"
sudo install -m 0755 rg /usr/local/bin/rg
rm -f rg ripgrep.tar.gz
rg --pcre2-version

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

# The body gate's own fixtures. Its negatives are the load-bearing half — a
# leak gate that blocks legitimate cross-repo references gets switched off,
# and then it protects nothing. Runs here so a regression is caught by CI
# rather than by a leak.
- name: body policy self-test (fixtures)
run: bash scripts/public-repo-guard/tests/body-policy.test.sh

# The other half of a public repo's surface. `guard` above scans the published
# TREE; a PR/issue/comment BODY is just as world-readable and, until this job,
# was scanned by nothing server-side. That gap was real, not theoretical: 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.
#
# Honest about what it 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 tells us to go redact, fast. Only the client-side pre-write hook
# can stop that class before publication.
body-guard:
name: Body content policy
if: >-
github.event_name == 'pull_request'
|| github.event_name == 'issues'
|| github.event_name == 'issue_comment'
|| github.event_name == 'pull_request_review_comment'
Comment on lines +150 to +156

@devin-ai-integration devin-ai-integration Bot Aug 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 The body gate only blocks a merge if branch protection is updated to require it

The new job's check name is Body content policy, distinct from the existing required Secrets + content policy. The PR states "On a PR this PREVENTS the merge", but that only holds once branch protection lists the new name as a required status check. Until then the body gate is detection-only on PRs too — the same status the PR assigns to issues/comments. Worth confirming the repo's branch-protection config is updated alongside this merge.

Open in Devin Review

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

|| github.event_name == 'pull_request_review'
concurrency:
# Keyed on the specific comment / review / PR / issue rather than github.ref,
# because issue events all report the default branch and a ref-keyed group
# would let two comments cancel each other, leaving one unscanned. The comment
# and review ids come FIRST: those payloads also carry the PR number, and
# keying them on the PR would collapse two rapid comments into one group,
# dropping a verdict.
#
# cancel-in-progress is deliberately FALSE. Every version of a body deserves a
# verdict, the job is seconds long, and a cancelled check-run lingers on the
# commit and makes an otherwise-green PR look broken.
group: public-repo-guard-body-${{ github.event.comment.id || github.event.review.id || github.event.pull_request.number || github.event.issue.number || github.ref }}
cancel-in-progress: false
Comment on lines +150 to +170

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

🔍 Body-gate verdicts for issue/comment events land on the default branch, not the PR

issues and issue_comment runs execute in the default-branch context, so the Body content policy check run produced by those events attaches to the latest default-branch commit rather than to any PR head. A leak in a comment therefore leaves a persistent red check on main's commit (and repeats on every subsequent comment, because .issue.body is re-scanned each time at .github/workflows/public-repo-guard.yml:216-220). Worth confirming that nothing downstream (release gates, badges, deploy checks) treats a red check on the default-branch commit as blocking, since a comment leak that has already been redacted will not clear it until another run overwrites the same check name.

Open in Devin Review

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

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

Accurate observation of an intended, documented property: the workflow explicitly states issue/comment scans are detection-only ("the text is already public the moment it posts"), and where check runs attach on those events is GitHub behavior, not something this workflow controls. Confirming downstream consumers of default-branch checks is an ops verification task, not a code change in this PR.

runs-on: ubuntu-latest
steps:
- 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
Comment on lines +173 to +178

@devin-ai-integration devin-ai-integration Bot Aug 6, 2026 •
edited
Loading

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 run their own copy of the gate script

The body-guard job checks out the PR merge ref (default actions/checkout behaviour under pull_request) and then executes scripts/public-repo-guard/body-policy.sh from that checkout, so a PR can modify the scanner itself in the same PR whose body is being scanned. This mirrors the pre-existing behaviour of the tree guard job and is contained by permissions: contents: read / no secrets other than vars.GUARD_PRIVATE_REPOS, but if the intent is that the body gate cannot be disarmed by the change under review, the body job would need to run the scripts from the base ref (e.g. a second checkout at github.event.pull_request.base.sha into a separate path).

Open in Devin Review

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

Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
# This job only reads the scripts — never leave the token sitting in
# .git/config while repo-supplied scripts execute in the workspace.
persist-credentials: false

# Same rationale as the tree job: body-policy.sh needs a PCRE2-enabled rg,
# and Ubuntu's apt package has none.
- name: Install ripgrep (pinned + checksum-verified, PCRE2 build)
env:
RIPGREP_VERSION: "14.1.1"
RIPGREP_SHA256: "4cf9f2741e6c465ffdb7c26f38056a59e2a2544b51f7cc128ef28337eeae4d8e"
run: |
if command -v rg >/dev/null && rg --pcre2-version >/dev/null 2>&1; then
echo "using preinstalled $(rg --version | head -n1) with PCRE2"; exit 0
fi
curl -fsSL --proto '=https' --tlsv1.2 -o ripgrep.tar.gz \
"https://github.com/BurntSushi/ripgrep/releases/download/${RIPGREP_VERSION}/ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl.tar.gz"
echo "${RIPGREP_SHA256} ripgrep.tar.gz" | sha256sum -c -
tar -xzf ripgrep.tar.gz --strip-components=1 "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl/rg"
sudo install -m 0755 rg /usr/local/bin/rg
rm -f rg ripgrep.tar.gz
rg --pcre2-version

# The body is read straight out of the event payload FILE and written to
# another file. It is never interpolated into a run: block and never placed
# in an environment variable, so shell metacharacters in a hostile PR body
# have nothing to act on. jq is preinstalled on the GitHub-hosted images.
- name: Materialize the untrusted title/body to a file
run: |
set -euo pipefail
mkdir -p "$RUNNER_TEMP/bodyscan"
# An UNRECOGNIZED payload shape must fail, never quietly scan nothing and
# report a pass. If the event schema ever moves, this job must go red
# rather than become a green rubber stamp over an unscanned body.
if [ "$(jq -r 'has("pull_request") or has("issue") or has("comment") or has("review")' "$GITHUB_EVENT_PATH")" != "true" ]; then
echo "::error title=public-repo-guard (body-guard)::Event payload contains no pull_request/issue/comment/review object — refusing to report a pass on an unscanned body."
exit 1
fi
jq -r '[.pull_request.title, .pull_request.body,
.issue.title, .issue.body,
.comment.body, .review.body]
| map(select(. != null)) | join("\n")' \
"$GITHUB_EVENT_PATH" > "$RUNNER_TEMP/bodyscan/body.txt"
echo "scanning $(wc -l < "$RUNNER_TEMP/bodyscan/body.txt") line(s) of body text"

- name: body policy (PR / issue / comment text)
env:
GUARD_PRIVATE_REPOS: ${{ vars.GUARD_PRIVATE_REPOS }}
run: bash scripts/public-repo-guard/body-policy.sh "$RUNNER_TEMP/bodyscan/body.txt"
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Loading
Loading