From 209e63cbff98ec485f155f32f10f5832e82a92f7 Mon Sep 17 00:00:00 2001 From: yakimoto <66892052+yakimoto@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:20:32 -0400 Subject: [PATCH 01/14] ci: this repo's public-repo-guard never scanned a single issue or comment body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .github/workflows/public-repo-guard.yml | 100 ++++++++++++- scripts/public-repo-guard/body-policy.sh | 139 ++++++++++++++++++ .../tests/body-policy.test.sh | 108 ++++++++++++++ 3 files changed, 342 insertions(+), 5 deletions(-) create mode 100755 scripts/public-repo-guard/body-policy.sh create mode 100755 scripts/public-repo-guard/tests/body-policy.test.sh diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index 719718a..bba2f67 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -13,10 +13,11 @@ 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 four files together: # .github/workflows/public-repo-guard.yml # .gitleaks.toml # scripts/public-repo-guard/content-policy.sh +# scripts/public-repo-guard/body-policy.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. @@ -25,24 +26,44 @@ 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: + types: [created, edited] 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 issue/comment events (the tree scan has nothing to say about a comment, + # and the org should not pay for a gitleaks run every time anyone posts one) and + # skips `edited` (a title or body edit does not change the tree). + if: >- + (github.event_name == 'pull_request' && github.event.action != 'edited') + || github.event_name == 'push' + || github.event_name == 'workflow_dispatch' + concurrency: + group: public-repo-guard-tree-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true 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 # itself is MIT-licensed and free. Pin the version AND verify the release @@ -71,3 +92,72 @@ jobs: 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' + concurrency: + # Keyed on the specific PR / comment / 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. + # + # 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.pull_request.number || github.event.comment.id || 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 + + - name: Install ripgrep + run: command -v rg >/dev/null || (sudo apt-get update -qq && sudo apt-get install -y -qq ripgrep) + + # 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")' "$GITHUB_EVENT_PATH")" != "true" ]; then + echo "::error title=public-repo-guard (body-guard)::Event payload contains no pull_request/issue/comment 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] + | 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" diff --git a/scripts/public-repo-guard/body-policy.sh b/scripts/public-repo-guard/body-policy.sh new file mode 100755 index 0000000..a0b421f --- /dev/null +++ b/scripts/public-repo-guard/body-policy.sh @@ -0,0 +1,139 @@ +#!/usr/bin/env bash +# WAVE public-repo BODY policy — the internal-leak gate for PR/issue/comment text. +# +# Companion to content-policy.sh. That script scans the published working TREE; +# this one scans the other half of a public repo's surface: pull-request titles +# and bodies, issue bodies, and comment bodies. Those are equally world-readable +# and, until this script existed, were scanned by NOTHING server-side. That gap +# was not theoretical — a PR was merged whose wrangler.toml was correctly BLOCKED +# for naming a private repo while the PR body named the same repo, with more +# operational detail attached, and sailed through. +# +# Usage: scripts/public-repo-guard/body-policy.sh +# holds the untrusted text, already materialized to disk. It is passed as +# a PATH and only ever read — the body is never interpolated into a command line +# or an environment variable, so no amount of shell metacharacters in a PR body +# can influence what runs here. +# +# Exit: 0 clean · 1 blocking violation · 2 scanner error (fail closed). +# +# Allowlisting: a line carrying `guard:allow ` is exempt (an accidental +# leak never carries the marker; a deliberate one is visible in a public diff), as +# is any line matching the ABOUT-THE-CONTROL allowlist below. +set -uo pipefail + +FILE="${1:-}" +[[ -n "$FILE" && -f "$FILE" ]] || { echo "::error::body-policy: usage: body-policy.sh "; exit 2; } +command -v rg >/dev/null 2>&1 || { echo "::error::body-policy: ripgrep (rg) required"; exit 2; } + +VIOLATIONS=0 + +# Lines that TALK ABOUT the control rather than leaking through it. Without this, +# the gate blocks its own pull requests and every security discussion — the +# self-referential trap that gets a gate switched off. Ported verbatim in intent +# from the client-side gate's allowlist, which was built for exactly this. +ABOUT_THE_CONTROL='(public-repo-guard|body-policy|content-policy|public-github-write-gate|\bNDA\s+(gate|guard|policy|denylist|sweep|scan|hook)\b|\bno\s+NDA\b|responsib\w*\s+disclos|SECURITY\.md)' + +# check +check() { + local sev="$1" name="$2" re="$3" why="$4" + [[ -z "$re" ]] && { echo "::error::body-policy: internal bug — empty regex for rule '$name'"; exit 2; } + # rg exit: 0=match, 1=no match, >=2=real error → FAIL CLOSED. A gate that passes + # because its scanner broke is worse than no gate: it reports success. + local raw rc + 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)" + [[ -z "$matches" ]] && return 0 + local count; count="$(printf '%s\n' "$matches" | grep -c '')" + # Print the LINE NUMBER only — never the matched text. This annotation is itself + # world-readable, so echoing the hit would re-publish the very thing we caught. + echo "::group::[$sev] $name — $why" + printf '%s\n' "$matches" | sed -E 's/^([0-9]+):.*/ line \1: «match redacted — view the body to see it»/' + echo "::endgroup::" + if [[ "$sev" == "BLOCK" ]]; then + echo "::error title=public-repo-guard ($name)::$why — $count occurrence(s) in the title/body. Edit the body to remove it, then re-run." + VIOLATIONS=$((VIOLATIONS+1)) + else + echo "::warning title=public-repo-guard ($name)::$why — $count occurrence(s) (non-blocking; review)." + fi +} + +# --- Credential formats — never legitimate in prose -------------------------- +check BLOCK stripe-live-key '(sk|rk)_live_[A-Za-z0-9]{16,}' 'Live Stripe secret/restricted key' +check BLOCK stripe-account 'acct_[A-Za-z0-9]{16,}' 'Live Stripe account ID — financial infra, never publish' +check BLOCK anthropic-key 'sk-ant-(api|admin)[0-9]{2}-[A-Za-z0-9_-]{20,}' 'Real Anthropic API/admin key' +check BLOCK github-pat 'github_pat_[A-Za-z0-9_]{30,}' 'GitHub fine-grained PAT' +check BLOCK supabase-pat 'sbp_[a-f0-9]{40}' 'Supabase personal access token' +check BLOCK aws-akid 'AKIA[0-9A-Z]{16}' 'AWS access key ID' +check BLOCK private-key '-----BEGIN [A-Z ]*PRIVATE KEY-----' 'Embedded private key material' + +# --- Infrastructure identifiers ---------------------------------------------- +# shellcheck disable=SC2016 # $CLOUDFLARE_ACCOUNT_ID is literal guidance text +check BLOCK cf-account-id 'account_id\s*[:=]\s*["'"'"']?[0-9a-f]{32}' 'Hardcoded Cloudflare account_id — reference the env var instead' +check BLOCK internal-ip '100\.(6[4-9]|[7-9][0-9]|1[01][0-9]|12[0-7])\.[0-9]{1,3}\.[0-9]{1,3}' 'Internal Tailscale-CGNAT IP (100.64.0.0/10) — internal fleet address' +# shellcheck disable=SC2016 # $HOME is literal guidance text +check BLOCK abs-user-path '/(Users|home)/(?!runner/)[a-z][a-z0-9._-]+/' 'Operator absolute home path — leaks identity and local layout' + +# --- Self-identified internal material --------------------------------------- +# USE vs MENTION. A body that SAYS "internal-only" is leaking; a body that QUOTES +# the phrase is describing a policy — including this one. The lookarounds exempt a +# marker wrapped in straight, smart, or backtick quotes. +# +# Not hypothetical: the first run of this job failed on its own pull request, +# because a review bot had edited the PR body to summarize the change and its +# summary quoted the phrase verbatim. The line-level allowlist could not help — +# that line named no gate. Only use-vs-mention separates the two. +# +# A quoted marker is also a trivial bypass, and that is an accepted trade. The +# threat here is the ACCIDENTAL paste; a deliberate evader has easier routes, and +# `guard:allow ` already exists as the honest, visible one. +check BLOCK internal-marker '(?#260"). A gate that fires on all of +# those gets switched off, and then it protects nothing. +# +# So a bare mention stays silent. What fires is a private repo name within ~140 +# characters of INTERNAL OPERATIONAL DETAIL — a SCREAMING_CASE credential NAME, a +# secret-binding verb, a service binding, or a secret COUNT. That is the topology +# of what is wired to what, and it is the shape that actually leaked. +# +# Names are NOT hardcoded (this file is public); CI injects them via the +# GUARD_PRIVATE_REPOS variable. Unset locally → this check is skipped. +if [[ -n "${GUARD_PRIVATE_REPOS:-}" ]]; then + OPS_DETAIL='(?:[A-Z][A-Z0-9]*_(?:SECRET|TOKEN|KEY|PASSWORD)|wrangler\s+secret|secret\s+(?:is\s+)?(?:bound|binding|list)|(?:is\s+)?bound\s+on|service\s+binding|\d{2,}\s+secrets)' + _ALT='' + IFS=', ' read -r -a _PRIV <<< "$GUARD_PRIVATE_REPOS" + for _name in "${_PRIV[@]}"; do + [[ -z "$_name" ]] && continue + # Regex-escape so metacharacters in a name match literally. + _esc="$(printf '%s' "$_name" | sed -E 's/[][(){}.^$*+?|\\]/\\&/g')" + _ALT="${_ALT:+$_ALT|}${_esc}" + done + if [[ -n "$_ALT" ]]; then + # Both orders: name-then-detail and detail-then-name. + check BLOCK private-repo-ops \ + "(?i)\\b(?:${_ALT})\\b[^\\n]{0,140}?\\b${OPS_DETAIL}|${OPS_DETAIL}[^\\n]{0,140}?\\b(?:${_ALT})\\b" \ + 'A private WAVE repo named alongside internal operational detail (credential name, secret binding, or secret count) — the wiring topology is not public' + fi +fi + +if (( VIOLATIONS > 0 )); then + echo "::error::public-repo-guard: $VIOLATIONS blocking body-policy violation(s) — see annotations above." + exit 1 +fi +echo "public-repo-guard: body policy OK" diff --git a/scripts/public-repo-guard/tests/body-policy.test.sh b/scripts/public-repo-guard/tests/body-policy.test.sh new file mode 100755 index 0000000..13cc9bf --- /dev/null +++ b/scripts/public-repo-guard/tests/body-policy.test.sh @@ -0,0 +1,108 @@ +#!/usr/bin/env bash +# Fixture tests for body-policy.sh. +# +# Deliberately fixture-only: the gate is NEVER proved by writing a real leak into a +# live public PR body, because doing so would publish the exact thing it guards. +# +# The negatives here are the load-bearing half. A leak gate that blocks everything +# is trivially "correct" and useless — it gets disabled within a week. The bare +# cross-reference case below is the one that keeps this gate deployable. +set -uo pipefail + +SCRIPT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/body-policy.sh" +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT + +# The names the real gate is configured with come from an org variable; the tests +# pin their own so they are hermetic and do not depend on CI configuration. +export GUARD_PRIVATE_REPOS="wave-gateway, wave-transports, agent-money" + +PASS=0; FAIL=0 + +# expect +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=$? + if [[ "$rc" == "$want" ]]; then + PASS=$((PASS+1)); printf ' ok %s\n' "$name" + else + FAIL=$((FAIL+1)); printf ' FAIL %s — want exit %s, got %s\n%s\n' "$name" "$want" "$rc" "$out" + fi + # The annotation is world-readable; a hit must never echo the matched text. + if [[ "$rc" == 1 ]] && printf '%s' "$out" | grep -qF "$body"; then + FAIL=$((FAIL+1)); printf ' FAIL %s — LEAKED the matched text into the annotation\n' "$name" + fi +} + +echo "body-policy fixtures" + +# --- must BLOCK --------------------------------------------------------------- +expect 1 'private repo + credential name' \ + 'Flip is live: WAVE_VIEWPORT_LEASE_SECRET is bound on wave-gateway now.' +expect 1 'private repo + credential name, reverse order' \ + 'The MOQ_JOIN_SECRET was added; wave-transports picks it up on deploy.' +expect 1 'private repo + secret count' \ + 'wave-gateway went from 74 secrets to 75 after this change.' +expect 1 'private repo + service binding' \ + 'This adds a service binding from the worker to agent-money for settlement.' +expect 1 'operator home path' \ + 'Repro: run it from /Users/someoperator/Documents/notes and it fails.' # enforce-ignore (fixture) +expect 1 'internal-only marker' \ + 'Attaching the internal-only rollout plan for context.' +# Assembled at run time rather than written as a literal: a fixture that LOOKS like +# a live AWS key trips this repo's own pre-commit secret scanners (it did, on the +# first draft). Splitting the prefix keeps the fixture exercising the real regex +# without parking a credential-shaped string in source. +AKID_FIXTURE="AKI""A1234567890ABCDEF" +expect 1 'AWS access key id' \ + "The failing job had ${AKID_FIXTURE} configured." +expect 1 'internal tailscale IP' \ + 'It resolves to 100.71.4.19 from inside the fleet.' + +# --- must PASS (precision — these keep the gate deployable) ------------------- +expect 0 'bare private-repo cross-reference' \ + 'This is the companion change to wave-transports#260; merge that one first.' +expect 0 'two private repos, no operational detail' \ + 'Both wave-gateway and wave-transports will need a follow-up for this.' +expect 0 'credential NAME with no private repo nearby' \ + 'The handler now reads SOME_API_TOKEN from the environment instead of a literal.' +expect 0 'public runner path is not an operator path' \ + 'CI checks out to /home/runner/work/repo/repo before the scan runs.' # enforce-ignore (fixture) +expect 0 'talking about the control' \ + 'body-policy blocks a private repo named next to a SECRET_TOKEN; that is intended.' +expect 0 'explicit guard:allow with a reason' \ + 'Example for the docs: wave-gateway holds EXAMPLE_SECRET — guard:allow documented-example' +expect 0 'ordinary clean body' \ + 'Bumps the draft revision and regenerates the fixtures. No behaviour change.' +# Regression: the first CI run of this job failed on its own PR, because a review +# bot edited the body to summarize the change and quoted the marker verbatim. +expect 0 'marker MENTIONED in straight quotes is a description' \ + 'Blocks infra identifiers and markers (account_id, home paths, "internal-only" text).' +expect 0 'marker MENTIONED in a code span' \ + 'The rule matches `internal-only` and `for internal use` in body text.' +expect 0 'marker MENTIONED in smart quotes' \ + 'Blocks operator home paths and “internal-only” text.' +expect 1 'marker USED unquoted still blocks' \ + 'Attaching the internal-only rollout plan; do not share outside the team.' + +# --- fail closed -------------------------------------------------------------- +# Invoked directly, not through expect(): expect() always materializes a file, so +# it cannot reach these paths. A gate that returns "OK" when it was handed nothing +# to scan is the failure mode this whole file exists to prevent. +for case in "no argument at all::" "nonexistent path::$TMP/does-not-exist.txt"; do + name="${case%%::*}"; arg="${case##*::}" + if [[ -n "$arg" ]]; then bash "$SCRIPT" "$arg" >/dev/null 2>&1; else bash "$SCRIPT" >/dev/null 2>&1; fi + rc=$? + if [[ "$rc" == 2 ]]; then + PASS=$((PASS+1)); printf ' ok %s → exit 2 (fails closed)\n' "$name" + else + FAIL=$((FAIL+1)); printf ' FAIL %s — want exit 2, got %s\n' "$name" "$rc" + fi +done + +echo " ---" +if (( FAIL > 0 )); then + echo " $PASS passed, $FAIL FAILED"; exit 1 +fi +echo " $PASS passed, 0 failed" From 823498379f765d0352e4cb1c4b7e72704fe73bbe Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 17:32:37 +0000 Subject: [PATCH 02/14] fix(ci): scope body-policy allowlists and guarantee a PCRE2 ripgrep Three review findings on the body gate: - ABOUT_THE_CONTROL exempted every rule, so a real credential on a line that mentioned the policy scanned clean. The allowlist is now opt-in per rule (prose flag) and only the heuristic rules (internal-marker, private-repo-ops) honour it; credential/infra formats always block. - A global (?i) leaked onto the SCREAMING_CASE credential-name branch of private-repo-ops, so lowercase code talk (session_token) near a repo name blocked ordinary prose. Case-insensitivity is now scoped to the repo names and the English phrase alternatives only. - Both policy scripts require rg -P, but Ubuntu's apt ripgrep is built without PCRE2, which would turn the required check permanently red. Both jobs now install the upstream binary pinned + SHA-256-verified (same pattern as gitleaks), and body-policy.sh refuses a PCRE2-less rg up front with a legible error. Fixtures added for all three regressions; suite passes 26/26. Co-authored-by: Codesmith --- .github/workflows/public-repo-guard.yml | 36 ++++++++++++++-- scripts/public-repo-guard/body-policy.sh | 43 ++++++++++++++----- .../tests/body-policy.test.sh | 17 ++++++++ 3 files changed, 81 insertions(+), 15 deletions(-) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index bba2f67..e6be4db 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -85,8 +85,24 @@ 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 are rg -P (PCRE2), and Ubuntu's apt ripgrep is built + # WITHOUT PCRE2 — with it, every rule exits 2 and the required check goes + # permanently red. Install the upstream binary the same way as gitleaks: + # version-pinned and SHA-256-verified. Skipped when a PCRE2-capable rg is + # already on the image. + - name: Install ripgrep (PCRE2-capable, pinned + checksum-verified) + env: + RIPGREP_VERSION: "14.1.1" + RIPGREP_SHA256: "4cf9f2741e6c465ffdb7c26f38056a59e2a2544b51f7cc128ef28337eeae4d8e" + run: | + if command -v rg >/dev/null && rg --pcre2-version >/dev/null 2>&1; then 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 "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl/rg" + sudo install -m 0755 "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl/rg" /usr/local/bin/rg + rm -rf ripgrep.tar.gz "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl" + rg --pcre2-version - name: content policy (WAVE trade-secret / internal-leak gate) env: @@ -132,8 +148,20 @@ jobs: sparse-checkout: scripts/public-repo-guard sparse-checkout-cone-mode: false - - name: Install ripgrep - run: command -v rg >/dev/null || (sudo apt-get update -qq && sudo apt-get install -y -qq ripgrep) + # Same PCRE2 requirement as the tree job above — apt's ripgrep won't do. + - name: Install ripgrep (PCRE2-capable, pinned + checksum-verified) + env: + RIPGREP_VERSION: "14.1.1" + RIPGREP_SHA256: "4cf9f2741e6c465ffdb7c26f38056a59e2a2544b51f7cc128ef28337eeae4d8e" + run: | + if command -v rg >/dev/null && rg --pcre2-version >/dev/null 2>&1; then 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 "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl/rg" + sudo install -m 0755 "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl/rg" /usr/local/bin/rg + rm -rf ripgrep.tar.gz "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl" + 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 diff --git a/scripts/public-repo-guard/body-policy.sh b/scripts/public-repo-guard/body-policy.sh index a0b421f..aeacbd5 100755 --- a/scripts/public-repo-guard/body-policy.sh +++ b/scripts/public-repo-guard/body-policy.sh @@ -18,13 +18,18 @@ # Exit: 0 clean · 1 blocking violation · 2 scanner error (fail closed). # # Allowlisting: a line carrying `guard:allow ` is exempt (an accidental -# leak never carries the marker; a deliberate one is visible in a public diff), as -# is any line matching the ABOUT-THE-CONTROL allowlist below. +# leak never carries the marker; a deliberate one is visible in a public diff). +# The ABOUT-THE-CONTROL allowlist below additionally exempts the HEURISTIC prose +# rules only — a credential format is a leak no matter what else the line says. set -uo pipefail FILE="${1:-}" [[ -n "$FILE" && -f "$FILE" ]] || { echo "::error::body-policy: usage: body-policy.sh "; exit 2; } command -v rg >/dev/null 2>&1 || { echo "::error::body-policy: ripgrep (rg) required"; exit 2; } +# Every rule here is -P (PCRE2). Distro rg packages are sometimes built without +# it; that build fails every scan with exit 2, so refuse it up front with a +# message that says WHY instead of 21 opaque "ripgrep failed" annotations. +rg --pcre2-version >/dev/null 2>&1 || { echo "::error::body-policy: this ripgrep build lacks PCRE2 (-P) — install a PCRE2-capable rg"; exit 2; } VIOLATIONS=0 @@ -32,11 +37,19 @@ VIOLATIONS=0 # the gate blocks its own pull requests and every security discussion — the # self-referential trap that gets a gate switched off. Ported verbatim in intent # from the client-side gate's allowlist, which was built for exactly this. +# +# SCOPE: only rules that opt in via the `prose` flag honour this allowlist — the +# HEURISTIC rules, whose matches are ordinary words that genuinely occur when +# discussing the gate. The credential-format and infrastructure rules never +# honour it: a real key is a leak even on a line that names SECURITY.md, and +# exempting it there would make "mention the policy" a one-line bypass. ABOUT_THE_CONTROL='(public-repo-guard|body-policy|content-policy|public-github-write-gate|\bNDA\s+(gate|guard|policy|denylist|sweep|scan|hook)\b|\bno\s+NDA\b|responsib\w*\s+disclos|SECURITY\.md)' -# check +# check [prose] +# The optional 5th arg `prose` marks a HEURISTIC rule whose hits may also be +# legitimate discussion of the gate itself; only those honour ABOUT_THE_CONTROL. check() { - local sev="$1" name="$2" re="$3" why="$4" + local sev="$1" name="$2" re="$3" why="$4" scope="${5:-}" [[ -z "$re" ]] && { echo "::error::body-policy: internal bug — empty regex for rule '$name'"; exit 2; } # rg exit: 0=match, 1=no match, >=2=real error → FAIL CLOSED. A gate that passes # because its scanner broke is worse than no gate: it reports success. @@ -51,8 +64,10 @@ check() { # 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)" + | rg -vN -- 'guard:allow[[:space:]]+[^[:space:]]' || true)" + if [[ "$scope" == "prose" ]]; then + matches="$(printf '%s' "$matches" | rg -vNiP -- "$ABOUT_THE_CONTROL" || true)" + fi [[ -z "$matches" ]] && return 0 local count; count="$(printf '%s\n' "$matches" | grep -c '')" # Print the LINE NUMBER only — never the matched text. This annotation is itself @@ -97,7 +112,7 @@ check BLOCK abs-user-path '/(Users|home)/(?!runner/)[a-z][a-z0-9._-]+/' 'O # A quoted marker is also a trivial bypass, and that is an accepted trade. The # threat here is the ACCIDENTAL paste; a deliberate evader has easier routes, and # `guard:allow ` already exists as the honest, visible one. -check BLOCK internal-marker '(? Date: Thu, 6 Aug 2026 17:41:46 +0000 Subject: [PATCH 03/14] ci: scan review bodies, fix install list, use synthetic fixture repo names Co-authored-by: Codesmith --- .github/workflows/public-repo-guard.yml | 26 ++++++++++++++----- .../tests/body-policy.test.sh | 26 ++++++++++--------- 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index e6be4db..3de3628 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -13,11 +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 four 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. @@ -34,6 +35,13 @@ on: types: [opened, edited] issue_comment: types: [created, edited] + # Review summaries and review-thread comments are just as world-readable as the + # PR body, and `issue_comment` does NOT fire for them — only for top-level PR + # comments. Without these two triggers, review text went unscanned. + pull_request_review: + types: [submitted, edited] + pull_request_review_comment: + types: [created, edited] push: branches: [main, master] workflow_dispatch: @@ -117,8 +125,8 @@ jobs: 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 + # TREE; a PR/issue/comment/review 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. # @@ -128,7 +136,12 @@ jobs: # 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' + 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' concurrency: # Keyed on the specific PR / comment / issue rather than github.ref, because # issue events all report the default branch and a ref-keyed group would let @@ -174,12 +187,13 @@ jobs: # 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")' "$GITHUB_EVENT_PATH")" != "true" ]; then - echo "::error title=public-repo-guard (body-guard)::Event payload contains no pull_request/issue/comment object — refusing to report a pass on 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, + .review.body, .comment.body] | map(select(. != null)) | join("\n")' \ "$GITHUB_EVENT_PATH" > "$RUNNER_TEMP/bodyscan/body.txt" diff --git a/scripts/public-repo-guard/tests/body-policy.test.sh b/scripts/public-repo-guard/tests/body-policy.test.sh index dff36ad..f2d06c2 100755 --- a/scripts/public-repo-guard/tests/body-policy.test.sh +++ b/scripts/public-repo-guard/tests/body-policy.test.sh @@ -14,8 +14,10 @@ TMP="$(mktemp -d)" trap 'rm -rf "$TMP"' EXIT # The names the real gate is configured with come from an org variable; the tests -# pin their own so they are hermetic and do not depend on CI configuration. -export GUARD_PRIVATE_REPOS="wave-gateway, wave-transports, agent-money" +# pin their own so they are hermetic and do not depend on CI configuration. The +# names are deliberately SYNTHETIC: this file is public and exempt from both tree +# scanners, so a real private repo name written here would itself be a leak. +export GUARD_PRIVATE_REPOS="example-private-alpha, example-private-bravo, example-private-charlie" PASS=0; FAIL=0 @@ -39,13 +41,13 @@ echo "body-policy fixtures" # --- must BLOCK --------------------------------------------------------------- expect 1 'private repo + credential name' \ - 'Flip is live: WAVE_VIEWPORT_LEASE_SECRET is bound on wave-gateway now.' + 'Flip is live: WAVE_VIEWPORT_LEASE_SECRET is bound on example-private-alpha now.' expect 1 'private repo + credential name, reverse order' \ - 'The MOQ_JOIN_SECRET was added; wave-transports picks it up on deploy.' + 'The MOQ_JOIN_SECRET was added; example-private-bravo picks it up on deploy.' expect 1 'private repo + secret count' \ - 'wave-gateway went from 74 secrets to 75 after this change.' + 'example-private-alpha went from 74 secrets to 75 after this change.' expect 1 'private repo + service binding' \ - 'This adds a service binding from the worker to agent-money for settlement.' + 'This adds a service binding from the worker to example-private-charlie for settlement.' expect 1 'operator home path' \ 'Repro: run it from /Users/someoperator/Documents/notes and it fails.' # enforce-ignore (fixture) expect 1 'internal-only marker' \ @@ -69,9 +71,9 @@ expect 1 'private key on a line citing SECURITY.md' \ # --- must PASS (precision — these keep the gate deployable) ------------------- expect 0 'bare private-repo cross-reference' \ - 'This is the companion change to wave-transports#260; merge that one first.' + 'This is the companion change to example-private-bravo#260; merge that one first.' expect 0 'two private repos, no operational detail' \ - 'Both wave-gateway and wave-transports will need a follow-up for this.' + 'Both example-private-alpha and example-private-bravo will need a follow-up for this.' expect 0 'credential NAME with no private repo nearby' \ 'The handler now reads SOME_API_TOKEN from the environment instead of a literal.' # Regression: the SCREAMING_CASE credential-name branch is case-SENSITIVE. A @@ -79,17 +81,17 @@ expect 0 'credential NAME with no private repo nearby' \ # (`session_token`) blocked ordinary prose — exactly the false-positive class # that gets a gate switched off. expect 0 'lowercase code identifier near a private repo' \ - 'The wave-gateway worker reads session_token from the request header.' + 'The example-private-alpha worker reads session_token from the request header.' expect 1 'repo name cased differently still pairs with a credential NAME' \ - 'Wave-Gateway now requires LEASE_SECRET at deploy time.' + 'Example-Private-Alpha now requires LEASE_SECRET at deploy time.' expect 0 'talking about the gate with a repo name and credential NAME' \ - 'body-policy blocks wave-gateway next to a SECRET_TOKEN; that is intended.' + 'body-policy blocks example-private-alpha next to a SECRET_TOKEN; that is intended.' expect 0 'public runner path is not an operator path' \ 'CI checks out to /home/runner/work/repo/repo before the scan runs.' # enforce-ignore (fixture) expect 0 'talking about the control' \ 'body-policy blocks a private repo named next to a SECRET_TOKEN; that is intended.' expect 0 'explicit guard:allow with a reason' \ - 'Example for the docs: wave-gateway holds EXAMPLE_SECRET — guard:allow documented-example' + 'Example for the docs: example-private-alpha holds EXAMPLE_SECRET — guard:allow documented-example' expect 0 'ordinary clean body' \ 'Bumps the draft revision and regenerates the fixtures. No behaviour change.' # Regression: the first CI run of this job failed on its own PR, because a review From 45fa1b4486237dd1f4fcf4bf1af678cbe66349f7 Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 17:50:01 +0000 Subject: [PATCH 04/14] fix(ci): run the tree scan on PR edits so a skipped check can't supersede a failing one Co-authored-by: Codesmith --- .github/workflows/public-repo-guard.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index 3de3628..04ca22f 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -60,10 +60,14 @@ jobs: guard: name: Secrets + content policy # Skips issue/comment events (the tree scan has nothing to say about a comment, - # and the org should not pay for a gitleaks run every time anyone posts one) and - # skips `edited` (a title or body edit does not change the tree). + # and the org should not pay for a gitleaks run every time anyone posts one). + # Deliberately does NOT skip `edited`: a title/body edit doesn't change the + # tree, but a skipped job still publishes a check run for the same head SHA, + # and GitHub treats skipped as passing and evaluates the MOST RECENT check run + # per name — so skipping here would let a description edit supersede a failing + # tree scan with a green rubber stamp. Re-running just reproduces the verdict. if: >- - (github.event_name == 'pull_request' && github.event.action != 'edited') + github.event_name == 'pull_request' || github.event_name == 'push' || github.event_name == 'workflow_dispatch' concurrency: From 9bac94c4af1bb3d0925b30ce492c896de147df46 Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 17:56:35 +0000 Subject: [PATCH 05/14] test: split private-key fixture literal so the foundation-gate secret scan cannot match it Co-authored-by: Codesmith --- scripts/public-repo-guard/tests/body-policy.test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/public-repo-guard/tests/body-policy.test.sh b/scripts/public-repo-guard/tests/body-policy.test.sh index f2d06c2..cf3ea79 100755 --- a/scripts/public-repo-guard/tests/body-policy.test.sh +++ b/scripts/public-repo-guard/tests/body-policy.test.sh @@ -67,7 +67,7 @@ expect 1 'internal tailscale IP' \ expect 1 'credential on a line that mentions the policy' \ "Discussing public-repo-guard: the key ${AKID_FIXTURE} was rotated." expect 1 'private key on a line citing SECURITY.md' \ - 'Per SECURITY.md: -----BEGIN RSA PRIVATE KEY-----' + "Per SECURITY.md: -----BEGIN RSA ""PRIVATE KEY-----" # pragma: allowlist secret (fixture, split like AKID above) # --- must PASS (precision — these keep the gate deployable) ------------------- expect 0 'bare private-repo cross-reference' \ From ee35c9e4eed21c5294fe1eef8528dae9927549ad Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 18:03:06 +0000 Subject: [PATCH 06/14] fix: drop the stray word boundary so compound credential names match after the repo name Co-authored-by: Codesmith --- scripts/public-repo-guard/body-policy.sh | 7 ++++++- scripts/public-repo-guard/tests/body-policy.test.sh | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/scripts/public-repo-guard/body-policy.sh b/scripts/public-repo-guard/body-policy.sh index aeacbd5..645d8ef 100755 --- a/scripts/public-repo-guard/body-policy.sh +++ b/scripts/public-repo-guard/body-policy.sh @@ -147,8 +147,13 @@ if [[ -n "${GUARD_PRIVATE_REPOS:-}" ]]; then if [[ -n "$_ALT" ]]; then # Both orders: name-then-detail and detail-then-name. Case-insensitivity is # scoped to the repo NAMES only — see the OPS_DETAIL comment above. + # No \b in front of OPS_DETAIL in either branch: for a multi-segment name + # like WAVE_VIEWPORT_LEASE_SECRET the credential alternative can only match + # the trailing LEASE_SECRET, and that position is NOT a word boundary + # (underscore is a word character) — a \b there silently drops every + # compound credential name from the name-first order. check BLOCK private-repo-ops \ - "\\b(?i:${_ALT})\\b[^\\n]{0,140}?\\b${OPS_DETAIL}|${OPS_DETAIL}[^\\n]{0,140}?\\b(?i:${_ALT})\\b" \ + "\\b(?i:${_ALT})\\b[^\\n]{0,140}?${OPS_DETAIL}|${OPS_DETAIL}[^\\n]{0,140}?\\b(?i:${_ALT})\\b" \ 'A private WAVE repo named alongside internal operational detail (credential name, secret binding, or secret count) — the wiring topology is not public' prose fi fi diff --git a/scripts/public-repo-guard/tests/body-policy.test.sh b/scripts/public-repo-guard/tests/body-policy.test.sh index cf3ea79..ccd116e 100755 --- a/scripts/public-repo-guard/tests/body-policy.test.sh +++ b/scripts/public-repo-guard/tests/body-policy.test.sh @@ -44,6 +44,11 @@ expect 1 'private repo + credential name' \ 'Flip is live: WAVE_VIEWPORT_LEASE_SECRET is bound on example-private-alpha now.' expect 1 'private repo + credential name, reverse order' \ 'The MOQ_JOIN_SECRET was added; example-private-bravo picks it up on deploy.' +# Regression: a MULTI-segment credential name after the repo name. The rule can +# only match the trailing LEASE_SECRET, which sits mid-word (after an +# underscore) — a \b in front of OPS_DETAIL silently dropped this whole order. +expect 1 'private repo, then compound credential name' \ + 'example-private-alpha now uses WAVE_VIEWPORT_LEASE_SECRET for renewals.' expect 1 'private repo + secret count' \ 'example-private-alpha went from 74 secrets to 75 after this change.' expect 1 'private repo + service binding' \ From e3a4265af7a36593dc77a1060e014663e99291c8 Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 18:09:24 +0000 Subject: [PATCH 07/14] docs: state precisely what per-job concurrency does and does not fix for the tree job Co-authored-by: Codesmith --- .github/workflows/public-repo-guard.yml | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index 04ca22f..1b2b5af 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -52,9 +52,14 @@ 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. +# A workflow-level group would force one policy on both, and it showed: comment and +# issue events (which the tree job skips entirely) cancelled in-flight tree scans, +# so a chatty review thread could leave a head SHA with NO completed tree verdict. +# Per-job groups end that: only same-PR pull_request events enter the tree group. +# Within that group, a body edit or new push still supersedes an in-flight tree run +# — deliberately. The replacement scans the same (or newer) head SHA and always +# lands a completed verdict, and GitHub evaluates the most recent check run per +# name, so the superseded run's cancelled record is cosmetic, not load-bearing. jobs: guard: From 5d9db11cc9a0acf11076901e3e01978debc38433 Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 18:15:41 +0000 Subject: [PATCH 08/14] fix: key the body-guard group on the comment/review id first so batched review comments are all scanned Co-authored-by: Codesmith --- .github/workflows/public-repo-guard.yml | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index 1b2b5af..94f62de 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -152,14 +152,21 @@ jobs: || github.event_name == 'pull_request_review' || github.event_name == 'pull_request_review_comment' concurrency: - # Keyed on the specific PR / comment / 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. + # Keyed on the MOST SPECIFIC object in the payload, and the comment/review id + # must come FIRST: `pull_request_review` and `pull_request_review_comment` + # payloads carry a top-level `pull_request` object, so a PR-number-first chain + # would short-circuit and file every review comment on a PR under ONE group. + # GitHub keeps at most one PENDING run per group, so a review submitted with + # several inline comments would silently drop the middle ones — distinct + # comments are distinct world-readable texts, and each needs its own verdict. + # A ref-keyed group has the same flaw (issue events all report the default + # branch). Only same-object versions may share a group: the newest scan of an + # edited body covers what is public now. # # 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.pull_request.number || github.event.comment.id || github.event.issue.number || github.ref }} + 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: From 7e086ee900f51235e2fcebfbadaf087b1966b030 Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 18:27:21 +0000 Subject: [PATCH 09/14] fix(ci): close two body-guard pass-by-default paths Review events (pull_request_review, pull_request_review_comment) run in the PR's context and publish a check run on the PR head SHA; with the guard job skipping them, that run lands as 'skipped', which GitHub treats as passing while evaluating the most recent check run per name, so any review comment could supersede a failing tree scan with a green rubber stamp. The guard job now re-runs the tree scan on those events, same as pull_request: edited. body-policy.sh now fails closed (exit 2) in CI when GUARD_PRIVATE_REPOS is empty or contains no names: a missing or renamed org variable must go red, not silently skip the private-repo proximity rule and report a pass over an unscanned leak class. Local runs still skip the rule, and both behaviours are pinned by new fixtures. Co-authored-by: Codesmith --- .github/workflows/public-repo-guard.yml | 18 ++++++---- scripts/public-repo-guard/body-policy.sh | 34 +++++++++++-------- .../tests/body-policy.test.sh | 19 +++++++++++ 3 files changed, 51 insertions(+), 20 deletions(-) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index 94f62de..a4aeb05 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -55,7 +55,8 @@ permissions: # A workflow-level group would force one policy on both, and it showed: comment and # issue events (which the tree job skips entirely) cancelled in-flight tree scans, # so a chatty review thread could leave a head SHA with NO completed tree verdict. -# Per-job groups end that: only same-PR pull_request events enter the tree group. +# Per-job groups end that: only same-PR pull_request and review events enter the +# tree group. # Within that group, a body edit or new push still supersedes an in-flight tree run # — deliberately. The replacement scans the same (or newer) head SHA and always # lands a completed verdict, and GitHub evaluates the most recent check run per @@ -66,13 +67,18 @@ jobs: name: Secrets + content policy # Skips issue/comment events (the tree scan has nothing to say about a comment, # and the org should not pay for a gitleaks run every time anyone posts one). - # Deliberately does NOT skip `edited`: a title/body edit doesn't change the - # tree, but a skipped job still publishes a check run for the same head SHA, - # and GitHub treats skipped as passing and evaluates the MOST RECENT check run - # per name — so skipping here would let a description edit supersede a failing - # tree scan with a green rubber stamp. Re-running just reproduces the verdict. + # That is safe ONLY because those runs report the DEFAULT branch, not the PR + # head. Review events are different: they run in the PR's context and publish + # a check run on the PR HEAD SHA, and a job skipped by `if` still publishes + # one with conclusion `skipped`, which GitHub treats as passing while + # evaluating the MOST RECENT check run per name. Skipping them would let any + # review comment supersede a failing tree scan with a green rubber stamp, so + # they re-run the tree scan instead (same reason `edited` is not skipped: + # re-running just reproduces the verdict). if: >- github.event_name == 'pull_request' + || github.event_name == 'pull_request_review' + || github.event_name == 'pull_request_review_comment' || github.event_name == 'push' || github.event_name == 'workflow_dispatch' concurrency: diff --git a/scripts/public-repo-guard/body-policy.sh b/scripts/public-repo-guard/body-policy.sh index 645d8ef..282c535 100755 --- a/scripts/public-repo-guard/body-policy.sh +++ b/scripts/public-repo-guard/body-policy.sh @@ -128,7 +128,11 @@ check BLOCK internal-marker '(? 0 )); then diff --git a/scripts/public-repo-guard/tests/body-policy.test.sh b/scripts/public-repo-guard/tests/body-policy.test.sh index ccd116e..f76e8f7 100755 --- a/scripts/public-repo-guard/tests/body-policy.test.sh +++ b/scripts/public-repo-guard/tests/body-policy.test.sh @@ -125,6 +125,25 @@ for case in "no argument at all::" "nonexistent path::$TMP/does-not-exist.txt"; fi done +# An empty GUARD_PRIVATE_REPOS is a documented local convenience but FAILS CLOSED +# in CI (GITHUB_ACTIONS set): a missing or renamed org variable must go red, never +# silently skip the private-repo rule and report a pass over an unscanned class. +printf '%s\n' 'Ordinary clean body with nothing to find.' > "$TMP/body.txt" +env -u GUARD_PRIVATE_REPOS GITHUB_ACTIONS=true bash "$SCRIPT" "$TMP/body.txt" >/dev/null 2>&1 +rc=$? +if [[ "$rc" == 2 ]]; then + PASS=$((PASS+1)); printf ' ok %s → exit 2 (fails closed)\n' 'GUARD_PRIVATE_REPOS unset in CI' +else + FAIL=$((FAIL+1)); printf ' FAIL %s: want exit 2, got %s\n' 'GUARD_PRIVATE_REPOS unset in CI' "$rc" +fi +env -u GUARD_PRIVATE_REPOS -u GITHUB_ACTIONS bash "$SCRIPT" "$TMP/body.txt" >/dev/null 2>&1 +rc=$? +if [[ "$rc" == 0 ]]; then + PASS=$((PASS+1)); printf ' ok %s → exit 0 (rule skipped)\n' 'GUARD_PRIVATE_REPOS unset locally' +else + FAIL=$((FAIL+1)); printf ' FAIL %s: want exit 0, got %s\n' 'GUARD_PRIVATE_REPOS unset locally' "$rc" +fi + echo " ---" if (( FAIL > 0 )); then echo " $PASS passed, $FAIL FAILED"; exit 1 From f2f50d7e566270aa8fd3a83fe915fe11c17e6c63 Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 18:36:27 +0000 Subject: [PATCH 10/14] ci: skip tree scan for review events on closed PRs Co-authored-by: Codesmith --- .github/workflows/public-repo-guard.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index a4aeb05..e7d29af 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -75,10 +75,15 @@ jobs: # review comment supersede a failing tree scan with a green rubber stamp, so # they re-run the tree scan instead (same reason `edited` is not skipped: # re-running just reproduces the verdict). + # Review events on a CLOSED PR are the one exception: nothing gates a closed + # PR, and refs/pull/N/merge goes stale after close, so checking it out can + # fail and paint a spurious red on the required check. A skipped run is + # harmless exactly there: a rubber stamp with nothing left to stamp. if: >- github.event_name == 'pull_request' - || github.event_name == 'pull_request_review' - || github.event_name == 'pull_request_review_comment' + || ((github.event_name == 'pull_request_review' + || github.event_name == 'pull_request_review_comment') + && github.event.pull_request.state == 'open') || github.event_name == 'push' || github.event_name == 'workflow_dispatch' concurrency: From c284c89bc145caec2b206696d696411926f9f0fc Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 18:42:45 +0000 Subject: [PATCH 11/14] fix(ci): skip body scan for review events on closed PRs Co-authored-by: Codesmith --- .github/workflows/public-repo-guard.yml | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index e7d29af..5d5ab59 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -156,12 +156,21 @@ jobs: # can stop that class before publication. body-guard: name: Body content policy + # Review events on a CLOSED PR are excluded for the same reason as in `guard` + # above: refs/pull/N/merge goes stale after close, so the checkout can fail + # and paint a spurious red on the PR. The cost is a detection gap — a review + # comment on a closed PR goes unscanned server-side — accepted because a + # guaranteed-flaky red is worse than no verdict, and the pre-write hook is + # the control that stops that class before publication anyway. (Comments on + # closed PRs still arrive as `issue_comment` and ARE scanned; only review + # summaries and review-thread comments are affected.) 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' + || ((github.event_name == 'pull_request_review' + || github.event_name == 'pull_request_review_comment') + && github.event.pull_request.state == 'open') concurrency: # Keyed on the MOST SPECIFIC object in the payload, and the comment/review id # must come FIRST: `pull_request_review` and `pull_request_review_comment` From d16a745eadb334d6b1916859aac5f9ed09f7ce98 Mon Sep 17 00:00:00 2001 From: Jake Fineman Date: Sun, 6 Sep 2026 18:45:15 -0400 Subject: [PATCH 12/14] fix(ci): pin body-guard checkout to a trusted ref Addresses a review finding (qodo-code-review, PR#27): the body-guard job checked out the tree with no ref, defaulting to refs/pull/N/merge on pull_request events -- the PRs OWN tree. A malicious PR could edit scripts/public-repo-guard/body-policy.sh in the same PR to always pass, defeating the body-leak gate. Pin to the PR base sha (untouched by the PR) when one exists, else the default branch (issues/ issue_comment already run in the default-branch context). --- .github/workflows/public-repo-guard.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index 5d5ab59..df1286b 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -192,6 +192,16 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: + # Trusted-ref pin (not the default `github.ref`): for `pull_request` / + # `pull_request_review` / `pull_request_review_comment` events the + # unqualified ref is refs/pull/N/merge -- the PR's OWN tree -- so a + # malicious PR could edit body-policy.sh in the same PR to always + # pass and defeat this gate. Pin to the PR's base sha (untouched by + # the PR) when one exists; otherwise (issues/issue_comment, which + # already run in the default-branch context) fall back to the + # default branch, which is equally trusted. + ref: ${{ github.event.pull_request.base.sha || github.event.repository.default_branch }} + persist-credentials: false # 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 From 326844ac2c7f6f1675e7850156770708f420aacc Mon Sep 17 00:00:00 2001 From: Jake Fineman Date: Sun, 6 Sep 2026 18:56:18 -0400 Subject: [PATCH 13/14] fix(ci): bootstrap fallback for the trusted-ref pin on body-guard The previous commit pinned body-guards checkout to the PR base sha, but THIS PR is what first adds scripts/public-repo-guard/body-policy.sh -- base (main, pre-merge) does not have it yet, so the job 404d on its own script (observed: two red "Body content policy" runs on this PR after the pin landed). Add a second, unpinned checkout that only runs when hashFiles finds the script missing at the trusted ref -- true only for this bootstrap PR, never for a later PR trying to tamper with an already-merged script. --- .github/workflows/public-repo-guard.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index df1286b..6627bfe 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -207,6 +207,19 @@ jobs: sparse-checkout: scripts/public-repo-guard sparse-checkout-cone-mode: false + # Bootstrap exception, safe by construction: this only fires when the TRUSTED + # ref genuinely lacks the script (true only for the one PR that first adds + # this gate -- an attacker cannot make a real base ref lose a file that is + # already merged to it). Once this PR merges, every later PR's base carries + # the script and this step is a no-op forever after. + - name: Bootstrap fallback for the PR that first introduces this gate + if: ${{ !hashFiles('scripts/public-repo-guard/body-policy.sh') }} + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + sparse-checkout: scripts/public-repo-guard + sparse-checkout-cone-mode: false + # Same PCRE2 requirement as the tree job above — apt's ripgrep won't do. - name: Install ripgrep (PCRE2-capable, pinned + checksum-verified) env: From 882e9656fed2edded0094a3cacd59e2b4cfc9b53 Mon Sep 17 00:00:00 2001 From: Jake Fineman Date: Tue, 8 Sep 2026 14:27:38 -0400 Subject: [PATCH 14/14] ci(guard): adopt the landed two-file body-scan generation, and drop the bootstrap deadlock This branch introduced the body scanner as a second job inside public-repo-guard.yml, pinned to a trusted ref and refusing to execute the PR own copy. That refusal is a bootstrap deadlock: the PR that introduces the scanner can never satisfy a check that will only run a copy already merged to the base. A later generation of exactly this change has since landed on main in two sibling public repos; this commit ports that shape rather than inventing a third variant. Three differences from what this branch had: 1. File-level split. The body scan moves to its own workflow file, .github/workflows/public-repo-guard-body.yml. public-repo-guard.yml now triggers only on tree-changing events, so a comment or review event no longer publishes a skipped check-run under the REQUIRED tree-scan name. Ruleset evaluation reads the newest run of a name and treats skipped as passing, so a chatty thread could mask a failed or never-completed tree verdict. A job-level if did not close that; removing the trigger does. 2. No trusted-copy dance. The body job runs the repo checked-out copy directly. It triggers on pull_request, never pull_request_target, so a fork PR gets no write token and no repo secrets, and the untrusted title and body are read out of the event payload file with jq into another file, never interpolated into a run block and never passed through an environment variable. The deadlock disappears because the gate no longer needs a pre-merged copy to be safe. 3. ripgrep pinned and SHA-256 verified in both jobs. The rules are rg -P and the apt package is built without PCRE2, which would fail every rule with exit 2 rather than a verdict. Two hardenings kept that only one landed copy carried, so this port is not weaker than either parent: - GUARD_PRIVATE_REPOS is normalised for newlines and carriage returns before splitting. read stops at the first newline, so a newline-separated value configured only the first name and reported a pass over the rest, and a CRLF value glued an invisible carriage return to every name so the built pattern matched nothing and the rule failed open silently. - An empty GUARD_PRIVATE_REPOS fails CLOSED in CI (exit 2) instead of warning. A missing or renamed variable means the flagship rule scanned nothing while the job reports green, which is the rubber stamp every other stage in this script refuses. Local runs still skip it. Both carry fixture regressions. The suite runs from the tree job and is 45 green locally, including the fail-closed cases for a broken filter stage, a missing argument, and an unconfigured variable. Fixture repo names stay synthetic: this file is public. Co-Authored-By: Claude Opus 5 --- .github/workflows/public-repo-guard-body.yml | 122 +++++++++ .github/workflows/public-repo-guard.yml | 236 +++++------------- scripts/public-repo-guard/body-policy.sh | 149 +++++++---- .../tests/body-policy.test.sh | 158 +++++++++--- 4 files changed, 402 insertions(+), 263 deletions(-) create mode 100644 .github/workflows/public-repo-guard-body.yml diff --git a/.github/workflows/public-repo-guard-body.yml b/.github/workflows/public-repo-guard-body.yml new file mode 100644 index 0000000..060e94d --- /dev/null +++ b/.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" diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index 6627bfe..bd3d9a7 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.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,10 @@ 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 (the guard job runs the +# fixture tests, so a repo missing the tests file fails on every run): # .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,73 +28,70 @@ name: public-repo-guard # # Allowlisting: annotate a verified-safe line with `# guard:allow `, 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 sharing one `on:` block): +# +# 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 +# (public-repo-guard-required). Before this split, that job's shared `on:` block +# had to include pull_request_review / pull_request_review_comment (needed only by +# the sibling body scan), and the job used a job-level `if:` to skip those events +# for the tree scan (a title/comment/review event cannot change the tree). GitHub +# still publishes a check-run named "Secrets + content policy" with conclusion +# `skipped` for every skipped event, on the same head SHA. Branch-protection/ +# ruleset required-status-check evaluation treats `skipped` as passing and reads +# only the NEWEST check-run of a given name — so a review comment (or any other +# skipped event) could flip an already-failed, or never-yet-completed, required +# tree scan to green with nothing re-examining the tree. Observed and confirmed on +# a sibling public repo before this shape was adopted here: wave-av/mcp-server +# PR 87 (merged) — the tree job's only non-skipped run on that head SHA was +# `cancelled`, followed by a dozen `skipped` runs, and the required check's final +# state read `skipped` (passing) despite no completed real verdict ever having +# been produced for that SHA. This file is the same fix, ported. +# +# 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 published tree (pull_request open/reopen/sync, push, workflow_dispatch, +# merge_group). 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. Coverage is unchanged: every event that +# could previously produce a real (non-skipped) tree-scan run still produces that +# same real run after the split. 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] - # Review summaries and review-thread comments are just as world-readable as the - # PR body, and `issue_comment` does NOT fire for them — only for top-level PR - # comments. Without these two triggers, review text went unscanned. - pull_request_review: - types: [submitted, edited] - pull_request_review_comment: - types: [created, edited] + types: [opened, reopened, synchronize] push: branches: [main, master] workflow_dispatch: + # Required by the merge queue: a `merge_group` build never runs the `pull_request` + # trigger above, so without this the required "Secrets + content policy" check + # never reports on the queue's temporary ref and every queued PR waits forever. + merge_group: # `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 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: comment and -# issue events (which the tree job skips entirely) cancelled in-flight tree scans, -# so a chatty review thread could leave a head SHA with NO completed tree verdict. -# Per-job groups end that: only same-PR pull_request and review events enter the -# tree group. -# Within that group, a body edit or new push still supersedes an in-flight tree run -# — deliberately. The replacement scans the same (or newer) head SHA and always -# lands a completed verdict, and GitHub evaluates the most recent check run per -# name, so the superseded run's cancelled record is cosmetic, not load-bearing. - jobs: guard: name: Secrets + content policy - # Skips issue/comment events (the tree scan has nothing to say about a comment, - # and the org should not pay for a gitleaks run every time anyone posts one). - # That is safe ONLY because those runs report the DEFAULT branch, not the PR - # head. Review events are different: they run in the PR's context and publish - # a check run on the PR HEAD SHA, and a job skipped by `if` still publishes - # one with conclusion `skipped`, which GitHub treats as passing while - # evaluating the MOST RECENT check run per name. Skipping them would let any - # review comment supersede a failing tree scan with a green rubber stamp, so - # they re-run the tree scan instead (same reason `edited` is not skipped: - # re-running just reproduces the verdict). - # Review events on a CLOSED PR are the one exception: nothing gates a closed - # PR, and refs/pull/N/merge goes stale after close, so checking it out can - # fail and paint a spurious red on the required check. A skipped run is - # harmless exactly there: a rubber stamp with nothing left to stamp. - if: >- - github.event_name == 'pull_request' - || ((github.event_name == 'pull_request_review' - || github.event_name == 'pull_request_review_comment') - && github.event.pull_request.state == 'open') - || github.event_name == 'push' - || github.event_name == 'workflow_dispatch' + # 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: group: public-repo-guard-tree-${{ github.event.pull_request.number || github.ref }} cancel-in-progress: true runs-on: ubuntu-latest steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + # Nothing in this job pushes or calls the API, so the token has no + # business lingering in .git/config while repo-checked-out scripts run. + 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 @@ -114,22 +114,25 @@ jobs: run: gitleaks detect --no-git --source . --config .gitleaks.toml --redact --no-banner --exit-code 1 # Both policy scripts are rg -P (PCRE2), and Ubuntu's apt ripgrep is built - # WITHOUT PCRE2 — with it, every rule exits 2 and the required check goes - # permanently red. Install the upstream binary the same way as gitleaks: - # version-pinned and SHA-256-verified. Skipped when a PCRE2-capable rg is - # already on the image. - - name: Install ripgrep (PCRE2-capable, pinned + checksum-verified) + # WITHOUT it — with that build every rule exits 2 and this required check + # goes permanently red. Install the upstream binary the same way as gitleaks + # above: version-pinned AND SHA-256-verified before it runs. Skipped when the + # runner image already carries a PCRE2-capable rg (probed, not assumed). + - name: Install ripgrep (PCRE2 build, pinned + checksum-verified) env: RIPGREP_VERSION: "14.1.1" RIPGREP_SHA256: "4cf9f2741e6c465ffdb7c26f38056a59e2a2544b51f7cc128ef28337eeae4d8e" run: | - if command -v rg >/dev/null && rg --pcre2-version >/dev/null 2>&1; then exit 0; fi + set -euo pipefail + 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 "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl/rg" - sudo install -m 0755 "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl/rg" /usr/local/bin/rg - rm -rf ripgrep.tar.gz "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl" + 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) @@ -143,122 +146,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/review 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 - # Review events on a CLOSED PR are excluded for the same reason as in `guard` - # above: refs/pull/N/merge goes stale after close, so the checkout can fail - # and paint a spurious red on the PR. The cost is a detection gap — a review - # comment on a closed PR goes unscanned server-side — accepted because a - # guaranteed-flaky red is worse than no verdict, and the pre-write hook is - # the control that stops that class before publication anyway. (Comments on - # closed PRs still arrive as `issue_comment` and ARE scanned; only review - # summaries and review-thread comments are affected.) - 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') - && github.event.pull_request.state == 'open') - concurrency: - # Keyed on the MOST SPECIFIC object in the payload, and the comment/review id - # must come FIRST: `pull_request_review` and `pull_request_review_comment` - # payloads carry a top-level `pull_request` object, so a PR-number-first chain - # would short-circuit and file every review comment on a PR under ONE group. - # GitHub keeps at most one PENDING run per group, so a review submitted with - # several inline comments would silently drop the middle ones — distinct - # comments are distinct world-readable texts, and each needs its own verdict. - # A ref-keyed group has the same flaw (issue events all report the default - # branch). Only same-object versions may share a group: the newest scan of an - # edited body covers what is public now. - # - # 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: - # Trusted-ref pin (not the default `github.ref`): for `pull_request` / - # `pull_request_review` / `pull_request_review_comment` events the - # unqualified ref is refs/pull/N/merge -- the PR's OWN tree -- so a - # malicious PR could edit body-policy.sh in the same PR to always - # pass and defeat this gate. Pin to the PR's base sha (untouched by - # the PR) when one exists; otherwise (issues/issue_comment, which - # already run in the default-branch context) fall back to the - # default branch, which is equally trusted. - ref: ${{ github.event.pull_request.base.sha || github.event.repository.default_branch }} - persist-credentials: false - # 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 - - # Bootstrap exception, safe by construction: this only fires when the TRUSTED - # ref genuinely lacks the script (true only for the one PR that first adds - # this gate -- an attacker cannot make a real base ref lose a file that is - # already merged to it). Once this PR merges, every later PR's base carries - # the script and this step is a no-op forever after. - - name: Bootstrap fallback for the PR that first introduces this gate - if: ${{ !hashFiles('scripts/public-repo-guard/body-policy.sh') }} - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - persist-credentials: false - sparse-checkout: scripts/public-repo-guard - sparse-checkout-cone-mode: false - - # Same PCRE2 requirement as the tree job above — apt's ripgrep won't do. - - name: Install ripgrep (PCRE2-capable, pinned + checksum-verified) - env: - RIPGREP_VERSION: "14.1.1" - RIPGREP_SHA256: "4cf9f2741e6c465ffdb7c26f38056a59e2a2544b51f7cc128ef28337eeae4d8e" - run: | - if command -v rg >/dev/null && rg --pcre2-version >/dev/null 2>&1; then 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 "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl/rg" - sudo install -m 0755 "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl/rg" /usr/local/bin/rg - rm -rf ripgrep.tar.gz "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl" - 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, - .review.body, - .comment.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" diff --git a/scripts/public-repo-guard/body-policy.sh b/scripts/public-repo-guard/body-policy.sh index 282c535..366b4d6 100755 --- a/scripts/public-repo-guard/body-policy.sh +++ b/scripts/public-repo-guard/body-policy.sh @@ -19,17 +19,14 @@ # # Allowlisting: a line carrying `guard:allow ` is exempt (an accidental # leak never carries the marker; a deliberate one is visible in a public diff). -# The ABOUT-THE-CONTROL allowlist below additionally exempts the HEURISTIC prose -# rules only — a credential format is a leak no matter what else the line says. +# Prose-shaped rules (tagged `prose` below) are additionally exempt on lines +# matching the ABOUT-THE-CONTROL allowlist; credential and infrastructure rules +# are NOT — a real key is a leak no matter what else shares its line. set -uo pipefail FILE="${1:-}" [[ -n "$FILE" && -f "$FILE" ]] || { echo "::error::body-policy: usage: body-policy.sh "; exit 2; } command -v rg >/dev/null 2>&1 || { echo "::error::body-policy: ripgrep (rg) required"; exit 2; } -# Every rule here is -P (PCRE2). Distro rg packages are sometimes built without -# it; that build fails every scan with exit 2, so refuse it up front with a -# message that says WHY instead of 21 opaque "ripgrep failed" annotations. -rg --pcre2-version >/dev/null 2>&1 || { echo "::error::body-policy: this ripgrep build lacks PCRE2 (-P) — install a PCRE2-capable rg"; exit 2; } VIOLATIONS=0 @@ -38,16 +35,18 @@ VIOLATIONS=0 # self-referential trap that gets a gate switched off. Ported verbatim in intent # from the client-side gate's allowlist, which was built for exactly this. # -# SCOPE: only rules that opt in via the `prose` flag honour this allowlist — the -# HEURISTIC rules, whose matches are ordinary words that genuinely occur when -# discussing the gate. The credential-format and infrastructure rules never -# honour it: a real key is a leak even on a line that names SECURITY.md, and -# exempting it there would make "mention the policy" a one-line bypass. +# Scope: consulted ONLY by rules tagged `prose` below — the ones that fire on the +# LANGUAGE of a sentence and therefore misfire on sentences about the gate. A +# credential or infrastructure identifier is a leak regardless of what else shares +# its line; naming the gate next to a live key must not launder the key, so those +# rules never see this allowlist and `guard:allow ` is their only +# (visible) escape hatch. ABOUT_THE_CONTROL='(public-repo-guard|body-policy|content-policy|public-github-write-gate|\bNDA\s+(gate|guard|policy|denylist|sweep|scan|hook)\b|\bno\s+NDA\b|responsib\w*\s+disclos|SECURITY\.md)' # check [prose] -# The optional 5th arg `prose` marks a HEURISTIC rule whose hits may also be -# legitimate discussion of the gate itself; only those honour ABOUT_THE_CONTROL. +# `prose` opts the rule into the ABOUT_THE_CONTROL allowlist above. Omit it for +# credential/infrastructure rules so a same-line mention of the gate can never +# suppress a real leak. check() { local sev="$1" name="$2" re="$3" why="$4" scope="${5:-}" [[ -z "$re" ]] && { echo "::error::body-policy: internal bug — empty regex for rule '$name'"; exit 2; } @@ -62,11 +61,24 @@ check() { # 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. + # + # The filters fail CLOSED exactly like the main scan: exit 1 only means every + # hit was filtered away (fine), but exit >= 2 is a broken filter, and a broken + # filter that empties the match list would convert detected leaks into a + # silent pass. That is why there is no `|| true` here. local matches matches="$(printf '%s' "$raw" \ - | rg -vN -- 'guard:allow[[:space:]]+[^[:space:]]' || true)" - if [[ "$scope" == "prose" ]]; then - matches="$(printf '%s' "$matches" | rg -vNiP -- "$ABOUT_THE_CONTROL" || true)" + | rg -vN -- 'guard:allow[[:space:]]+[^[:space:]]')"; rc=$? + if (( rc >= 2 )); then + echo "::error title=public-repo-guard ($name)::ripgrep failed (exit $rc) in the guard:allow filter for rule '$name'. Failing closed." + exit 2 + fi + if [[ "$scope" == "prose" && -n "$matches" ]]; then + matches="$(printf '%s' "$matches" | rg -vNiP -- "$ABOUT_THE_CONTROL")"; rc=$? + if (( rc >= 2 )); then + echo "::error title=public-repo-guard ($name)::ripgrep failed (exit $rc) in the ABOUT_THE_CONTROL filter for rule '$name'. Failing closed." + exit 2 + fi fi [[ -z "$matches" ]] && return 0 local count; count="$(printf '%s\n' "$matches" | grep -c '')" @@ -95,9 +107,27 @@ check BLOCK private-key '-----BEGIN [A-Z ]*PRIVATE KEY-----' 'Em # --- Infrastructure identifiers ---------------------------------------------- # shellcheck disable=SC2016 # $CLOUDFLARE_ACCOUNT_ID is literal guidance text check BLOCK cf-account-id 'account_id\s*[:=]\s*["'"'"']?[0-9a-f]{32}' 'Hardcoded Cloudflare account_id — reference the env var instead' -check BLOCK internal-ip '100\.(6[4-9]|[7-9][0-9]|1[01][0-9]|12[0-7])\.[0-9]{1,3}\.[0-9]{1,3}' 'Internal Tailscale-CGNAT IP (100.64.0.0/10) — internal fleet address' +# The BODY profile diverges from the FILE gate here too. The tree gate excludes +# the guard's own directory from scanning, so its copy of this rule never sees +# the range literal in its own comments; body text has no such exclusion, and +# security discussion names the range's documentation form constantly (including +# quoting this very rule). Two shapes are RANGE-talk, not a fleet address, and +# are exempted: an all-zero host portion (100.64.0.0) and a CIDR-suffixed subnet +# (100.64.0.0/10, 100.71.4.0/24). A concrete host like 100.71.4.19 still blocks. +# Trade accepted: a live address written with a /32 suffix no longer fires. +check BLOCK internal-ip '100\.(6[4-9]|[7-9][0-9]|1[01][0-9]|12[0-7])\.(?!0\.0(?![0-9]))[0-9]{1,3}\.[0-9]{1,3}(?![0-9]|/[0-9])' 'Internal Tailscale-CGNAT IP (100.64.0.0/10) — internal fleet address' # shellcheck disable=SC2016 # $HOME is literal guidance text -check BLOCK abs-user-path '/(Users|home)/(?!runner/)[a-z][a-z0-9._-]+/' 'Operator absolute home path — leaks identity and local layout' +# The BODY profile diverges from the FILE gate here for the same reason as +# private-repo-ops below: body text is prose, and prose contains app routes. +# "/home//" is an ordinary URL path shape ("See /home/dashboard/settings"), +# so the file gate's bare two-segment form would fire on routine product talk. +# What marks an OPERATOR path is what follows the username: further layout (one +# more path segment) or a file (a dot-bearing final segment, which also catches +# dotdirs like .config). The lookbehind keeps the rule out of absolute URLs, +# where /home/ is preceded by a hostname character. The username class accepts +# capitals: /Users/Someone/ leaks exactly as much as /Users/someone/. Trade +# accepted: a bare "/home/alice/" with nothing after it no longer fires. +check BLOCK abs-user-path '(?` already exists as the honest, visible one. -check BLOCK internal-marker '(?#260"). A gate that fires on all of # those gets switched off, and then it protects nothing. # -# So a bare mention stays silent. What fires is a private repo name within ~140 -# characters of INTERNAL OPERATIONAL DETAIL — a SCREAMING_CASE credential NAME, a -# secret-binding verb, a service binding, or a secret COUNT. That is the topology -# of what is wired to what, and it is the shape that actually leaked. +# So a bare mention stays silent. What fires is a private repo name and INTERNAL +# OPERATIONAL DETAIL on the SAME LINE, within ~140 characters of each other — a +# SCREAMING_CASE credential NAME, a secret-binding verb, a service binding, or a +# secret COUNT. That is the topology of what is wired to what, and it is the +# shape that actually leaked. Trade accepted: the scan is line-scoped (rg matches +# per line and the separator excludes newlines), so a repo name on one line and +# the detail on the next does not fire. Cross-line proximity would need multiline +# scanning with its own false-positive budget; revisit if that shape leaks. # # Names are NOT hardcoded (this file is public); CI injects them via the -# GUARD_PRIVATE_REPOS variable. Unset locally → this check is skipped. In CI -# (GITHUB_ACTIONS set) an empty variable FAILS CLOSED instead: a missing or -# renamed org variable would otherwise silently skip this rule and report a -# pass over an unscanned leak class: a green rubber stamp, not a verdict. -_ALT='' +# GUARD_PRIVATE_REPOS variable. Unset locally → this check is skipped. +_PRIVATE_REPO_OPS_RAN=0 if [[ -n "${GUARD_PRIVATE_REPOS:-}" ]]; then - # The credential-NAME alternative is SCREAMING_CASE on purpose (that is how a - # binding name is written; `session_token` in prose is just code talk), so it - # must stay case-SENSITIVE. The phrase alternatives are ordinary English and - # get their own scoped (?i:) — never a global flag, which would silently make - # the SCREAMING_CASE branch match lowercase identifiers too. - OPS_DETAIL='(?:[A-Z][A-Z0-9]*_(?:SECRET|TOKEN|KEY|PASSWORD)|(?i:wrangler\s+secret|secret\s+(?:is\s+)?(?:bound|binding|list)|(?:is\s+)?bound\s+on|service\s+binding)|\d{2,}\s+secrets)' - IFS=', ' read -r -a _PRIV <<< "$GUARD_PRIVATE_REPOS" + OPS_DETAIL='(?:[A-Z][A-Z0-9]*_(?:SECRET|TOKEN|KEY|PASSWORD)|wrangler\s+secret|secret\s+(?:is\s+)?(?:bound|binding|list)|(?:is\s+)?bound\s+on|service\s+binding|\d{2,}\s+secrets)' + _ALT='' + # The org variable may be comma- OR newline-separated; `read` stops at the first + # newline, which would silently configure only the FIRST name and then report a + # pass over every unscanned name after it. Normalise newlines to spaces before + # splitting — carriage returns too: a CRLF-stored value would otherwise leave an + # invisible \r glued to each name, so the built regex matches nothing and the + # rule fail-opens with no diagnostic at all. + IFS=', ' read -r -a _PRIV <<< "${GUARD_PRIVATE_REPOS//[$'\n'$'\r']/ }" for _name in "${_PRIV[@]}"; do [[ -z "$_name" ]] && continue # Regex-escape so metacharacters in a name match literally. _esc="$(printf '%s' "$_name" | sed -E 's/[][(){}.^$*+?|\\]/\\&/g')" _ALT="${_ALT:+$_ALT|}${_esc}" done + if [[ -n "$_ALT" ]]; then + _PRIVATE_REPO_OPS_RAN=1 + # Both orders: name-then-detail and detail-then-name. Case-insensitivity is + # scoped with (?i:...) to the REPO NAME alone: a leading (?i) would bleed into + # OPS_DETAIL and turn its deliberate SCREAMING_CASE requirement into a match + # on everyday lowercase words (docs/setup_key.md, process.env.api_token), + # blocking exactly the bare cross-references this rule promises to leave alone. + # + # No \b in front of OPS_DETAIL: a multi-segment credential name like + # EXAMPLE_LEASE_SECRET can only start its match at the inner segment (LEASE), + # and the underscore before it is a word character, so a boundary there never + # exists — a leading \b silently exempted every credential name with more than + # one underscore when it followed the repo name. Uppercase-shape matching does + # not need the anchor; starting mid-token still evidences a credential name. + check BLOCK private-repo-ops \ + "(?i:\\b(?:${_ALT})\\b)[^\\n]{0,140}?${OPS_DETAIL}|${OPS_DETAIL}[^\\n]{0,140}?(?i:\\b(?:${_ALT})\\b)" \ + 'A private WAVE repo named alongside internal operational detail (credential name, secret binding, or secret count) — the wiring topology is not public' \ + prose + fi fi -if [[ -n "$_ALT" ]]; then - # Both orders: name-then-detail and detail-then-name. Case-insensitivity is - # scoped to the repo NAMES only — see the OPS_DETAIL comment above. - # No \b in front of OPS_DETAIL in either branch: for a multi-segment name - # like WAVE_VIEWPORT_LEASE_SECRET the credential alternative can only match - # the trailing LEASE_SECRET, and that position is NOT a word boundary - # (underscore is a word character) — a \b there silently drops every - # compound credential name from the name-first order. - check BLOCK private-repo-ops \ - "\\b(?i:${_ALT})\\b[^\\n]{0,140}?${OPS_DETAIL}|${OPS_DETAIL}[^\\n]{0,140}?\\b(?i:${_ALT})\\b" \ - 'A private WAVE repo named alongside internal operational detail (credential name, secret binding, or secret count) — the wiring topology is not public' prose -elif [[ -n "${GITHUB_ACTIONS:-}" ]]; then - echo "::error title=public-repo-guard (private-repo-ops)::GUARD_PRIVATE_REPOS is empty: the org/repo variable is missing, renamed, or contains no names. Refusing to report a pass over an unscanned leak class; configure the variable (fails closed in CI only)." +# Unset locally is fine (the fixtures pin their own names). In CI it is not: an +# empty or names-free variable means the flagship rule scanned NOTHING while the +# job still reports green — the quiet inverse of this script's fail-closed +# posture, and precisely the "green rubber stamp over an unexamined class" that +# every other stage here refuses. A missing or renamed org variable must go RED, +# not emit a warning nobody reads, so this fails CLOSED in CI (GITHUB_ACTIONS +# set) and stays a silent skip only for local runs. +if [[ "$_PRIVATE_REPO_OPS_RAN" == 0 && -n "${GITHUB_ACTIONS:-}" ]]; then + echo "::error title=public-repo-guard (private-repo-ops)::GUARD_PRIVATE_REPOS is empty or contains no names: the private-repo-ops rule scanned nothing this run. Refusing to report a pass over an unscanned leak class — configure the org/repo Actions variable. (Fails closed in CI only; a local run skips the rule.)" exit 2 fi diff --git a/scripts/public-repo-guard/tests/body-policy.test.sh b/scripts/public-repo-guard/tests/body-policy.test.sh index f76e8f7..6cc8500 100755 --- a/scripts/public-repo-guard/tests/body-policy.test.sh +++ b/scripts/public-repo-guard/tests/body-policy.test.sh @@ -13,11 +13,22 @@ SCRIPT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/body-policy.sh" TMP="$(mktemp -d)" trap 'rm -rf "$TMP"' EXIT +# The rules use -P (PCRE2), and not every rg build ships it (Ubuntu's apt package +# does not). On such a build the scanner's fail-closed posture turns EVERY fixture +# into "want exit 1, got 2" — dozens of opaque failures indistinguishable from a +# broken gate. Probe once up front, exactly like the workflow's install step, so +# the suite fails with the actual cause named instead. +command -v rg >/dev/null 2>&1 \ + || { echo "FAIL: ripgrep (rg) is required to run these fixtures" >&2; exit 1; } +echo probe | rg -qP 'p(?=robe)' \ + || { echo "FAIL: this ripgrep build lacks PCRE2 (-P) support, which the policy rules require — install a PCRE2-enabled rg (Ubuntu noble's apt package, brew, or cargo install ripgrep --features pcre2)" >&2; exit 1; } + # The names the real gate is configured with come from an org variable; the tests # pin their own so they are hermetic and do not depend on CI configuration. The -# names are deliberately SYNTHETIC: this file is public and exempt from both tree -# scanners, so a real private repo name written here would itself be a leak. -export GUARD_PRIVATE_REPOS="example-private-alpha, example-private-bravo, example-private-charlie" +# pinned names are deliberately SYNTHETIC: this file is public, and hardcoding a +# real private-repo name here would publish the very fact the gate suppresses. +# The rules are shape-based, so synthetic names exercise identical code paths. +export GUARD_PRIVATE_REPOS="example-priv-alpha, example-priv-beta, example-priv-gamma" PASS=0; FAIL=0 @@ -41,22 +52,38 @@ echo "body-policy fixtures" # --- must BLOCK --------------------------------------------------------------- expect 1 'private repo + credential name' \ - 'Flip is live: WAVE_VIEWPORT_LEASE_SECRET is bound on example-private-alpha now.' + 'Flip is live: EXAMPLE_LEASE_SECRET is bound on example-priv-alpha now.' expect 1 'private repo + credential name, reverse order' \ - 'The MOQ_JOIN_SECRET was added; example-private-bravo picks it up on deploy.' -# Regression: a MULTI-segment credential name after the repo name. The rule can -# only match the trailing LEASE_SECRET, which sits mid-word (after an -# underscore) — a \b in front of OPS_DETAIL silently dropped this whole order. -expect 1 'private repo, then compound credential name' \ - 'example-private-alpha now uses WAVE_VIEWPORT_LEASE_SECRET for renewals.' + 'The EXAMPLE_JOIN_SECRET was added; example-priv-beta picks it up on deploy.' expect 1 'private repo + secret count' \ - 'example-private-alpha went from 74 secrets to 75 after this change.' + 'example-priv-alpha went from 74 secrets to 75 after this change.' +expect 1 'repo name matches case-insensitively' \ + 'Flip is live: EXAMPLE_LEASE_SECRET is bound on Example-Priv-Alpha now.' expect 1 'private repo + service binding' \ - 'This adds a service binding from the worker to example-private-charlie for settlement.' + 'This adds a service binding from the worker to example-priv-gamma for settlement.' +# Regression: a leading \b before the credential-name shape made multi-underscore +# names (only matchable from their inner segment, which follows a word character) +# unmatchable in name-then-detail order, silently exempting exactly these bodies. +expect 1 'private repo then multi-segment credential name' \ + 'example-priv-alpha now reads EXAMPLE_LEASE_SECRET at boot.' +expect 1 'private repo then multi-segment token name' \ + 'example-priv-alpha now reads WAVE_API_TOKEN at boot.' expect 1 'operator home path' \ 'Repro: run it from /Users/someoperator/Documents/notes and it fails.' # enforce-ignore (fixture) +expect 1 'operator home path, capitalized username' \ + 'Logs land in /Users/Someone/Library/Logs/wave.log on my machine.' # enforce-ignore (fixture) +expect 1 'operator home path, file directly under the home dir' \ + 'The crash referenced /home/someoperator/wrangler.toml directly.' # enforce-ignore (fixture) expect 1 'internal-only marker' \ 'Attaching the internal-only rollout plan for context.' +# The marker rules are case-insensitive on purpose: sentence-initial and shouted +# forms are how these phrases are actually written. +expect 1 'capitalized internal-only marker' \ + 'Internal-only rollout plan attached.' +expect 1 'shouted do-not-share marker' \ + 'DO NOT SHARE outside the team.' +expect 1 'for-internal-use marker, sentence-initial' \ + 'For internal use only; see the attached doc.' # Assembled at run time rather than written as a literal: a fixture that LOOKS like # a live AWS key trips this repo's own pre-commit secret scanners (it did, on the # first draft). Splitting the prefix keeps the fixture exercising the real regex @@ -66,37 +93,53 @@ expect 1 'AWS access key id' \ "The failing job had ${AKID_FIXTURE} configured." expect 1 'internal tailscale IP' \ 'It resolves to 100.71.4.19 from inside the fleet.' -# Regression: ABOUT_THE_CONTROL is scoped to the heuristic prose rules. A real -# credential must block even when the same line talks about the gate — otherwise -# "mention the policy" is a one-line bypass of every format rule. -expect 1 'credential on a line that mentions the policy' \ - "Discussing public-repo-guard: the key ${AKID_FIXTURE} was rotated." -expect 1 'private key on a line citing SECURITY.md' \ - "Per SECURITY.md: -----BEGIN RSA ""PRIVATE KEY-----" # pragma: allowlist secret (fixture, split like AKID above) +# The same-line bypass: mentioning the gate must never launder a credential. +# ABOUT_THE_CONTROL is prose-rules-only; a key next to "public-repo-guard" blocks. +expect 1 'credential on a line that names the control still blocks' \ + "public-repo-guard flagged ${AKID_FIXTURE} in the run linked from SECURITY.md." +expect 1 'internal IP on a line that names the control still blocks' \ + 'body-policy missed 100.71.4.19 on the first pass; fixed now.' # --- must PASS (precision — these keep the gate deployable) ------------------- expect 0 'bare private-repo cross-reference' \ - 'This is the companion change to example-private-bravo#260; merge that one first.' + 'This is the companion change to example-priv-beta#260; merge that one first.' +# Case-insensitivity must stay scoped to the repo NAME: lowercase everyday words +# ending in key/token/secret are not operational detail. +expect 0 'lowercase key-ish word near a private repo is not ops detail' \ + 'Companion to example-priv-beta#260; see docs/setup_key.md for the steps.' +expect 0 'lowercase env accessor near a private repo is not ops detail' \ + 'example-priv-alpha now reads the value from process.env.api_token in dev.' expect 0 'two private repos, no operational detail' \ - 'Both example-private-alpha and example-private-bravo will need a follow-up for this.' + 'Both example-priv-alpha and example-priv-beta will need a follow-up for this.' expect 0 'credential NAME with no private repo nearby' \ 'The handler now reads SOME_API_TOKEN from the environment instead of a literal.' -# Regression: the SCREAMING_CASE credential-name branch is case-SENSITIVE. A -# global (?i) once leaked onto it and lowercase code talk near a repo name -# (`session_token`) blocked ordinary prose — exactly the false-positive class -# that gets a gate switched off. -expect 0 'lowercase code identifier near a private repo' \ - 'The example-private-alpha worker reads session_token from the request header.' -expect 1 'repo name cased differently still pairs with a credential NAME' \ - 'Example-Private-Alpha now requires LEASE_SECRET at deploy time.' -expect 0 'talking about the gate with a repo name and credential NAME' \ - 'body-policy blocks example-private-alpha next to a SECRET_TOKEN; that is intended.' expect 0 'public runner path is not an operator path' \ 'CI checks out to /home/runner/work/repo/repo before the scan runs.' # enforce-ignore (fixture) +# Body text is prose, and prose contains app routes: /home// is an ordinary +# URL path shape. Only username-plus-layout (or a dot-bearing file segment) fires. +expect 0 'app route under /home/ is not an operator path' \ + 'See /home/dashboard/settings route for the new page.' +expect 0 'absolute URL with a deep /home/ path is not an operator path' \ + 'Deep link: https://app.wave.online/home/dashboard/settings/profile works now.' +# RANGE-talk is not a fleet address: the documentation form of the CGNAT range +# (all-zero host, or any CIDR-suffixed subnet) appears in ordinary security +# discussion — including quotes of this gate's own comments — and must pass. +expect 0 'CGNAT range in documentation form (CIDR)' \ + 'The internal-ip rule covers the Tailscale CGNAT range 100.64.0.0/10 by design.' +expect 0 'CGNAT range with all-zero host, no CIDR' \ + 'The fleet overlay uses 100.64.0.0 as its network address.' +expect 0 'CIDR-suffixed subnet of the range' \ + 'Traffic from 100.71.4.0/24 is routed through the tunnel.' expect 0 'talking about the control' \ 'body-policy blocks a private repo named next to a SECRET_TOKEN; that is intended.' +# Prose rules DO consult ABOUT_THE_CONTROL: a sentence describing the gate's +# behaviour with a real repo name stays discussable. +expect 0 'prose rule discussing the gate (repo + credential name)' \ + 'public-repo-guard fires when example-priv-alpha appears near EXAMPLE_SECRET; see the fixtures.' +expect 0 'unquoted marker on a line that names the control' \ + 'public-repo-guard blocks internal-only markers wherever they appear in body text.' expect 0 'explicit guard:allow with a reason' \ - 'Example for the docs: example-private-alpha holds EXAMPLE_SECRET — guard:allow documented-example' + 'Example for the docs: example-priv-alpha holds EXAMPLE_SECRET — guard:allow documented-example' expect 0 'ordinary clean body' \ 'Bumps the draft revision and regenerates the fixtures. No behaviour change.' # Regression: the first CI run of this job failed on its own PR, because a review @@ -125,23 +168,60 @@ for case in "no argument at all::" "nonexistent path::$TMP/does-not-exist.txt"; fi done -# An empty GUARD_PRIVATE_REPOS is a documented local convenience but FAILS CLOSED -# in CI (GITHUB_ACTIONS set): a missing or renamed org variable must go red, never -# silently skip the private-repo rule and report a pass over an unscanned class. +# Regression: the post-scan filter stages must fail CLOSED too. They once ran +# `rg ... || true`, so a filter error (exit >= 2, e.g. a PCRE2-less rg on the -P +# allowlist filter) emptied the match list and reported CLEAN on a body whose +# main scan had already found a leak. Simulate with an rg shim that errors on +# inverted-match (-v*) invocations and delegates everything else to the real rg: +# the main scan still hits, and the gate must exit 2, never 0. +REAL_RG="$(command -v rg)" +mkdir -p "$TMP/fakebin" +cat > "$TMP/fakebin/rg" < "$TMP/body.txt" +PATH="$TMP/fakebin:$PATH" bash "$SCRIPT" "$TMP/body.txt" >/dev/null 2>&1 +rc=$? +if [[ "$rc" == 2 ]]; then + PASS=$((PASS+1)); printf ' ok broken filter stage → exit 2 (fails closed)\n' +else + FAIL=$((FAIL+1)); printf ' FAIL broken filter stage — want exit 2, got %s\n' "$rc" +fi + +# Regression: `read` stops at the first newline, so a newline-separated org +# variable once configured only the first name and reported a pass over every +# unscanned name after it. A CRLF-stored value glued an invisible \r to each name, +# which made the built regex match nothing and fail OPEN with no diagnostic. +GUARD_PRIVATE_REPOS=$'example-priv-alpha\nexample-priv-beta\nexample-priv-gamma' \ +expect 1 'newline-separated GUARD_PRIVATE_REPOS still scans later names' \ + 'The EXAMPLE_JOIN_SECRET was added; example-priv-beta picks it up on deploy.' +GUARD_PRIVATE_REPOS=$'example-priv-alpha\r\nexample-priv-beta\r\nexample-priv-gamma\r' \ +expect 1 'CRLF-separated GUARD_PRIVATE_REPOS still scans every name' \ + 'The EXAMPLE_JOIN_SECRET was added; example-priv-beta picks it up on deploy.' + +# An empty GUARD_PRIVATE_REPOS is a documented local convenience, but in CI it +# means the flagship rule scanned nothing while the job reports green. It must +# fail CLOSED there (exit 2) and stay a silent skip locally (exit 0). printf '%s\n' 'Ordinary clean body with nothing to find.' > "$TMP/body.txt" env -u GUARD_PRIVATE_REPOS GITHUB_ACTIONS=true bash "$SCRIPT" "$TMP/body.txt" >/dev/null 2>&1 rc=$? if [[ "$rc" == 2 ]]; then - PASS=$((PASS+1)); printf ' ok %s → exit 2 (fails closed)\n' 'GUARD_PRIVATE_REPOS unset in CI' + PASS=$((PASS+1)); printf ' ok GUARD_PRIVATE_REPOS unset in CI → exit 2 (fails closed)\n' else - FAIL=$((FAIL+1)); printf ' FAIL %s: want exit 2, got %s\n' 'GUARD_PRIVATE_REPOS unset in CI' "$rc" + FAIL=$((FAIL+1)); printf ' FAIL GUARD_PRIVATE_REPOS unset in CI — want exit 2, got %s\n' "$rc" fi env -u GUARD_PRIVATE_REPOS -u GITHUB_ACTIONS bash "$SCRIPT" "$TMP/body.txt" >/dev/null 2>&1 rc=$? if [[ "$rc" == 0 ]]; then - PASS=$((PASS+1)); printf ' ok %s → exit 0 (rule skipped)\n' 'GUARD_PRIVATE_REPOS unset locally' + PASS=$((PASS+1)); printf ' ok GUARD_PRIVATE_REPOS unset locally → exit 0 (rule skipped)\n' else - FAIL=$((FAIL+1)); printf ' FAIL %s: want exit 0, got %s\n' 'GUARD_PRIVATE_REPOS unset locally' "$rc" + FAIL=$((FAIL+1)); printf ' FAIL GUARD_PRIVATE_REPOS unset locally — want exit 0, got %s\n' "$rc" fi echo " ---"