fix(ci): stop public-repo-guard's required check from wedging on comment/review bursts

.github/workflows/public-repo-guard-body.yml

@@ -0,0 +1,122 @@
+name: public-repo-guard-body
+
+# The other half of public-repo-guard.yml's coverage, deliberately in its OWN
+# workflow file — see the long comment block at the top of public-repo-guard.yml
+# for the incident (wave-av/cli PR #68) that caused the split and why it is a
+# file-level split, not just a job-level one.
+#
+# `guard` (in public-repo-guard.yml) scans the published TREE and produces the
+# REQUIRED check "Secrets + content policy". This job scans a PR/issue/comment
+# BODY, which is just as world-readable and, until this job existed, 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.
+#
+# 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
+# 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.
+on:
+  # `edited` matters as much as `opened`: a body can be made to leak long after
+  # the PR is first raised, and until this job covered it, nothing re-scanned it.
+  pull_request:
+    types: [opened, edited, reopened, synchronize]
+  issues:
+    types: [opened, edited]
+  issue_comment:
+    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]
+  # 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]
+
+# `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
+
+jobs:
+  body-guard:
+    name: Body content policy
+    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. Since this check-run name is not required, a lingering cancelled
+      # run here cannot wedge a merge the way the tree scan's could — but a dropped
+      # verdict on a body would still be a real coverage gap, so the same "let it
+      # finish" policy applies.
+      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
+    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
+          # 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::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"

.github/workflows/public-repo-guard.yml

@@ -1,6 +1,7 @@
 name: public-repo-guard
 
-# Pre-publication content gate for WAVE public repos. Two complementary checks:
+# Pre-publication content gate for WAVE public repos. Two complementary checks,
+# split across TWO workflow files (this one, plus public-repo-guard-body.yml):
 #   1. gitleaks  — formatted secrets (API keys, tokens, private keys) in the tree.
 #   2. content-policy.sh — WAVE-specific leaks gitleaks misses: live Stripe account
 #      IDs, hardcoded Cloudflare account_ids, developer absolute paths, references
@@ -13,8 +14,9 @@ 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 five files together:
+# To install on a new repo, copy all six files together:
 #   .github/workflows/public-repo-guard.yml
+#   .github/workflows/public-repo-guard-body.yml
 #   .gitleaks.toml
 #   scripts/public-repo-guard/content-policy.sh
 #   scripts/public-repo-guard/body-policy.sh
@@ -25,25 +27,53 @@ name: public-repo-guard
 #
 # Allowlisting: annotate a verified-safe line with `# guard:allow <reason>`, add a
 # path glob to a repo-root `.guardignore`, or extend the repo-local `.gitleaks.toml`.
-
+#
+# WHY THIS IS A SEPARATE WORKFLOW FROM public-repo-guard-body.yml (this used to be
+# one file with two jobs — see PR history for the incident that split it):
+#
+# The `guard` job below produces the check-run named "Secrets + content policy",
+# which is the REQUIRED status context in this repo's branch-protection ruleset.
+# Two failure modes are possible for any job that produces a required check-run
+# from a shared, comment/review-triggered `on:` block, and BOTH were hit in
+# production before this split:
+#
+#   (a) MASKING: if this job's `on:` trigger set includes pull_request_review /
+#       pull_request_review_comment (needed by the BODY scan, not the tree scan)
+#       and the job then uses a job-level `if:` to skip those events (because the
+#       tree hasn't changed), GitHub still publishes a check-run named "Secrets +
+#       content policy" with conclusion `skipped` on that event. Branch protection
+#       treats `skipped` as passing, and the newest check-run for a name wins — so
+#       a bare review comment could flip an already-FAILED required tree scan
+#       green with nothing re-examining the tree. Real incident: this repo shipped
+#       a version that closed (a) by making the job run for real (never skip) on
+#       every PR-context event instead of skipping.
+#
+#   (b) CHURN / FALSE BLOCK: making the job run for real on every review comment,
+#       combined with `cancel-in-progress: true` (needed so a genuine new commit
+#       supersedes a stale scan promptly), means a burst of review-bot comments —
+#       which do not change the tree at all — repeatedly re-fires and cancels the
+#       SAME job. Every cancelled run leaves a `cancelled` check-run attached to
+#       the commit under the required name. Observed live: wave-av/cli PR #68,
+#       head 90b00ded3 — 3 CodeRabbit + 1 gitar-bot comment within 68s produced 7
+#       check-runs named "Secrets + content policy" (5 cancelled, 2 success); the
+#       checks tab showed the latest as green, but the commit's status-check
+#       rollup reported FAILURE and the PR was permanently MERGEABLE/BLOCKED even
+#       though the gate had genuinely passed.
+#
+# (a) and (b) are the SAME structural problem: this job's required check-run name
+# was reachable from an `on:` trigger set that also had to serve comment/review
+# events for the (unrelated) body scan. Splitting into two workflow FILES — not
+# just two jobs — removes the shared trigger set entirely: this file's `on:` block
+# now lists ONLY events that can change the tree (pull_request open/reopen/sync,
+# push, workflow_dispatch). A review comment or a title/body edit never matches
+# this workflow's trigger at all, so GitHub never runs it and never publishes ANY
+# check-run — skipped, cancelled, or otherwise — under the required name for that
+# event. There is nothing left to mask and nothing left to cancel. Coverage is
+# unchanged: every event that can actually alter the published tree still gets a
+# real, non-skippable gitleaks + content-policy run, exactly as before.
 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:
-    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]
-  # 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]
+    types: [opened, reopened, synchronize]
   push:
     branches: [main, master]
   workflow_dispatch:
@@ -53,27 +83,20 @@ on:
 permissions:
   contents: read
 
-# 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'
+    # No job-level `if:` needed: the `on:` block above already scopes this job to
+    # exactly the tree-changing events, so every triggering event is a real run —
+    # never skipped, never a candidate for the masking bug described above.
     concurrency:
+      # cancel-in-progress is TRUE here and it is now safe: the only rapid
+      # retrigger this workflow can see is `synchronize` (a new commit landing
+      # while a prior scan of the OLD tree is still running), and superseding a
+      # stale in-flight scan with a fresh one for the new tree is exactly the
+      # right behaviour. Comment/review bursts cannot reach this workflow at all
+      # (see the block comment above), so this can no longer produce the
+      # cancelled-run pileup that caused PR #68 to wedge.
       group: public-repo-guard-tree-${{ github.event.pull_request.number || github.ref }}
       cancel-in-progress: true
     runs-on: ubuntu-latest
@@ -136,91 +159,3 @@ jobs:
       # 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'
-      || 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
-    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
-          # 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"