From 864f4f54b08b3a4af0f65be956680ef701255617 Mon Sep 17 00:00:00 2001 From: yakimoto <66892052+yakimoto@users.noreply.github.com> Date: Thu, 6 Aug 2026 11:53:08 -0400 Subject: [PATCH 1/8] 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 1cbbf9f9f758996476715f0b1647ef11b05d5005 Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 16:06:11 +0000 Subject: [PATCH 2/8] fix: scope the about-the-control allowlist to prose rules and (?i) to repo names Review findings from wave-av/cli#22: - The ABOUT_THE_CONTROL allowlist applied to every rule, so a credential pasted on a line that happened to name the gate was reported clean. It now applies only to the self-referential prose rules (internal-marker, private-repo-ops); credential-format and infrastructure rules honour only the explicit guard:allow marker. - A global (?i) leaked into OPS_DETAIL, letting lowercase prose like api_key count as a SCREAMING_CASE credential name next to a private repo. Case-insensitivity is now scoped to the repo-name alternation and the prose verbs. - GUARD_PRIVATE_REPOS parsing read only the first line; newlines are now normalized to spaces before splitting. - Both jobs install a pinned, checksum-verified PCRE2-enabled ripgrep release instead of relying on apt (Ubuntu's package lacks PCRE2, so rg -P exits 2 and every check fails closed). Adds regression fixtures for all of the above; suite is 24/24. Co-authored-by: Codesmith --- .github/workflows/public-repo-guard.yml | 41 +++++++++++++++-- scripts/public-repo-guard/body-policy.sh | 44 ++++++++++++++----- .../tests/body-policy.test.sh | 14 ++++++ 3 files changed, 84 insertions(+), 15 deletions(-) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index bba2f67..70757e4 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -85,8 +85,26 @@ jobs: - name: gitleaks (secret scan — published tree) run: gitleaks detect --no-git --source . --config .gitleaks.toml --redact --no-banner --exit-code 1 - - name: Install ripgrep - run: command -v rg >/dev/null || (sudo apt-get update -qq && sudo apt-get install -y -qq ripgrep) + # Both policy scripts use `rg -P` (PCRE2). Ubuntu's apt ripgrep is built + # WITHOUT PCRE2, so `rg -P` exits 2 there and the scripts fail closed — + # red on every run, which gets a gate switched off. Accept a preinstalled + # rg only if it actually has PCRE2; otherwise install the official release + # binary (PCRE2 compiled in), pinned and checksum-verified like gitleaks. + - name: Install ripgrep (pinned + checksum-verified, PCRE2 build) + env: + RIPGREP_VERSION: "14.1.1" + RIPGREP_SHA256: "4cf9f2741e6c465ffdb7c26f38056a59e2a2544b51f7cc128ef28337eeae4d8e" + run: | + if command -v rg >/dev/null && rg --pcre2-version >/dev/null 2>&1; then + echo "using preinstalled $(rg --version | head -n1) with PCRE2"; exit 0 + fi + curl -fsSL --proto '=https' --tlsv1.2 -o ripgrep.tar.gz \ + "https://github.com/BurntSushi/ripgrep/releases/download/${RIPGREP_VERSION}/ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl.tar.gz" + echo "${RIPGREP_SHA256} ripgrep.tar.gz" | sha256sum -c - + tar -xzf ripgrep.tar.gz --strip-components=1 "ripgrep-${RIPGREP_VERSION}-x86_64-unknown-linux-musl/rg" + sudo install -m 0755 rg /usr/local/bin/rg + rm -f rg ripgrep.tar.gz + rg --pcre2-version - name: content policy (WAVE trade-secret / internal-leak gate) env: @@ -132,8 +150,23 @@ 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 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 diff --git a/scripts/public-repo-guard/body-policy.sh b/scripts/public-repo-guard/body-policy.sh index a0b421f..15f85b7 100755 --- a/scripts/public-repo-guard/body-policy.sh +++ b/scripts/public-repo-guard/body-policy.sh @@ -18,8 +18,9 @@ # 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 self-referential +# PROSE rules only — never the credential-format or infrastructure rules. set -uo pipefail FILE="${1:-}" @@ -32,11 +33,22 @@ 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: this allowlist applies ONLY to the rules that can self-trip when a body +# DESCRIBES the control (the internal-marker and private-repo-ops rules below — +# they match prose, and prose about the gate looks like prose about a leak). It +# must NEVER apply to the credential-format or infrastructure-identifier rules: +# a live key is a live key even when the sentence around it names the gate, and +# PRs about this gate are exactly the ones whose bodies contain these words. For +# those rules the only exemption is the explicit, visible `guard:allow `. 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 [about-the-control-exempt] +# Pass the literal string `about-the-control-exempt` as the 5th argument to let +# lines matching ABOUT_THE_CONTROL through. Only self-referential prose rules +# may opt in; hard-format rules must not. check() { - local sev="$1" name="$2" re="$3" why="$4" + local sev="$1" name="$2" re="$3" why="$4" about_exempt="${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 +63,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 [[ "$about_exempt" == "about-the-control-exempt" ]]; 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 +111,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 16:07:33 +0000 Subject: [PATCH 3/8] fix: fail closed when an allowlist filter errors instead of masking it with || true The initial rg -nP scan already failed closed on exit >=2, but the guard:allow and about-the-control filter pipelines appended || true, so a filter error would be treated as an empty match set and report a clean scan. The filters now distinguish exit 1 (every line filtered, fine) from >=2 (scanner error, exit 2). Co-authored-by: Codesmith --- scripts/public-repo-guard/body-policy.sh | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/scripts/public-repo-guard/body-policy.sh b/scripts/public-repo-guard/body-policy.sh index 15f85b7..8cc085e 100755 --- a/scripts/public-repo-guard/body-policy.sh +++ b/scripts/public-repo-guard/body-policy.sh @@ -61,11 +61,23 @@ 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 too: exit 1 just means every line was filtered (fine), + # but >=2 is a scanner error, and treating it as "no matches" would turn a broken + # allowlist into a green check over real hits. local matches matches="$(printf '%s' "$raw" \ - | rg -vN -- 'guard:allow[[:space:]]+[^[:space:]]' || true)" + | rg -vN -- 'guard:allow[[:space:]]+[^[:space:]]')"; rc=$? + if (( rc >= 2 )); then + echo "::error title=public-repo-guard ($name)::ripgrep failed (exit $rc) applying the guard:allow filter for rule '$name' — failing closed." + exit 2 + fi if [[ "$about_exempt" == "about-the-control-exempt" ]]; then - matches="$(printf '%s' "$matches" | rg -vNiP -- "$ABOUT_THE_CONTROL" || true)" + 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) applying the about-the-control allowlist for rule '$name' — failing closed." + exit 2 + fi fi [[ -z "$matches" ]] && return 0 local count; count="$(printf '%s\n' "$matches" | grep -c '')" From 897b6cb202837dd14db04ec753cc43a482db393e Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 16:17:45 +0000 Subject: [PATCH 4/8] fix(guard): remove author-controlled allowlist escapes from body hard rules A body has no reviewable diff, so a guard:allow marker there is one edit away, not one review away: the same untrusted author who writes a leaking line could append the marker and turn every rule green, including the live-credential rules. Scope guard:allow (like the about-the-control list) to the self-referential prose rules only; credential-format and infrastructure rules now have no author-controlled escape in a body, with a regression fixture proving a guard:allow'd AWS key still blocks. Also port body-policy's newline normalization of GUARD_PRIVATE_REPOS to content-policy.sh: read stops at the first newline, so a one-name-per-line org variable silently configured only the first name and the tree scanner passed over the unscanned rest. Co-authored-by: Codesmith --- scripts/public-repo-guard/body-policy.sh | 41 +++++++++++++------ scripts/public-repo-guard/content-policy.sh | 7 +++- .../tests/body-policy.test.sh | 5 +++ 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/scripts/public-repo-guard/body-policy.sh b/scripts/public-repo-guard/body-policy.sh index 8cc085e..b376662 100755 --- a/scripts/public-repo-guard/body-policy.sh +++ b/scripts/public-repo-guard/body-policy.sh @@ -17,10 +17,16 @@ # # 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). -# The ABOUT-THE-CONTROL allowlist below additionally exempts the self-referential -# PROSE rules only — never the credential-format or infrastructure rules. +# Allowlisting: unlike the tree scanner, where a `guard:allow` marker lands in a +# reviewable diff, a body is free-form text the untrusted author controls and can +# edit at any time — an allowlist marker there is one edit away, not one review +# away. So BOTH allowlists (the `guard:allow ` marker and the +# ABOUT-THE-CONTROL list below) apply ONLY to the self-referential PROSE rules +# (internal-marker, private-repo-ops) — never to the credential-format or +# infrastructure rules, which have NO author-controlled escape in a body. The +# residual prose-rule escape is an accepted trade: the threat model is the +# accidental paste, and a gate that blocks its own security discussions gets +# switched off. set -uo pipefail FILE="${1:-}" @@ -45,8 +51,10 @@ ABOUT_THE_CONTROL='(public-repo-guard|body-policy|content-policy|public-github-w # check [about-the-control-exempt] # Pass the literal string `about-the-control-exempt` as the 5th argument to let -# lines matching ABOUT_THE_CONTROL through. Only self-referential prose rules -# may opt in; hard-format rules must not. +# lines matching ABOUT_THE_CONTROL or carrying `guard:allow ` through. +# Only self-referential prose rules may opt in; hard-format rules must not — +# in a body both escapes are author-controlled, so a live key stays a hit no +# matter what else its line says. check() { local sev="$1" name="$2" re="$3" why="$4" about_exempt="${5:-}" [[ -z "$re" ]] && { echo "::error::body-policy: internal bug — empty regex for rule '$name'"; exit 2; } @@ -65,14 +73,21 @@ check() { # The filters fail closed too: exit 1 just means every line was filtered (fine), # but >=2 is a scanner error, and treating it as "no matches" would turn a broken # allowlist into a green check over real hits. - local matches - matches="$(printf '%s' "$raw" \ - | rg -vN -- 'guard:allow[[:space:]]+[^[:space:]]')"; rc=$? - if (( rc >= 2 )); then - echo "::error title=public-repo-guard ($name)::ripgrep failed (exit $rc) applying the guard:allow filter for rule '$name' — failing closed." - exit 2 - fi + # + # BOTH allowlists live inside the opt-in branch: a body has no reviewable diff, + # so the untrusted author could otherwise neutralize any rule — including the + # live-credential rules — by appending `guard:allow ` to the same line. + # Only the self-referential prose rules may be exempted, and only because their + # alternative (blocking every discussion of the gate itself) gets the gate + # switched off. + local matches="$raw" if [[ "$about_exempt" == "about-the-control-exempt" ]]; then + matches="$(printf '%s' "$matches" \ + | rg -vN -- 'guard:allow[[:space:]]+[^[:space:]]')"; rc=$? + if (( rc >= 2 )); then + echo "::error title=public-repo-guard ($name)::ripgrep failed (exit $rc) applying the guard:allow filter for rule '$name' — failing closed." + exit 2 + fi 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) applying the about-the-control allowlist for rule '$name' — failing closed." diff --git a/scripts/public-repo-guard/content-policy.sh b/scripts/public-repo-guard/content-policy.sh index 4f67fb8..09d5d81 100755 --- a/scripts/public-repo-guard/content-policy.sh +++ b/scripts/public-repo-guard/content-policy.sh @@ -99,7 +99,12 @@ check BLOCK abs-user-path '/(Users|home)/(?!runner/)[a-z][a-z0-9._-]+/' # at run time via GUARD_PRIVATE_REPOS (CI injects it from an org-level Actions # variable), comma- or space-separated. Unset locally → this check is skipped. if [[ -n "${GUARD_PRIVATE_REPOS:-}" ]]; then - IFS=', ' read -r -a _PRIV <<< "$GUARD_PRIVATE_REPOS" + # The org variable may be comma- OR newline-separated; `read` stops at the first + # newline, which would silently configure only the first name and report a pass + # over the unscanned rest. Normalize newlines to spaces before splitting — kept + # in lockstep with body-policy.sh so both halves of the gate parse the same + # variable identically. + IFS=', ' read -r -a _PRIV <<< "${GUARD_PRIVATE_REPOS//$'\n'/ }" for _name in "${_PRIV[@]}"; do [[ -z "$_name" ]] && continue # Regex-escape the name so metacharacters in a repo name (., -, etc.) match diff --git a/scripts/public-repo-guard/tests/body-policy.test.sh b/scripts/public-repo-guard/tests/body-policy.test.sh index 2b79325..75bbe67 100755 --- a/scripts/public-repo-guard/tests/body-policy.test.sh +++ b/scripts/public-repo-guard/tests/body-policy.test.sh @@ -62,6 +62,11 @@ expect 1 'AWS access key id' \ # formats are never legitimate in prose — no gate word may exempt one. expect 1 'gate word on the same line does NOT exempt a credential' \ "body-policy note: the leaked key was ${AKID_FIXTURE} here." +# Regression: `guard:allow` once exempted EVERY rule, but a body has no reviewable +# diff — the untrusted author can append the marker in the same edit that leaks. +# The marker may only exempt the self-referential prose rules, never a credential. +expect 1 'guard:allow does NOT exempt a credential in a body' \ + "Key for the repro: ${AKID_FIXTURE} — guard:allow repro-example" expect 1 'internal tailscale IP' \ 'It resolves to 100.71.4.19 from inside the fleet.' # Regression: `read` stops at the first newline, so a newline-separated org variable From 141c057ba187893e798625ab95b7ec4783d7e228 Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 16:20:06 +0000 Subject: [PATCH 5/8] fix(guard): close two body/tree scan gaps in the workflow triggers Retargeting a PR fires only `edited` (never `synchronize`), so the blanket edited-skip let a base-branch change reach merge with the old merge tree's green check; the tree job now runs on edited events whose changes carry a base object. Inline diff review comments are a separate event from issue_comment and were world-readable text no job ever scanned; add the pull_request_review_comment trigger to body-guard and key its concurrency group on the comment id first so two rapid review comments on one PR cannot collapse into a single group and drop a verdict. Co-authored-by: Codesmith --- .github/workflows/public-repo-guard.yml | 26 +++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index 70757e4..f71ae3b 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -34,6 +34,10 @@ on: 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] push: branches: [main, master] workflow_dispatch: @@ -53,9 +57,13 @@ 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) and - # skips `edited` (a title or body edit does not change the tree). + # skips `edited` UNLESS the base branch changed: a title or body edit does not + # change the tree, but retargeting a PR changes the merge result and fires only + # `edited` (never `synchronize`), so without this carve-out nothing would ever + # re-scan the new merge tree before merge. if: >- - (github.event_name == 'pull_request' && github.event.action != 'edited') + (github.event_name == 'pull_request' + && (github.event.action != 'edited' || github.event.changes.base != null)) || github.event_name == 'push' || github.event_name == 'workflow_dispatch' concurrency: @@ -130,16 +138,22 @@ 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_comment' concurrency: - # Keyed on the specific PR / comment / issue rather than github.ref, because + # Keyed on the specific comment / 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. + # two comments cancel each other, leaving one unscanned. The comment id comes + # FIRST: review-comment payloads also carry the PR number, and keying those on + # the PR would collapse two rapid comments into one group, dropping a verdict. # # cancel-in-progress is deliberately FALSE. Every version of a body deserves a # verdict, the job is seconds long, and a cancelled check-run lingers on the # commit and makes an otherwise-green PR look broken. - group: public-repo-guard-body-${{ github.event.pull_request.number || github.event.comment.id || github.event.issue.number || github.ref }} + group: public-repo-guard-body-${{ github.event.comment.id || github.event.pull_request.number || github.event.issue.number || github.ref }} cancel-in-progress: false runs-on: ubuntu-latest steps: From 6435e7997c169e6df4295a149f0c5a72fee83ec0 Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 16:36:45 +0000 Subject: [PATCH 6/8] ci: never publish a skippable verdict on a PR head; exempt the CGNAT range designation A job skipped by a job-level if still publishes a check run with conclusion skipped on the PR head SHA, branch protection treats skipped as passing, and the newest check run for a name wins. So skipping the tree scan on edited (or on review/review-comment events) let a title edit or an inline comment flip an already-failed required check green. The guard job now skips only issues and issue_comment events, whose runs attach to the default branch and cannot mask a PR verdict. Also: scan top-level review bodies via pull_request_review (trigger, if, concurrency key, jq .review.body), set persist-credentials: false on both checkouts to match the repo's other workflows, and stop the internal-ip rule matching the range designation 100.64.0.0(/nn), which is the public NAME of the CGNAT range, appears in the gate's own docs, and had no allowlist escape. Fixtures added for the designation (pass) and an adjacent real host (block). Co-authored-by: Codesmith --- .github/workflows/public-repo-guard.yml | 55 ++++++++++++------- scripts/public-repo-guard/body-policy.sh | 9 ++- .../tests/body-policy.test.sh | 11 ++++ 3 files changed, 54 insertions(+), 21 deletions(-) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index f71ae3b..a0de011 100644 --- a/.github/workflows/public-repo-guard.yml +++ b/.github/workflows/public-repo-guard.yml @@ -38,6 +38,11 @@ on: # 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] push: branches: [main, master] workflow_dispatch: @@ -55,23 +60,28 @@ permissions: 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` UNLESS the base branch changed: a title or body edit does not - # change the tree, but retargeting a PR changes the merge result and fires only - # `edited` (never `synchronize`), so without this carve-out nothing would ever - # re-scan the new merge tree before merge. - if: >- - (github.event_name == 'pull_request' - && (github.event.action != 'edited' || github.event.changes.base != null)) - || github.event_name == 'push' - || github.event_name == 'workflow_dispatch' + # Skips ONLY issues/issue_comment events: the tree scan has nothing to say + # about a comment, and those events run against the DEFAULT branch, so their + # skipped check runs cannot attach to any PR head. Every event that runs in a + # PR's context (pull_request INCLUDING `edited`, pull_request_review, + # pull_request_review_comment) must run the scan for real: a job skipped by a + # job-level `if` still publishes a check run named "Secrets + content policy" + # with conclusion `skipped` on the PR head SHA, branch protection treats + # skipped as passing, and the newest check run for a name wins — so a mere + # title edit or review comment would flip an already-FAILED required tree + # scan green with nothing re-examining the tree. Re-scanning an unchanged + # tree costs minutes; a maskable required check costs the gate. + if: github.event_name != 'issues' && github.event_name != 'issue_comment' 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: + # This job only reads the tree — never leave the token sitting in + # .git/config while repo-supplied scripts execute in the workspace. + persist-credentials: false # gitleaks' GitHub Action requires a paid license for organizations; the CLI # itself is MIT-licensed and free. Pin the version AND verify the release @@ -143,17 +153,19 @@ jobs: || github.event_name == 'issues' || github.event_name == 'issue_comment' || github.event_name == 'pull_request_review_comment' + || github.event_name == 'pull_request_review' concurrency: - # Keyed on the specific comment / 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 id comes - # FIRST: review-comment payloads also carry the PR number, and keying those on - # the PR would collapse two rapid comments into one group, dropping a verdict. + # Keyed on the specific comment / review / PR / issue rather than github.ref, + # because issue events all report the default branch and a ref-keyed group + # would let two comments cancel each other, leaving one unscanned. The comment + # and review ids come FIRST: those payloads also carry the PR number, and + # keying them on the PR would collapse two rapid comments into one group, + # dropping a verdict. # # cancel-in-progress is deliberately FALSE. Every version of a body deserves a # verdict, the job is seconds long, and a cancelled check-run lingers on the # commit and makes an otherwise-green PR look broken. - group: public-repo-guard-body-${{ github.event.comment.id || github.event.pull_request.number || 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: @@ -163,6 +175,9 @@ jobs: # 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. @@ -193,13 +208,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, - .comment.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" diff --git a/scripts/public-repo-guard/body-policy.sh b/scripts/public-repo-guard/body-policy.sh index b376662..b69a9c5 100755 --- a/scripts/public-repo-guard/body-policy.sh +++ b/scripts/public-repo-guard/body-policy.sh @@ -121,7 +121,14 @@ 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 leading lookahead exempts the range's own DESIGNATION — the all-zero +# network address 100.64.0.0, with or without a CIDR suffix. That string is the +# public NAME of the CGNAT range (it appears in this rule's own message), and +# infrastructure rules deliberately accept no allowlist marker, so matching it +# would block every body that so much as quotes the gate's documentation, with +# no remedy short of deleting the text. Every real fleet address — any host +# with a non-zero octet — is still a hit. +check BLOCK internal-ip '(?!100\.64\.0\.0(?:/[0-9]{1,2})?(?![0-9]))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' diff --git a/scripts/public-repo-guard/tests/body-policy.test.sh b/scripts/public-repo-guard/tests/body-policy.test.sh index 75bbe67..b9269da 100755 --- a/scripts/public-repo-guard/tests/body-policy.test.sh +++ b/scripts/public-repo-guard/tests/body-policy.test.sh @@ -69,6 +69,10 @@ expect 1 'guard:allow does NOT exempt a credential in a body' \ "Key for the repro: ${AKID_FIXTURE} — guard:allow repro-example" expect 1 'internal tailscale IP' \ 'It resolves to 100.71.4.19 from inside the fleet.' +# The range-designation exemption must stay razor-thin: a host one address past +# the all-zero network form is a real fleet machine and still blocks. +expect 1 'host adjacent to the network address still blocks' \ + 'The subnet router answers on 100.64.0.1 inside the fleet.' # Regression: `read` stops at the first newline, so a newline-separated org variable # once configured only the first name and passed over the unscanned rest. GUARD_PRIVATE_REPOS=$'wave-gateway\nwave-transports\nagent-money' \ @@ -94,6 +98,13 @@ 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 risk called out in review: the gate's own docs (and any body quoting +# them, which review bots do) name the range as 100.64.0.0/10. That is the NAME +# of the range, not a host on it, and infra rules have no allowlist escape. +expect 0 'the CGNAT range designation is the name of the range, not a host' \ + 'The internal-ip rule covers the 100.64.0.0/10 space by design.' +expect 0 'the bare all-zero network address is a designation too' \ + 'Traffic in 100.64.0.0 space never leaves the tailnet.' # 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' \ From 2410a12fc32669db7ea743e8894230bf93edc92f Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 16:46:19 +0000 Subject: [PATCH 7/8] =?UTF-8?q?ci:=20install=20list=20is=20five=20files=20?= =?UTF-8?q?=E2=80=94=20the=20workflow=20runs=20the=20fixture=20self-test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard job executes scripts/public-repo-guard/tests/body-policy.test.sh, so a repo installed from the four-file list fails every run on the missing script (bash exits 127 under the default -e shell), and a permanently-red required gate gets switched off. Co-authored-by: Codesmith --- .github/workflows/public-repo-guard.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/public-repo-guard.yml b/.github/workflows/public-repo-guard.yml index a0de011..7b2da26 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. From 743a99f365ba9dbbca4a2701d49cf41b7f2613fe Mon Sep 17 00:00:00 2001 From: yakimoto Date: Thu, 6 Aug 2026 16:55:53 +0000 Subject: [PATCH 8/8] fix: strip carriage returns from GUARD_PRIVATE_REPOS in both policy scripts A CRLF-stored org variable glued an invisible \r to every name after the LF normalization, so the built regex matched nothing and the private-repo gate fail-opened with no diagnostic. Normalize \r alongside \n in both scripts and add a CRLF regression fixture. Co-authored-by: Codesmith --- scripts/public-repo-guard/body-policy.sh | 7 +++++-- scripts/public-repo-guard/content-policy.sh | 9 +++++---- scripts/public-repo-guard/tests/body-policy.test.sh | 5 +++++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/scripts/public-repo-guard/body-policy.sh b/scripts/public-repo-guard/body-policy.sh index b69a9c5..965385f 100755 --- a/scripts/public-repo-guard/body-policy.sh +++ b/scripts/public-repo-guard/body-policy.sh @@ -171,8 +171,11 @@ if [[ -n "${GUARD_PRIVATE_REPOS:-}" ]]; then _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 report a pass - # over the unscanned rest. Normalize newlines to spaces before splitting. - IFS=', ' read -r -a _PRIV <<< "${GUARD_PRIVATE_REPOS//$'\n'/ }" + # over the unscanned rest. Normalize newlines to spaces before splitting — and + # 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 gate + # fail-opens with no diagnostic. + 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. diff --git a/scripts/public-repo-guard/content-policy.sh b/scripts/public-repo-guard/content-policy.sh index 09d5d81..40e6734 100755 --- a/scripts/public-repo-guard/content-policy.sh +++ b/scripts/public-repo-guard/content-policy.sh @@ -101,10 +101,11 @@ check BLOCK abs-user-path '/(Users|home)/(?!runner/)[a-z][a-z0-9._-]+/' if [[ -n "${GUARD_PRIVATE_REPOS:-}" ]]; then # The org variable may be comma- OR newline-separated; `read` stops at the first # newline, which would silently configure only the first name and report a pass - # over the unscanned rest. Normalize newlines to spaces before splitting — kept - # in lockstep with body-policy.sh so both halves of the gate parse the same - # variable identically. - IFS=', ' read -r -a _PRIV <<< "${GUARD_PRIVATE_REPOS//$'\n'/ }" + # over the unscanned rest. Normalize newlines AND carriage returns to spaces + # before splitting (a CRLF-stored value would otherwise glue an invisible \r to + # each name and fail-open) — kept in lockstep with body-policy.sh so both + # halves of the gate parse the same variable identically. + IFS=', ' read -r -a _PRIV <<< "${GUARD_PRIVATE_REPOS//[$'\n'$'\r']/ }" for _name in "${_PRIV[@]}"; do [[ -z "$_name" ]] && continue # Regex-escape the name so metacharacters in a repo name (., -, etc.) match diff --git a/scripts/public-repo-guard/tests/body-policy.test.sh b/scripts/public-repo-guard/tests/body-policy.test.sh index b9269da..fa94bb0 100755 --- a/scripts/public-repo-guard/tests/body-policy.test.sh +++ b/scripts/public-repo-guard/tests/body-policy.test.sh @@ -78,6 +78,11 @@ expect 1 'host adjacent to the network address still blocks' \ GUARD_PRIVATE_REPOS=$'wave-gateway\nwave-transports\nagent-money' \ expect 1 'newline-separated GUARD_PRIVATE_REPOS still scans later names' \ 'The MOQ_JOIN_SECRET was added; wave-transports picks it up on deploy.' +# Regression: a CRLF-stored variable once glued an invisible \r to every name, so +# the built regex matched nothing and the gate fail-opened with no diagnostic. +GUARD_PRIVATE_REPOS=$'wave-gateway\r\nwave-transports\r\nagent-money\r' \ +expect 1 'CRLF-separated GUARD_PRIVATE_REPOS still scans every name' \ + 'The MOQ_JOIN_SECRET was added; wave-transports picks it up on deploy.' # --- must PASS (precision — these keep the gate deployable) ------------------- expect 0 'bare private-repo cross-reference' \