fix(pii-gate): fail closed when unconfigured, and stop losing matches to SIGPIPE (backend#1409)

[LukasWodka]

Code half of tracebloc/backend#1409. One file: .github/workflows/public-pii-gate.yml.

The gate whose only job is keeping customer and partner names out of public repos was reporting green on six public repos while having evaluated nothing. Three independent fail-open paths, all now refusals.

Defect 1 — unconfigured means inactive, and inactive looked like pass

:51-55 before (now :77-85): an unset or empty PII_DENYLIST printed a ::warning:: and exit 0. The secret does not exist org-wide, so pii-gate / pii-check had never compared a single term in any repo. The warning is invisible in the checks UI — the check was green.

Now exit 1. A guard that cannot read its own denylist must refuse, not pass. The same message covers the other way this goes empty: a caller that forgot secrets: inherit.

Defect 2 — the matcher discarded matches (SIGPIPE)

:77 before (now :169-191):

ifprintf'%s'"$HAYSTACK"| grep -iqF -- "$t";then

grep -q exits on its first match and closes the pipe, printf takes SIGPIPE, and under pipefail the pipeline returns 141 — so if read false and the hit was thrown away. The haystack is title + body + commit messages with the title first, so an early match on a large PR was the likely case: the gate was least reliable exactly when it mattered most.

Reproduction. The command in the ticket does not reproduce — see "corrections" below. What does, on the runner image's toolchain (bash 5.2.21, GNU grep 3.11, Ubuntu 24.04):

=== match on its own early line, then padding lines ===
size=100000 rc=141 PIPESTATUS=141 0
size=1000000 rc=141 PIPESTATUS=141 0
=== herestring, same data ===
herestring rc=0 PIPESTATUS=0
=== as the workflow uses it (set -uo pipefail, inside `if`) ===
MATCH LOST rc=141 PIPESTATUS=141 0
herestring: MATCH DETECTED

And end-to-end against the real script, denylisted term in the PR title, ~200KB haystack:

OLD code: "PII gate passed — no denylisted terms found." rc=0 <-- P0
NEW code: "::error::This PR's title, body, or commit ..." rc=1

Fixed by giving grep the haystack without a pipe. One deliberate deviation from the ticket's prescribed fix: the haystack is staged in a named temp file and grepped as grep -iqF -- "$t" "$HAYSTACK_FILE", rather than the suggested <<<"$HAYSTACK" herestring. A herestring is a temp file, so it fixes the SIGPIPE bug equally — but if bash cannot create it, the redirection fails, grep never runs, and the status is 1, the one code that means "no match". Reading a named file, grep itself returns 2 on any read failure, so "could not check" stays distinguishable from "checked, found nothing". It is also written once instead of once per term. grep exiting >1 is now a refusal; previously it was indistinguishable from a clean scan.

Defect 3 — the Compare API failure was swallowed

:66-67 before (now :112-152): 2>/dev/null || true turned a 404, a 403, a rate limit and a jq error all into an empty string. The gate then scanned title + body only and reported a pass, with no trace of the failed read.

Now: the response goes to a file, the exit status is checked, and a failure prints gh's own stderr and exits 1. Verified — old code on a 403: PII gate passed, rc=0; new code: ::error::Could not read commit messages for aaa...bbb, rc=1.

The "distinguish a real failure from an empty result" part is done with total_commits vs the returned commits[] length:

• equal → the whole PR is in hand, scan it
• 0/0 → a legitimately empty PR, scan title + body and pass (not a refusal — this is the one empty result that is genuinely clean)
• fewer than total_commits → we are holding a truncated haystack (the API caps commits[] at 250) and refuse rather than call it clean. Escape hatch is the audited override label.
• count missing or non-numeric → refuse. Guarded with a case glob because [ "$x" -lt "$y" ] on a non-number exits 2, and with no set -e that would have made the if false and fallen straight through to a pass.

Three more fail-opens found while walking the same error paths

Review on this epic keeps finding "a failure path that reports success", so I walked every path in the file:

1. A denylist of only commas/whitespace passed every PR having compared nothing — non-empty secret, zero usable terms after trimming: the exact fail-open of an unset secret wearing a populated secret's clothes. Now counted (CHECKED) and refused if zero.
2. The override label matched on substring — grep -q 'pii-gate-override' against the labels JSON, so a label like discuss-pii-gate-override-policy silently disarmed the gate. Now an exact jq match per label name. An unparseable payload yields "no override", which is the safe direction: the scan still runs.
3. for term in $DENYLIST glob-expanded terms containing * or ? into filenames. set -f added.
4. mktemp failure would have left sed reading stdin and hung the job; explicitly guarded.
5. A failed sed in the term-trim yielded an empty $t, which the empty-term guard then skipped silently — term unevaluated, gate still green. Now checked.

Items 4 and 5 and the named-file change above were found by re-walking my own error paths after the first commit, and are in the second commit.

The pass message now states how many terms were compared against how many commit messages, so a green check is distinguishable from a green check that did nothing. Counts only — the terms themselves are still never printed, so the gate cannot leak what it protects.

Verification

• actionlint — clean, whole repo (required check).
• shellcheck -s bash -S style on the extracted script — clean.
• 17-case harness with a stubbed gh (and a stubbed grep that exits 2) on Ubuntu 24.04 / bash 5.2.21 / GNU grep 3.11, covering every branch above. All 17 behave as intended; the four cases that are genuinely clean still pass, so this is not a blanket "refuse everything".

Corrections to the ticket

1. The repro command in #1409 / #1408 does not reproduce.haystack="SECRET$big" puts the match and all 200KB of padding on one line, so grep must read to EOF before it can match a line and printf never gets EPIPE. That form returns rc=0 PIPESTATUS=0 0 at 200KB, 1MB and 5MB, on both bash 3.2/BSD grep and bash 5.2/GNU grep. The bug is real, but it needs the match on an early line followed by a newline and enough further lines to fill the 64KB pipe buffer. Worth correcting because the stated repro would read as "not a bug".
2. The line numbers were not stale after all. I located everything by content expecting drift, but :51-55, :58, :66-67 and :77 are exact against origin/develop as well as @main — the file has not moved between the two. Noting it so the next person does not spend the same time re-deriving them.
3. :58, the override-label check, has the same pipe-to-grep -q shape as :77 and is not mentioned in the ticket. LABELS is small so it will not hit the pipe buffer in practice, but it was fixed along with the substring bug above.

Out of scope — the remaining half

Needs org-owner rights or a separate ticket, and this PR does nothing about it:

• Create the PII_DENYLIST org secret (--visibility all). Until this exists, this change turns the gate red on every PR in the six public repos — that is the intended, visible state for an unconfigured guard, but it means the secret should land promptly.
• Make pii-gate / pii-check a required check on the six public repos — per #1409 it is required nowhere, so even armed and red it currently blocks nothing.
• Add the public-pii-gate caller to public .github, the only public repo without one (#1408 P2).

Refs tracebloc/backend#1409, evidence in tracebloc/backend#1408 (P0 1).

🤖 Generated with Claude Code

---

Note

Medium Risk
This only changes a compliance CI gate, but behavior shifts from permissive green checks to hard failures—including every public PR until PII_DENYLIST is configured—so merge velocity depends on org secret and caller setup.

Overview
The public PII gate reusable workflow is reworked so it refuses instead of passing whenever it cannot complete a real scan (backend#1409). Missing or empty PII_DENYLIST, unreadable Compare API output, truncated commit lists (>250 commits), bad commit counts, mktemp/haystack staging failures, empty denylist after trimming, and grep exit codes other than 0/1 all now exit 1 with ::error:: messages instead of warnings or silent success.

Matching reliability fixes the pipeline where printf | grep -q could return 141 (SIGPIPE) under pipefail and drop hits on large PRs; the haystack is written once to a temp file and scanned with grep on the file. set -euo pipefail is explicit, denylist terms use grep … || rc=$? so “no match” does not kill the step under errexit, and set -f avoids glob expansion of special characters in terms.

Override and audit: the bypass label is an exactjq match on label names (not a substring grep). Success logs now state how many denylist terms and commit messages were checked.

^{Reviewed by Cursor Bugbot for commit 72153d0. Bugbot is set up for automated code reviews on this repo. Configure here.}

[LukasWodka]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 72153d0. Configure here.}