From b3976eb5f39c22b2712047acda8fa6f36e399096 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 21 Sep 2026 17:43:05 -0700 Subject: [PATCH 1/3] Add the nightly security audit: one Opus domain against SECURITY.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SECURITY.md states its guarantees as FAIL IF conditions so a machine can execute them, but nothing executed them. A single Opus agent now runs every one as a mechanical check with evidence, then reads the auth and infrastructure code adversarially, and publishes one report. The port of Dormouse's nightly audit drops everything that only exists because that repo has three domains: no orchestrator, no subagents, no fragment merge, no separate status file. The verdict is the report's own first line, and a `` sentinel is what says the report is finished — so an agent cut off after rewriting that line reads as inconclusive rather than as a pass. Three outcomes, not two: an audit that never decided files under the same label as a finding and holds the check run red, but says in its own issue that it is not a security finding. The push-to-main trigger is load-bearing rather than convenience. A consumer that vendors a packed tarball recovers the commit from dist/provenance.json; it can now confirm that commit was audited by reading the security-audit check run on it, which is why the job id and its name are both that string. Claude Code is invoked directly instead of through claude-code-action, which throws `Unsupported event type: push` — its context parser accepts only workflow_dispatch, repository_dispatch, schedule and workflow_run, and a step-level GITHUB_EVENT_NAME cannot stand in because the runner writes the real GITHUB_* values over a step's env. The CLI keeps the per-commit check run and makes CI and the local runner the same command over the same prompt files. Co-Authored-By: Claude Fable 5.1 --- .github/audit/_preamble.md | 62 +++++ .github/audit/security.md | 97 ++++++++ .github/workflows/security-audit.yml | 346 +++++++++++++++++++++++++++ .gitignore | 4 + PACKAGES.md | 2 +- SECURITY.md | 9 +- scripts/clamp-issue-body.mjs | 60 +++++ scripts/security-audit-local.sh | 64 +++++ 8 files changed, 641 insertions(+), 3 deletions(-) create mode 100644 .github/audit/_preamble.md create mode 100644 .github/audit/security.md create mode 100644 .github/workflows/security-audit.yml create mode 100644 scripts/clamp-issue-body.mjs create mode 100755 scripts/security-audit-local.sh diff --git a/.github/audit/_preamble.md b/.github/audit/_preamble.md new file mode 100644 index 0000000..db578f7 --- /dev/null +++ b/.github/audit/_preamble.md @@ -0,0 +1,62 @@ +# Shared preamble — the security auditor + +Read `SECURITY.md` first: it states what the packages guarantee, what the +consuming application owns, what is not defended, and the `FAIL IF` conditions +this run executes. Your scope is exactly the files listed under **Scope** in +your own prompt file. + +For each `FAIL IF` in your scope, run the mechanical check — a test suite, a +script, a grep, a file read — and record PASS or FAIL with concrete evidence: +file path and line number, or command output. A `FAIL IF` bullet asserts +several properties in one sentence; **each clause gets its own verdict and its +own evidence**. Never satisfy a bullet in bulk. + +Then do the qualitative pass described in your own file, rating findings +BLOCKER / WARNING / INFO. Report what you can prove. Use `UNVERIFIABLE` only +for a check you could not determine — a container that would not start, a +transient network error, or an area you ran out of room to reach — and say +which it was. It is never a substitute for a check you could have run. + +Where `SECURITY.md` says a risk is accepted ("What is not defended"), do not +re-report it as a finding — report only if the situation has changed or is +worse than described. + +Write your findings to the file named in your own prompt, and write them **as +you determine them — never buffered in your context for one write-up at the +end.** What is in that file is the whole of what the audit publishes: a run +that holds its results for a final write-up it never reaches publishes nothing. +Open the file before your first check: + +```sh +printf 'VERDICT: INCONCLUSIVE\n\n### FAIL IF results\n\n' > +``` + +Then append each check's line as you determine it, and each finding as you rate +it, under `### FAIL IF results` (one line per check) and `### Qualitative +findings` (severity-tagged). **Append; never rewrite the file whole.** + +**Its very first line must be literally `VERDICT: PASS`, `VERDICT: FAIL`, or +`VERDICT: INCONCLUSIVE`** — nothing else on that line. The reporting step reads +it, so it is the one part of your report a machine reads. It opens as +`INCONCLUSIVE` so a report you never finish fails closed on its own. Rewrite +that one line at the end, with Edit rather than `sed -i` (whose in-place flag +differs between GNU and BSD), then close the file: + +```sh +printf '\n\n' >> +``` + +**That sentinel is what tells the reporting step your report is finished**, so +write it last, once, and only when the verdict line above it is the one you +reached. A report that exists is a report still being filled in; a report +without the sentinel is read as inconclusive however its verdict line reads — +except a `VERDICT: FAIL`, which is a finding whether or not you finished. + +Return `FAIL` if any `FAIL IF` in your scope is violated or any qualitative +finding is BLOCKER. Otherwise `INCONCLUSIVE` if any check is `UNVERIFIABLE` or +unfinished; `PASS` only when every check was determined. + +Never print a secret value. Do not run `printenv` or `set -x`, and do not paste +the contents of a credential file into your report — report its mode and +location instead. This repository is public, and both your report and the +session transcript are world-readable. diff --git a/.github/audit/security.md b/.github/audit/security.md new file mode 100644 index 0000000..f0bdcf1 --- /dev/null +++ b/.github/audit/security.md @@ -0,0 +1,97 @@ +# Domain: security + +**Scope — this file, and no other:** + +- `SECURITY.md` + +**Output file:** `audit-report.md` + +This is a code-and-policy audit of what the packed packages guarantee to a +consuming application. Everything `SECURITY.md` assigns to the application — +TLS, the origin gate, page CSP, secret storage, database provisioning, provider +registration — is out of scope; so are `@pgstencil/stripe` and the original +code/link `Auth` exports, which carry no rules yet. + +## Mechanical pass + +Run every `FAIL IF` in `SECURITY.md`. Most of them are pinned by a named test, +so start by running the suites and reading their assertions, then read the code +each bullet names. + +```sh +pnpm db:verify # migrations apply and match the committed schema +pnpm test:unit # diagnostics allowlist and the other unit rules +pnpm test:better-auth # the session, CSRF, email-code, OAuth and linking suites +pnpm packages:verify # packs, installs into a clean project, checks provenance +``` + +`test:better-auth` and `packages:verify` need a working Docker daemon (see +`compose.yaml`). If the daemon is unreachable, every check those suites pin is +`UNVERIFIABLE` — say so and say why. **Never record a check as PASS on the +strength of a test you did not run**, and never quietly skip one. + +Each `Pinned by` line at the end of a `SECURITY.md` section names the tests for +that section. Confirm each named test still exists and still asserts what the +bullet claims: a test that was renamed, skipped, or reduced to a smoke check +leaves its bullet unenforced, and that is a finding even when the code is fine. + +The remaining bullets are read rather than run: + +- The `Continuous checks` bullets are grep-and-read over + `.github/workflows/check.yml` and `.github/workflows/security-audit.yml`. + Check the triggers, the job names, `persist-credentials: false`, the + permission blocks, the reporting step's verdict grammar, and that the + redaction step is present and covers every file the archive step publishes. +- The `Packed provenance` bullets are read over `scripts/build-packages.ts`, + `scripts/pack-packages.ts` and `scripts/verify-packages.ts`, with + `pnpm packages:verify` as the evidence. + +## Qualitative pass + +Be adversarial, and go past the `FAIL IF` list. Read +`packages/auth/src/better-auth*.ts`, `packages/pgstencil/src/*.ts`, and the +suites under `tests/` — including what they do _not_ cover. Ask at least: + +- **Can a caller influence where an OAuth callback lands, or reuse state across + applications?** Trace `oauthRequest` in `better-auth-oauth.ts` from the start + of the flow through the signed state cookie, the `oauth_flows` row, and the + atomic claim in `003_oauth_claims.sql`. Every parameter beyond the provider + name must come from server configuration; a state row must bind the browser, + the provider, and one application. +- **Does `verifiedOidc` cover every enabled provider?** Compare the providers in + `socialProviders` against the ones `verifiedOidc` and `providerSubject` in + `better-auth-email.ts` actually verify. A provider that is enabled but falls + through the verification switch signs users in on an unverified assertion. +- **Can a real user register an address in the `identity.pgstencil.invalid` + namespace?** Trace every writer of a user email — sign-up, linking, + `allowMissingEmail` account creation, and any profile update — against + `isIdentityEmail`. A reserved namespace that is only filtered on the way out + is not reserved. +- **Can a raw upstream error carrying a token reach diagnostics?** Follow a + provider or database error from where it is thrown to `diagnostic` and + `diagnosticError` in `packages/pgstencil/src/diagnostics.ts`, and to + `onAPIError`. Look for a path that stringifies an exception, a response body, + or a URL with a query string. +- **Is any rate-limit key derivable without the application secret?** Read + `keyed` and `consume` in `better-auth-security.ts`. A key an attacker can + compute lets them exhaust another address's budget, and one that varies with + something the caller controls lets them escape their own. +- **Can `@pgstencil/auth/better-auth-testing` be reached from a production + import graph?** Start at the package's `exports` map, then at + `examples/better-auth/src/worker.ts` and the esbuild `inject` that only test + bundles carry. A conditional import, a re-export, or a bundler that keeps the + module is the finding, not the presence of the file. +- **In `packages/pgstencil/src/database.ts` and `paths.ts`, is any shell or + Compose invocation built from a path or environment value a project could + control?** `PGSTENCIL_PROJECT_ROOT`, the derived Compose project name, the + `pgstencil.json` migrations path, and the bundled `compose.yaml` fallback all + cross into a child process. Look for string-built commands, unquoted + interpolation, and a path that escapes the project root. +- **What do the integration tests not cover?** Name the gaps. A guarantee whose + only evidence is that nobody has attacked it is worth saying out loud. +- **Have `SECURITY.md` and the code drifted?** Say which side is wrong. A bullet + that describes a control the code no longer has is a BLOCKER; a control the + code has that the file does not claim is an INFO. + +Rate every finding BLOCKER, WARNING or INFO, and give each one a file path and +line number. diff --git a/.github/workflows/security-audit.yml b/.github/workflows/security-audit.yml new file mode 100644 index 0000000..20fa3d8 --- /dev/null +++ b/.github/workflows/security-audit.yml @@ -0,0 +1,346 @@ +name: security-audit + +# Audits this repository against SECURITY.md: every `FAIL IF` run as a +# mechanical check with evidence, then an adversarial read of the code behind +# them. One agent, one domain, one report — there is nothing to orchestrate. +# +# The `push` trigger is load-bearing, not convenience. A consumer that vendors a +# packed tarball verifies the commit it vendored by reading this job's check run +# on that commit (`gh api repos/diffplug/pgstencil/commits//check-runs`), +# so the job id and its `name:` are both exactly `security-audit`. Renaming +# either one silently breaks every consumer's verification. + +on: + schedule: + - cron: '51 4 * * *' + workflow_dispatch: + push: + branches: [main] + +# No `concurrency` block. A superseded commit on `main` still needs its own +# verdict, because the check run a consumer reads is per commit. + +permissions: + contents: read # checkout + actions: read # deep-link this run's transcript artifact from the issue + issues: write # file, append to and close the failure issue + +jobs: + security-audit: + name: security-audit + runs-on: ubuntu-latest + # Generous: the audit runs the integration suite against real Postgres + # containers as evidence, then reads the auth package adversarially. + timeout-minutes: 40 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + fetch-depth: 1 + + - uses: pnpm/action-setup@ea17c68df8912ef543352723c149a84f56e3d413 # v6.1.0 + + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 24 + cache: pnpm + + - run: pnpm install --frozen-lockfile + + # Before the audit, not after: without the secret the agent never starts, + # and the reporting step below would otherwise file an issue that says + # only "no report". Writes a readable report instead of failing bare, so + # the issue names the missing secret. + - name: Verify CLAUDE_CODE_OAUTH_TOKEN is provisioned + env: + CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + run: | + [ -n "$CLAUDE_CODE_OAUTH_TOKEN" ] && exit 0 + # The backticks below are Markdown for the issue body, not command + # substitution. + # shellcheck disable=SC2016 + { + echo 'VERDICT: INCONCLUSIVE' + echo + echo 'The audit never ran: `CLAUDE_CODE_OAUTH_TOKEN` is not set as a repository secret.' + echo 'Add it under Settings -> Secrets and variables -> Actions, then re-run this workflow.' + echo + echo '' + } > audit-report.md + echo "::error::CLAUDE_CODE_OAUTH_TOKEN is not set; the audit cannot run." + exit 1 + + # Claude Code is invoked directly rather than through + # `anthropics/claude-code-action`, which cannot run on this workflow's + # load-bearing trigger: its `parseGitHubContext` switch has cases for + # `workflow_dispatch`, `repository_dispatch`, `schedule` and + # `workflow_run` only, and throws `Unsupported event type: push` on + # everything else. A step-level `GITHUB_EVENT_NAME` override does not + # help — the runner writes the real `GITHUB_*` values over a step's `env:` + # after evaluating it (actions/runner, `ScriptHandler.cs` -> "expose + # context to environment"). Invoking the CLI keeps the per-commit check + # run, and has the side benefit that CI and + # `scripts/security-audit-local.sh` run the same binary over the same + # prompt files, so they cannot drift. + # + # The version is pinned, and is the one the action installs the same way. + # The prompt is a pointer, never a copy: the content lives in + # `.github/audit/` so CI and the local runner cannot disagree. + - name: Audit against SECURITY.md + env: + CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + CLAUDE_CODE_VERSION: 2.1.278 + # Process-wide cap on a single Bash call. The default is two minutes — + # under the integration suite's runtime — and a call that hits the cap + # is moved to the background, so the agent would read a truncated + # result as the evidence for a `FAIL IF`. + BASH_DEFAULT_TIMEOUT_MS: '600000' + run: | + set -eo pipefail + curl -fsSL https://claude.ai/install.sh | bash -s -- "$CLAUDE_CODE_VERSION" + export PATH="$HOME/.local/bin:$PATH" + + # stdout is the transcript, not the log. The archive step publishes it + # after redaction; a public step log would carry raw `tool_result` + # bodies with no redaction pass in front of them. + # + # A nonzero exit is an annotation, not the verdict. The verdict is the + # report's own first line and its sentinel — an agent killed mid-run + # leaves no sentinel, which the reporting step reads as INCONCLUSIVE, + # and that is the failure mode the sentinel exists to catch. + # The backticks are Markdown in the prompt, not command substitution. + # shellcheck disable=SC2016 + claude -p 'Read `.github/audit/_preamble.md` and then `.github/audit/security.md`, and follow them exactly.' \ + --model opus \ + --allowed-tools "Read,Write,Edit,Bash,Grep,Glob" \ + --disallowed-tools "Task,Agent,Workflow" \ + --verbose --output-format stream-json \ + > "$RUNNER_TEMP/claude-execution-output.json" \ + || echo "::error::claude exited nonzero; the report below is whatever it had written." + + # Both sinks, not just the archive. `audit-report.md` is `cat` into a + # public issue, which outlives the artifact's 14 days, is indexed, and is + # emailed to subscribers — and the preamble asks for evidence as "command + # output", so it invites exactly the paste this guards against. + # + # Fail closed. `Archive audit transcript` below is `if: always()` and does + # not depend on this step's outcome, so a throwing redactor would + # otherwise ship the raw files: on any error, delete every sink instead. + # Deleted rather than truncated — `: >` has to open the file and so fails + # on exactly the unreadable file that made the redactor throw, whereas + # `rm` needs only the directory. A nonzero count is not routine; it means + # a secret reached a file in cleartext, so it surfaces as an annotation + # and the right response is to rotate. + # + # Keep every shell comment outside the single-quoted script — inside it, + # Node parses it as JavaScript and the step fails closed on every run, + # which `bash -n` does not catch. + - name: Redact secrets from agent output + if: always() + env: + CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + run: | + TRANSCRIPT="$RUNNER_TEMP/claude-execution-output.json" + node -e ' + const fs = require("fs"); + let hits = 0; + for (const p of process.argv.slice(1)) { + if (!fs.existsSync(p)) continue; + let s = fs.readFileSync(p, "utf8"); + for (const name of ["CLAUDE_CODE_OAUTH_TOKEN"]) { + const v = process.env[name]; + if (!v || v.length < 8) continue; + const parts = s.split(v); + hits += parts.length - 1; + s = parts.join("***"); + } + fs.writeFileSync(p, s); + } + if (hits > 0) { + console.log("::warning::Redacted " + hits + " literal secret occurrence(s) from agent output. A secret reached a file in cleartext — rotate CLAUDE_CODE_OAUTH_TOKEN."); + } else { + console.log("No literal secret occurrences found."); + } + ' "$TRANSCRIPT" audit-report.md \ + || { rm -f "$TRANSCRIPT" audit-report.md; exit 1; } + + # The transcript is the only record of what the audit actually did: its + # stdout never reaches the step log, and the runner is ephemeral. Uploaded + # unconditionally — a PASS transcript is the baseline you compare a bad + # one against. This repository is public, so the artifact is world + # readable; that is consistent with the reports already posted to public + # issues, and is why the redaction step above runs first. + # + # `if-no-files-found: warn` is load-bearing: a run that died before the + # agent started leaves neither file, and that must not fail the upload. + - name: Archive audit transcript + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: audit-transcript + path: | + ${{ runner.temp }}/claude-execution-output.json + audit-report.md + retention-days: 14 + if-no-files-found: warn + + - name: Surface result, file or close issue + if: always() + env: + GH_TOKEN: ${{ github.token }} + run: | + set -eo pipefail + + # Three outcomes, not two. PASS and FAIL are verdicts the audit + # reached; INCONCLUSIVE means it never reached one. Collapsing the + # third into FAIL files an identical issue for "this repository is + # insecure" and "the auditor stopped early", leaving no way to tell a + # real finding from a no-op run. It still exits non-zero and still + # files under the same label, so a later PASS closes it. + + # What the *file* said, parsed once and never mutated. Every note + # below is written about a condition rather than about a branch. + REPORT=audit-report.md + FINISHED=no + if [ -s "$REPORT" ]; then + # Exact match on the two arms that can be read as complete; a + # failure with an appended explanation is still a finding, so only + # FAIL matches as a prefix. + case "$(head -n1 "$REPORT")" in + 'VERDICT: PASS') FILE_VERDICT=PASS ;; + 'VERDICT: FAIL'*) FILE_VERDICT=FAIL ;; + 'VERDICT: INCONCLUSIVE') FILE_VERDICT=INCONCLUSIVE ;; + *) FILE_VERDICT=UNREADABLE ;; + esac + # A verdict line is not a finished report. The agent appends + # findings as it determines them and writes the sentinel last, so a + # report without one belongs to a run that was cut off — and its + # first line may already have been rewritten to `VERDICT: PASS` in + # the moment before it died. Last non-blank line, not `tail -n1`: a + # trailing blank line after the sentinel still ends a report. + if [ "$(sed -e '/^[[:space:]]*$/d' "$REPORT" | tail -n1)" = "" ]; then + FINISHED=yes + fi + else + FILE_VERDICT=MISSING + fi + + # Escalation, in ONE place, from what was parsed above. PASS is the + # only arm that has to be earned: an exact verdict line *and* a + # finished report. Everything else that is not an outright finding is + # inconclusive. + if [ "$FILE_VERDICT" = "FAIL" ]; then + STATUS=FAIL + elif [ "$FILE_VERDICT" = "PASS" ] && [ "$FINISHED" = "yes" ]; then + STATUS=PASS + else + STATUS=INCONCLUSIVE + fi + + DATE=$(date -u +%Y-%m-%dT%H:%MZ) + RUN_URL="https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" + + # Idempotent label creation; ignore "already exists" errors. + gh label create security-audit-failure \ + --color B60205 --description "Security audit failure" 2>/dev/null || true + + if [ "$STATUS" = "PASS" ]; then + # Auto-close any open audit-failure issues so the tracker reflects + # the live state. + for n in $(gh issue list --label security-audit-failure \ + --state open --json number --jq '.[].number'); do + gh issue close "$n" --comment "Audit passed at $DATE. [Run]($RUN_URL)" + done + echo "Audit passed." + exit 0 + fi + + # Deep-link the transcript so the issue points at the evidence rather + # than at a run page the reader has to dig through. The id lookup + # needs `actions: read`, which this job holds. Left empty when there + # is no artifact — a run cancelled on `timeout-minutes` never writes + # one, and those are exactly the runs a link to the run page would + # dead-end on, so both consumers below are gated rather than falling + # back. + ART_ID=$(gh api "repos/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID/artifacts" \ + --jq '.artifacts[] | select(.name == "audit-transcript") | .id' \ + 2>/dev/null | head -n1 || true) + if [ -n "$ART_ID" ]; then + TRANSCRIPT_URL="$RUN_URL/artifacts/$ART_ID" + else + TRANSCRIPT_URL="" + fi + + # One note per condition that HOLDS, never one block per combination + # of conditions. Prose proportional to combinations cannot be kept + # correct by patching combinations: each time a gate widens, new + # combinations become reachable and the arm that catches them + # describes a different failure. Adding a fifth condition later means + # adding one note, and it cannot make any existing note wrong, because + # no note claims anything about the others. + NOTES=$(mktemp) + if [ "$FILE_VERDICT" = "MISSING" ]; then + echo "::warning::No audit-report.md was produced." + echo "- **The audit produced no report.** \`audit-report.md\` was absent or empty, so the run ended before writing anything. The redactor deletes the report and the transcript when it throws; check that step's result as well as this run's \`audit-transcript\` artifact." >> "$NOTES" + fi + if [ "$FILE_VERDICT" = "UNREADABLE" ]; then + echo "::warning::audit-report.md has no VERDICT line." + echo "- **The audit's verdict could not be read.** The first line of \`audit-report.md\` is not an exact \`VERDICT: PASS\`, \`VERDICT: FAIL\`, or \`VERDICT: INCONCLUSIVE\`. The run did report; its verdict is unconfirmed." >> "$NOTES" + fi + if [ "$FILE_VERDICT" = "INCONCLUSIVE" ]; then + echo "::warning::The audit could not determine every check." + echo "- **The audit could not determine every check.** Read its \`UNVERIFIABLE\` lines; each names a check that was reached but not determined, and those do not count as passing." >> "$NOTES" + fi + if [ "$FILE_VERDICT" != "MISSING" ] && [ "$FINISHED" != "yes" ]; then + echo "::warning::audit-report.md has no completion sentinel; the audit was cut off mid-report." + echo "- **The audit was cut off mid-report.** It never wrote its \`\` sentinel, so what follows is what it had recorded when it stopped, and its verdict line covers less than it appears to. The findings it did write are still findings." >> "$NOTES" + fi + + if [ "$STATUS" = "FAIL" ]; then + TITLE="[security-audit] FAIL on $(date -u +%Y-%m-%d)" + HEADLINE="Audit failed at $DATE." + else + TITLE="[security-audit] INCONCLUSIVE on $(date -u +%Y-%m-%d)" + HEADLINE="Audit reached no usable verdict at $DATE. This is not a security finding: the run ended without deciding." + fi + + { + LINKS="[Run]($RUN_URL)" + [ -n "$TRANSCRIPT_URL" ] && LINKS="$LINKS · [Transcript]($TRANSCRIPT_URL)" + echo "$HEADLINE $LINKS" + echo + if [ -s "$NOTES" ]; then + cat "$NOTES" + echo + fi + if [ -s "$REPORT" ]; then + cat "$REPORT" + fi + } > audit-comment.md + rm -f "$NOTES" + + # Truncate before posting, non-fatally: GitHub rejects an over-long + # body outright, which loses the whole finding. + node scripts/clamp-issue-body.mjs audit-comment.md \ + --note "The untruncated \`audit-report.md\` is in this run's \`audit-transcript\` artifact${TRANSCRIPT_URL:+ ([download]($TRANSCRIPT_URL))}." \ + || echo "clamp-issue-body.mjs failed; posting audit-comment.md unclamped." >&2 + + EXISTING=$(gh issue list --label security-audit-failure \ + --state open --json number --jq '.[0].number' || true) + if [ -n "$EXISTING" ]; then + gh issue comment "$EXISTING" --body-file audit-comment.md + # Upward only. FAIL is the ceiling: an inconclusive run must not + # relabel an issue that already carries real findings, and the title + # never needs walking back down because a PASS closes the issue + # outright. + if [ "$STATUS" = "FAIL" ]; then + gh issue edit "$EXISTING" --title "$TITLE" + fi + echo "Appended $STATUS to issue #$EXISTING" + else + gh issue create \ + --title "$TITLE" \ + --label security-audit-failure \ + --body-file audit-comment.md + fi + exit 1 diff --git a/.gitignore b/.gitignore index f4a7644..fcf83e3 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,7 @@ packages/*/LICENSE .wrangler/ .dev.vars* + +# Written by the security audit; see .github/workflows/security-audit.yml. +audit-report.md +audit-comment.md diff --git a/PACKAGES.md b/PACKAGES.md index d31dc4a..0f85d0d 100644 --- a/PACKAGES.md +++ b/PACKAGES.md @@ -6,7 +6,7 @@ Run `pnpm packages:pack` to produce the three archives in `dist/packages`. They Copy the tarballs to a consumer's `vendor/` directory, depend on them using `file:` paths, and override all three package names to those same paths in the consumer's pnpm configuration. The override for `pgstencil` ensures auth and billing's peer also resolves locally. `packages:pack` refuses a modified build input tree, because an untracked file under `migrations` would otherwise ship in the archive; `--allow-dirty` overrides that for a local experiment. Commit the archives and lockfile together for a reproducible temporary distribution. Once public npm is configured, replace these paths with exact registry versions and remove the overrides. -Every archive carries `package/dist/provenance.json`, holding the 40-character `commit` it was packed from plus `"dirty": true` when `--allow-dirty` packed a modified tree. Read it without unpacking the archive: `tar -xOf vendor/pgstencil-0.1.0.tgz package/dist/provenance.json`. That path is the contract; nothing else in the archive identifies its source, since npm's `gitHead` is absent from a `pnpm pack` of a private package. A consumer should record the commit it vendored, re-derive it from the archive on every build, and refuse an archive whose commit differs or that is marked dirty. `pnpm packages:verify` makes the same assertion against this checkout's `HEAD`, so a stale archive fails CI here. +Every archive carries `package/dist/provenance.json`, holding the 40-character `commit` it was packed from plus `"dirty": true` when `--allow-dirty` packed a modified tree. Read it without unpacking the archive: `tar -xOf vendor/pgstencil-0.1.0.tgz package/dist/provenance.json`. That path is the contract; nothing else in the archive identifies its source, since npm's `gitHead` is absent from a `pnpm pack` of a private package. A consumer should record the commit it vendored, re-derive it from the archive on every build, and refuse an archive whose commit differs or that is marked dirty. `pnpm packages:verify` makes the same assertion against this checkout's `HEAD`, so a stale archive fails CI here. Having recovered the commit, a consumer confirms it was audited by reading its check run — `gh api repos/diffplug/pgstencil/commits//check-runs` lists `security-audit`, which succeeds only when that commit's audit against [SECURITY.md](SECURITY.md) reached `VERDICT: PASS`. `pnpm packages:verify` builds, packs, installs into an unrelated temporary pnpm project, checks TypeScript declarations, and runs an email login and required-card trial against a real cloned database. The project declares each peer at the version this workspace tests, and fails on an unmet peer. It shares this repository's Docker service state, but loads all code and SQL from installed archives. The consumer directory is printed for inspection. diff --git a/SECURITY.md b/SECURITY.md index d210012..443cd18 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -19,7 +19,7 @@ Pinned by `integration/better-auth.test.ts`: `email policy: secret-keyed codes, - **FAIL IF** a state-changing POST is accepted without an exact `Origin` match, a signature-verified CSRF cookie, a matching `X-CSRF-Token` and an `application/json` body; inspect `protectAuth` in `better-auth-security.ts`. - **FAIL IF** an upstream Better Auth route outside the read/write allowlist answers with anything but 404, or a provider callback accepts a method other than GET and Apple's `form_post` relay; inspect `protectAuth`. - **FAIL IF** a response leaves without `Cache-Control: no-store`, `Referrer-Policy: no-referrer`, `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY` and a CSP denying framing, inline script and third-party sources; inspect `protectAuth`. -- **FAIL IF** the app accepts a non-canonical origin, a secret under 32 characters, or a success/error path off the application origin; inspect `authOptions` in `better-auth.ts`. `audit`. +- **FAIL IF** the app accepts a non-canonical origin, a secret under 32 characters, or a success/error path off the application origin; inspect `authOptions` in `better-auth.ts`. Pinned by `integration/better-auth.test.ts`: `auth surface: explicit CSRF, exact origin, security headers and disabled unused endpoints`, `Better Auth email rejects expired codes and cross-origin sign-in`; `integration/better-auth-oauth.test.ts`: `Better Auth OAuth: Apple form_post relay, wrong browser, mismatched provider and expired state`. @@ -74,7 +74,12 @@ Pinned by `pnpm packages:verify` in `.github/workflows/check.yml`. ## Continuous checks -- **FAIL IF** `.github/workflows/check.yml` stops running `db:verify`, `test` and `packages:verify` on pushes to `main` and on every pull request, drops `persist-credentials: false`, or grants any permission beyond `contents: read`; inspect `.github/workflows/check.yml`. `audit`. +- **FAIL IF** `.github/workflows/check.yml` stops running `db:verify`, `test` and `packages:verify` on pushes to `main` and on every pull request, drops `persist-credentials: false`, or grants any permission beyond `contents: read`; inspect `.github/workflows/check.yml`. +- **FAIL IF** `.github/workflows/security-audit.yml` is missing or disabled, loses its `push` to `main` trigger or its `security-audit` job name — a consumer reads that check run by name — lets the reporting step treat anything but an exact `VERDICT: PASS` line above a `` sentinel as passing, or drops the step that redacts secrets from the report and the transcript; inspect `.github/workflows/security-audit.yml`. + +## How this file is checked + +`.github/workflows/security-audit.yml` executes this file nightly at 04:51 UTC, on demand, and on every push to `main`. One agent runs every `FAIL IF` above as a mechanical check, then reads the code behind them adversarially, following `.github/audit/_preamble.md` and `.github/audit/security.md`; `scripts/security-audit-local.sh` runs the same prompts against the same files locally. A run that does not reach an exact `VERDICT: PASS` files or appends to an open issue labelled `security-audit-failure` and exits non-zero, and a later pass closes it. Every run archives the report and the session transcript as the `audit-transcript` artifact — public, like this repository, and kept 14 days. ## Reporting a vulnerability diff --git a/scripts/clamp-issue-body.mjs b/scripts/clamp-issue-body.mjs new file mode 100644 index 0000000..a8f9f45 --- /dev/null +++ b/scripts/clamp-issue-body.mjs @@ -0,0 +1,60 @@ +#!/usr/bin/env node +/** + * Truncate an issue/comment body in place so GitHub cannot reject it as too + * long. A rejection is not a truncation: `gh issue create` fails outright and + * the whole finding reaches nobody. Called by + * `.github/workflows/security-audit.yml` -> "Surface result, file or close + * issue"; ported from diffplug/dormouse. + * + * Usage: node scripts/clamp-issue-body.mjs [--note ""] + */ + +import { readFileSync, writeFileSync } from 'node:fs'; +import { pathToFileURL } from 'node:url'; + +/** Well under GitHub's 65536-character ceiling, and past what anyone reads. */ +export const BODY_LIMIT = 32_000; + +/** Truncate `body` to `BODY_LIMIT`, keeping the head. Unchanged if it fits. */ +export function clampIssueBody(body, note = '') { + if (body.length <= BODY_LIMIT) return body; + const footer = `\n\n---\n\n_Truncated to fit: the full body is ${body.length} characters.${note ? ` ${note}` : ''}_\n`; + let kept = body.slice(0, Math.max(0, BODY_LIMIT - footer.length)); + // Cut at a line boundary, unless that would throw away most of what we kept. + const lastNewline = kept.lastIndexOf('\n'); + if (lastNewline > kept.length * 0.8) kept = kept.slice(0, lastNewline); + // Final slice covers a `--note` long enough to blow the budget by itself. + return `${kept.trimEnd()}${footer}`.slice(0, BODY_LIMIT); +} + +function main(argv) { + const args = argv.slice(2); + const noteAt = args.indexOf('--note'); + const note = noteAt === -1 ? '' : (args.splice(noteAt, 2)[1] ?? ''); + const file = args[0]; + if (!file) { + console.error('usage: clamp-issue-body.mjs [--note ""]'); + process.exit(2); + } + + const original = readFileSync(file, 'utf8'); + const clamped = clampIssueBody(original, note); + if (clamped === original) { + console.log( + `${file}: ${original.length} characters, within ${BODY_LIMIT}.`, + ); + return; + } + writeFileSync(file, clamped); + console.log( + `${file}: truncated ${original.length} -> ${clamped.length} characters.`, + ); +} + +// Only when run as the CLI, so the self-test can import the pure function. +if ( + process.argv[1] && + import.meta.url === pathToFileURL(process.argv[1]).href +) { + main(process.argv); +} diff --git a/scripts/security-audit-local.sh b/scripts/security-audit-local.sh new file mode 100755 index 0000000..f0cb8cc --- /dev/null +++ b/scripts/security-audit-local.sh @@ -0,0 +1,64 @@ +#!/bin/bash +# +# Run the security audit locally, against the same prompt files CI uses +# (`.github/audit/`). Nothing is duplicated here: if this and +# `.github/workflows/security-audit.yml` ever disagree, it is a bug in one of +# them, not a drift in the prompts. +# +# The report lands in ./audit-report.md, which .gitignore covers. +# +# Usage: +# scripts/security-audit-local.sh + +set -euo pipefail + +cd "$(dirname "$0")/.." +AUDIT_DIR=.github/audit +OUT=audit-report.md + +if ! command -v claude >/dev/null 2>&1; then + echo "error: the \`claude\` CLI is not on PATH." >&2 + exit 1 +fi + +for f in _preamble security; do + [ -f "$AUDIT_DIR/$f.md" ] || { echo "error: missing $AUDIT_DIR/$f.md" >&2; exit 1; } +done + +echo "==> security -> $OUT (--model opus)" +rm -f "$OUT" +# Same model and the same tool grants as CI. Local and CI must agree here, or a +# local run stops being a rehearsal of the nightly. +if ! claude -p "$(cat "$AUDIT_DIR/_preamble.md"; echo; cat "$AUDIT_DIR/security.md")" \ + --model opus \ + --allowed-tools "Read,Write,Edit,Bash,Grep,Glob" \ + --disallowed-tools "Task,Agent,Workflow"; then + echo "==> the auditor process failed" >&2 + exit 1 +fi + +if [ ! -s "$OUT" ]; then + echo "==> no $OUT was produced — in CI that is an INCONCLUSIVE audit, not a FAIL" >&2 + exit 1 +fi + +# Same sentinel CI reads, for the same reason: the agent appends findings as it +# determines them, so a report without its last line is one that stopped early — +# and its first line may already say PASS. Last non-blank line, not `tail -n1`: +# a trailing blank line after the sentinel still ends a finished report. +if [ "$(sed -e '/^[[:space:]]*$/d' "$OUT" | tail -n1)" != "" ]; then + echo "==> the audit was cut off before finishing $OUT — findings kept, its verdict line covers less than it appears to" >&2 + case "$(head -n1 "$OUT")" in 'VERDICT: FAIL'*) echo "==> the audit reports FAIL" >&2 ;; esac + exit 1 +fi + +# The same grammar CI applies in .github/workflows/security-audit.yml, and for +# the same reason: a failure with an appended explanation is still a finding, so +# only the PASS arm matches exactly. Drifting from CI here would report a real +# finding as an unreadable report. +case "$(head -n1 "$OUT")" in + 'VERDICT: PASS') echo "==> wrote $OUT"; exit 0 ;; + 'VERDICT: FAIL'*) echo "==> the audit reports FAIL" >&2; exit 1 ;; + 'VERDICT: INCONCLUSIVE') echo "==> the audit could not determine every check" >&2; exit 1 ;; + *) echo "==> the audit produced no readable verdict" >&2; exit 1 ;; +esac From 09a765c903b3eccfee9640c4acc08abbee73c47b Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 21 Sep 2026 17:43:33 -0700 Subject: [PATCH 2/3] Pin the audit's reporting grammar and redactor with tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reporting step is the only thing that decides whether a commit's security-audit check run goes green, and it was enforced by nobody: a `VERDICT: PASS` widened to a prefix match, a dropped sentinel test, or a sink removed from the redactor's argument list would all have passed review and shipped. `pnpm test:scripts` extracts the shipped shell out of the workflow by indentation and runs it — a renamed step or a moved block scalar throws rather than quietly handing the test the rest of the file. The sinks the redactor must cover are read from the archive step's own path list, so a file that starts being published without starting to be redacted fails here instead. The local runner is covered against the same grammar, and `.github/audit/_preamble.md` is pinned as the producer of the sentinel every reader waits for, since nothing else writes that literal. Verified by mutation: widening the PASS match, dropping the sentinel requirement, and removing a sink from the redactor each turn the suite red. `clamp-issue-body.mjs` and its self-test come from Dormouse unchanged but for their pointers; GitHub rejects an over-long issue body outright, which loses the whole finding rather than truncating it. Co-Authored-By: Claude Fable 5.1 --- .github/workflows/check.yml | 1 + SECURITY.md | 4 +- package.json | 3 +- scripts/clamp-issue-body.test.mjs | 117 ++++++++ scripts/security-audit.test.mjs | 479 ++++++++++++++++++++++++++++++ 5 files changed, 602 insertions(+), 2 deletions(-) create mode 100644 scripts/clamp-issue-body.test.mjs create mode 100644 scripts/security-audit.test.mjs diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 1c162bf..56deaea 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -25,6 +25,7 @@ jobs: - run: pnpm format:check - run: pnpm db:verify - run: pnpm typecheck + - run: pnpm test:scripts - run: pnpm test - run: pnpm --filter @pgstencil/example-workers build - run: pnpm packages:verify diff --git a/SECURITY.md b/SECURITY.md index 443cd18..c655c2d 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -74,9 +74,11 @@ Pinned by `pnpm packages:verify` in `.github/workflows/check.yml`. ## Continuous checks -- **FAIL IF** `.github/workflows/check.yml` stops running `db:verify`, `test` and `packages:verify` on pushes to `main` and on every pull request, drops `persist-credentials: false`, or grants any permission beyond `contents: read`; inspect `.github/workflows/check.yml`. +- **FAIL IF** `.github/workflows/check.yml` stops running `db:verify`, `test:scripts`, `test` and `packages:verify` on pushes to `main` and on every pull request, drops `persist-credentials: false`, or grants any permission beyond `contents: read`; inspect `.github/workflows/check.yml`. - **FAIL IF** `.github/workflows/security-audit.yml` is missing or disabled, loses its `push` to `main` trigger or its `security-audit` job name — a consumer reads that check run by name — lets the reporting step treat anything but an exact `VERDICT: PASS` line above a `` sentinel as passing, or drops the step that redacts secrets from the report and the transcript; inspect `.github/workflows/security-audit.yml`. +Pinned by `pnpm test:scripts`, which runs the shipped reporting and redaction shell rather than a copy: `scripts/security-audit.test.mjs`. + ## How this file is checked `.github/workflows/security-audit.yml` executes this file nightly at 04:51 UTC, on demand, and on every push to `main`. One agent runs every `FAIL IF` above as a mechanical check, then reads the code behind them adversarially, following `.github/audit/_preamble.md` and `.github/audit/security.md`; `scripts/security-audit-local.sh` runs the same prompts against the same files locally. A run that does not reach an exact `VERDICT: PASS` files or appends to an open issue labelled `security-audit-failure` and exits non-zero, and a later pass closes it. Every run archives the report and the session transcript as the `audit-transcript` artifact — public, like this repository, and kept 14 days. diff --git a/package.json b/package.json index ddf273d..4f46011 100644 --- a/package.json +++ b/package.json @@ -13,11 +13,12 @@ "test": "vitest run", "test:watch": "vitest", "test:unit": "vitest run tests/unit", + "test:scripts": "node --test scripts/*.test.mjs", "snapshot:update": "node --import tsx scripts/update-snapshots.ts", "typecheck": "tsc --noEmit", "format": "prettier --write .", "format:check": "prettier --check .", - "check": "pnpm format:check && pnpm typecheck && pnpm test", + "check": "pnpm format:check && pnpm typecheck && pnpm test:scripts && pnpm test", "db:status": "tsx scripts/db.ts status", "db:validate": "tsx scripts/db.ts validate", "db:migrate": "tsx scripts/db.ts migrate", diff --git a/scripts/clamp-issue-body.test.mjs b/scripts/clamp-issue-body.test.mjs new file mode 100644 index 0000000..1be4a42 --- /dev/null +++ b/scripts/clamp-issue-body.test.mjs @@ -0,0 +1,117 @@ +#!/usr/bin/env node +/** + * Proves `clamp-issue-body.mjs` produces a body GitHub will accept. + * + * The regression, ported with the script from diffplug/dormouse: a security + * audit reached `VERDICT: FAIL`, composed a 68 KB comment, and `gh issue + * create` rejected it as too long — so the finding reached no issue and no + * comment. + */ + +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { BODY_LIMIT, clampIssueBody } from './clamp-issue-body.mjs'; + +const failures = []; + +function check(name, condition, detail = '') { + if (condition) return; + failures.push(detail ? `${name}\n ${detail}` : name); +} + +/** A body shaped like the audit comment that was rejected: head, then bulk. */ +function auditShapedBody(totalLength) { + const head = [ + 'Audit failed at 2026-08-29. [Run](https://example.invalid/run) · [Transcript](https://example.invalid/art)', + '', + '- **The audit was cut off mid-report.** It never wrote its `` sentinel.', + '', + ].join('\n'); + const filler = `${'finding detail line'.padEnd(72, '.')}\n`; + return ( + head + filler.repeat(Math.ceil((totalLength - head.length) / filler.length)) + ); +} + +// The regression: a 68 KB audit body comes back postable, head intact. +{ + const body = auditShapedBody(68_424); + const clamped = clampIssueBody( + body, + 'The full report is in the run artifact.', + ); + check( + 'a clamped body fits the limit', + clamped.length <= BODY_LIMIT, + `got ${clamped.length}`, + ); + check('the head survives', clamped.startsWith('Audit failed at 2026-08-29.')); + check( + 'the condition note survives', + clamped.includes('**The audit was cut off mid-report.**'), + ); + check( + 'the reader is told it was truncated', + clamped.includes('Truncated to fit'), + ); + check( + 'the caller-supplied pointer survives', + clamped.includes('The full report is in the run artifact.'), + ); + check('re-clamping is a no-op', clampIssueBody(clamped, 'n.') === clamped); +} + +// A body under the limit is left byte-identical, so the workflow can run this +// unconditionally. +{ + const body = auditShapedBody(1_000); + check('a short body is untouched', clampIssueBody(body, 'ignored') === body); +} + +// A `--note` long enough to blow the budget by itself must still fit. +{ + const clamped = clampIssueBody('x'.repeat(70_000), 'n'.repeat(BODY_LIMIT)); + check( + 'an over-long note still fits', + clamped.length <= BODY_LIMIT, + `got ${clamped.length}`, + ); +} + +// The CLI rewrites the file in place — what the workflows actually call. +{ + const dir = mkdtempSync(join(tmpdir(), 'clamp-issue-body-')); + try { + const file = join(dir, 'audit-comment.md'); + writeFileSync(file, auditShapedBody(68_424)); + const cli = fileURLToPath( + new URL('./clamp-issue-body.mjs', import.meta.url), + ); + execFileSync('node', [cli, file, '--note', 'See the artifact.'], { + stdio: 'pipe', + }); + const written = readFileSync(file, 'utf8'); + check( + 'the CLI rewrites the file under the limit', + written.length <= BODY_LIMIT, + `got ${written.length}`, + ); + check( + 'the CLI passes --note through', + written.includes('See the artifact.'), + ); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +if (failures.length > 0) { + console.error(`clamp-issue-body-selftest: ${failures.length} failure(s)\n`); + for (const failure of failures) console.error(` - ${failure}`); + process.exit(1); +} +console.log('clamp-issue-body-selftest: OK'); diff --git a/scripts/security-audit.test.mjs b/scripts/security-audit.test.mjs new file mode 100644 index 0000000..6431227 --- /dev/null +++ b/scripts/security-audit.test.mjs @@ -0,0 +1,479 @@ +/** + * Pins the security audit's reporting grammar and its redactor by executing the + * shipped shell, not a copy of it. See `.github/workflows/security-audit.yml` + * and `SECURITY.md` -> "Continuous checks". + */ + +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { + copyFileSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { test } from 'node:test'; +import { fileURLToPath } from 'node:url'; + +const repo = fileURLToPath(new URL('..', import.meta.url)); +const workflow = readFileSync( + join(repo, '.github/workflows/security-audit.yml'), + 'utf8', +); + +/** The one literal every reader waits for. The producer copy in + * `.github/audit/_preamble.md` is pinned against it below. */ +const SENTINEL = ''; + +/** The index of the step named `stepName`, and the column its `- ` sits in. */ +function findStep(lines, stepName) { + const at = lines.findIndex((line) => line.trim() === `- name: ${stepName}`); + if (at < 0) throw new Error(`missing workflow step: ${stepName}`); + return { at, indent: lines[at].indexOf('- ') }; +} + +/** + * The lines of the block scalar introduced by `key` inside the named step, + * dedented. Scanned by indentation rather than matched by a regex, which would + * run past the end of the block and hand a test the rest of the file. A step + * that has been renamed, or a key that has moved, must fail loudly here — + * silently returning the next step's body, or an empty string, would leave a + * test passing against nothing. + */ +function workflowBlock(source, stepName, key) { + const lines = source.split('\n'); + const step = findStep(lines, stepName); + let keyAt = -1; + for (let i = step.at + 1; i < lines.length; i += 1) { + if ( + lines[i].trim().startsWith('- ') && + lines[i].indexOf('- ') === step.indent + ) + break; + if (lines[i].trim() === `${key}: |`) { + keyAt = i; + break; + } + } + if (keyAt < 0) + throw new Error( + `missing \`${key}: |\` block in workflow step: ${stepName}`, + ); + const bodyIndent = lines[keyAt].search(/\S/) + 2; + const body = []; + for (const line of lines.slice(keyAt + 1)) { + if (line.trim() !== '' && line.search(/\S/) < bodyIndent) break; + body.push(line.slice(bodyIndent)); + } + return body.join('\n'); +} + +/** The shell body of one workflow step, as shipped. */ +const workflowRunBlock = (source, stepName) => + workflowBlock(source, stepName, 'run'); + +/** + * Every file the archive step publishes, as the redactor must see them. Read + * from the workflow rather than listed here, so a sink added to the artifact + * without being added to the redactor fails this suite instead of shipping. + */ +const publishedSinks = workflowBlock( + workflow, + 'Archive audit transcript', + 'path', +) + .split('\n') + .map((line) => line.trim().replace('${{ runner.temp }}/', '')) + .filter(Boolean); + +function tempDir(t, prefix) { + const dir = mkdtempSync(join(tmpdir(), prefix)); + t.after(() => rmSync(dir, { recursive: true, force: true })); + return dir; +} + +function fixture(t) { + const dir = tempDir(t, 'pgstencil-audit-'); + mkdirSync(join(dir, 'bin')); + mkdirSync(join(dir, 'scripts')); + copyFileSync( + join(repo, 'scripts/clamp-issue-body.mjs'), + join(dir, 'scripts/clamp-issue-body.mjs'), + ); + const env = { + ...process.env, + PATH: `${join(dir, 'bin')}:${process.env.PATH}`, + RUNNER_TEMP: dir, + GITHUB_REPOSITORY: 'fixture/repo', + GITHUB_RUN_ID: '123', + CLAUDE_CODE_OAUTH_TOKEN: 'fixture-oauth-token', + }; + return { dir, env }; +} + +function stub(dir, name, source) { + writeFileSync(join(dir, 'bin', name), `#!${process.execPath}\n${source}\n`, { + mode: 0o755, + }); +} + +/** A stub `gh` that records every call; `openIssue` decides what `issue list` finds. */ +function ghStub(dir, openIssue = true) { + stub( + dir, + 'gh', + ` + const fs = require('node:fs'); + const args = process.argv.slice(2); + fs.appendFileSync('gh-calls.jsonl', JSON.stringify(args) + '\\n'); + if (args[0] === 'issue' && args[1] === 'list') process.stdout.write(${openIssue ? "'23\\n'" : "''"}); + `, + ); +} + +function ghCalls(dir) { + const file = join(dir, 'gh-calls.jsonl'); + if (!existsSync(file)) return []; + return readFileSync(file, 'utf8') + .trim() + .split('\n') + .filter(Boolean) + .map(JSON.parse); +} + +const reporting = workflowRunBlock( + workflow, + 'Surface result, file or close issue', +); + +/** `report: null` means the file is never written at all. */ +const cases = [ + { + name: 'a finished PASS closes the open issues', + report: `VERDICT: PASS\nEvidence\n\n${SENTINEL}\n`, + expected: 'PASS', + }, + // A trailing blank line after the sentinel still ends a finished report; read + // with an exact `tail -n1` this PASS would be reported as cut off. + { + name: 'a trailing blank line still ends a report', + report: `VERDICT: PASS\nEvidence\n\n${SENTINEL}\n\n\n`, + expected: 'PASS', + }, + // Cut off between rewriting the verdict line and writing the sentinel, a run + // reads as a clean PASS on line 1. Without the sentinel guard that is a green + // check run over a report that stopped early. + { + name: 'PASS without a sentinel is a cut-off audit', + report: 'VERDICT: PASS\nEvidence\n', + expected: 'INCONCLUSIVE', + notes: ['cut off mid-report'], + }, + { + name: 'FAIL with an appended explanation is still a finding', + report: `VERDICT: FAIL — credential leaked\nEvidence\n\n${SENTINEL}\n`, + expected: 'FAIL', + notes: [], + }, + { + name: 'a cut-off FAIL is still a finding', + report: 'VERDICT: FAIL\nEvidence\n', + expected: 'FAIL', + notes: ['cut off mid-report'], + }, + { + name: 'INCONCLUSIVE is not a security finding', + report: `VERDICT: INCONCLUSIVE\nEvidence\n\n${SENTINEL}\n`, + expected: 'INCONCLUSIVE', + notes: ['could not determine every check'], + }, + { + name: 'a missing report is inconclusive', + report: null, + expected: 'INCONCLUSIVE', + notes: ['produced no report'], + }, + { + name: 'an empty report is inconclusive', + report: '', + expected: 'INCONCLUSIVE', + notes: ['produced no report'], + }, + { + name: 'an unreadable verdict is not a pass', + report: `Summary\nVERDICT: PASS\n\n${SENTINEL}\n`, + expected: 'INCONCLUSIVE', + notes: ['could not be read'], + }, + { + name: 'embedded whitespace is not PASS', + report: `VERDICT: PASS\n\n${SENTINEL}\n`, + expected: 'INCONCLUSIVE', + notes: ['could not be read'], + }, + { + name: 'a PASS prefix with a suffix is unreadable', + report: `VERDICT: PASS but unfinished\n\n${SENTINEL}\n`, + expected: 'INCONCLUSIVE', + notes: ['could not be read'], + }, + // Both conditions hold at once, and each is reported on its own terms: one + // note per condition, never one block per combination. + { + name: 'an unreadable and unfinished report records both conditions', + report: 'garbled\nevidence\n', + expected: 'INCONCLUSIVE', + notes: ['could not be read', 'cut off mid-report'], + }, +]; + +for (const scenario of cases) { + test(`reporting: ${scenario.name}`, (t) => { + const { dir, env } = fixture(t); + ghStub(dir); + if (scenario.report !== null) + writeFileSync(join(dir, 'audit-report.md'), scenario.report); + const result = spawnSync('bash', ['-c', reporting], { + cwd: dir, + env, + encoding: 'utf8', + }); + assert.equal( + result.status, + scenario.expected === 'PASS' ? 0 : 1, + result.stderr, + ); + const calls = ghCalls(dir); + assert.equal( + calls.some((args) => args[0] === 'issue' && args[1] === 'close'), + scenario.expected === 'PASS', + ); + if (scenario.expected === 'PASS') return; + const body = readFileSync(join(dir, 'audit-comment.md'), 'utf8'); + assert.match( + body, + scenario.expected === 'FAIL' + ? /Audit failed/ + : /Audit reached no usable verdict/, + ); + for (const note of scenario.notes ?? []) + assert.ok(body.includes(note), `missing note: ${note}`); + // Whatever the run did write is published; a report is never summarised away. + if (scenario.report) + assert.ok(body.includes(scenario.report.split('\n')[0]), body); + }); +} + +// Upward only. FAIL is the ceiling: an inconclusive run must not relabel an +// issue that already carries real findings, and the title never needs walking +// back down, because a PASS closes the issue outright. +for (const [name, report, retitles] of [ + ['a FAIL retitles the open issue', 'VERDICT: FAIL\nEvidence\n', true], + [ + 'an INCONCLUSIVE never retitles a FAIL issue', + 'VERDICT: INCONCLUSIVE\n', + false, + ], +]) { + test(`reporting: ${name}`, (t) => { + const { dir, env } = fixture(t); + ghStub(dir); + writeFileSync(join(dir, 'audit-report.md'), report); + const result = spawnSync('bash', ['-c', reporting], { + cwd: dir, + env, + encoding: 'utf8', + }); + assert.equal(result.status, 1, result.stderr); + const calls = ghCalls(dir); + assert.ok( + calls.some((args) => args[0] === 'issue' && args[1] === 'comment'), + 'expected an append', + ); + assert.ok( + !calls.some((args) => args[0] === 'issue' && args[1] === 'create'), + 'expected no second issue', + ); + const edits = calls.filter( + (args) => args[0] === 'issue' && args[1] === 'edit', + ); + assert.equal(edits.length, retitles ? 1 : 0, JSON.stringify(calls)); + if (retitles) + assert.match( + edits[0].join(' '), + /\[security-audit\] FAIL on \d{4}-\d{2}-\d{2}/, + ); + }); +} + +test('reporting: with no open issue a new one is filed under the label', (t) => { + const { dir, env } = fixture(t); + ghStub(dir, false); + writeFileSync(join(dir, 'audit-report.md'), 'VERDICT: FAIL\nEvidence\n'); + const result = spawnSync('bash', ['-c', reporting], { + cwd: dir, + env, + encoding: 'utf8', + }); + assert.equal(result.status, 1, result.stderr); + const create = ghCalls(dir).find( + (args) => args[0] === 'issue' && args[1] === 'create', + ); + assert.ok(create, 'expected an issue to be created'); + assert.match( + create.join(' '), + /\[security-audit\] FAIL on \d{4}-\d{2}-\d{2}/, + ); + assert.ok(create.includes('security-audit-failure'), create.join(' ')); +}); + +test('redaction covers every published sink', (t) => { + const { dir, env } = fixture(t); + for (const sink of publishedSinks) + writeFileSync( + join(dir, sink), + `before ${env.CLAUDE_CODE_OAUTH_TOKEN} after`, + ); + const result = spawnSync( + 'bash', + ['-c', workflowRunBlock(workflow, 'Redact secrets from agent output')], + { + cwd: dir, + env, + encoding: 'utf8', + }, + ); + assert.equal(result.status, 0, result.stderr); + assert.ok( + publishedSinks.length >= 2, + `expected every sink, got ${publishedSinks.join(', ')}`, + ); + for (const sink of publishedSinks) + assert.equal(readFileSync(join(dir, sink), 'utf8'), 'before *** after'); + assert.match( + result.stdout, + /::warning::Redacted 2 literal secret occurrence/, + ); +}); + +test('a throwing redactor removes every published sink', (t) => { + const { dir, env } = fixture(t); + for (const sink of publishedSinks) + writeFileSync(join(dir, sink), env.CLAUDE_CODE_OAUTH_TOKEN); + stub(dir, 'node', 'process.exit(1);'); + const result = spawnSync( + 'bash', + ['-c', workflowRunBlock(workflow, 'Redact secrets from agent output')], + { + cwd: dir, + env, + encoding: 'utf8', + }, + ); + assert.equal(result.status, 1); + for (const sink of publishedSinks) + assert.equal(existsSync(join(dir, sink)), false, sink); +}); + +// The local runner applies the same grammar CI does. 'FAIL — explained' pins +// that an appended explanation is still a finding rather than an unreadable +// report; status alone cannot tell the two apart, so that row checks the +// message too. The `false` rows write no sentinel: a PASS on line 1 of a +// cut-off report does not exit zero here either. +for (const [verdict, cliExit, expected, sentinel = true] of [ + ['PASS', 0, 0], + ['FAIL', 0, 1], + ['FAIL — explained', 0, 1], + ['INCONCLUSIVE', 0, 1], + ['PASS extra', 0, 1], + ['PASS', 7, 1], + ['PASS', 0, 1, false], + ['FAIL', 0, 1, false], +]) { + test(`local runner: ${verdict}, CLI exit ${cliExit}${sentinel ? '' : ', no sentinel'}`, (t) => { + const { dir, env } = fixture(t); + copyFileSync( + join(repo, 'scripts/security-audit-local.sh'), + join(dir, 'scripts/security-audit-local.sh'), + ); + mkdirSync(join(dir, '.github/audit'), { recursive: true }); + for (const name of ['_preamble', 'security']) { + copyFileSync( + join(repo, `.github/audit/${name}.md`), + join(dir, `.github/audit/${name}.md`), + ); + } + // Reads the output file out of the prompt, so the domain file declaring a + // different one fails here rather than producing an empty run. + stub( + dir, + 'claude', + ` + const fs = require('node:fs'); + const prompt = process.argv[3]; + const output = prompt.match(/\\*\\*Output file:\\*\\* \\x60([^\\x60]+)\\x60/)[1]; + fs.writeFileSync(output, ${JSON.stringify(`VERDICT: ${verdict}\nEvidence\n${sentinel ? `${SENTINEL}\n\n` : ''}`)}); + process.exit(${cliExit}); + `, + ); + const result = spawnSync('bash', ['scripts/security-audit-local.sh'], { + cwd: dir, + env, + encoding: 'utf8', + }); + assert.equal(result.status, expected, result.stderr); + if (cliExit === 0) + assert.ok(existsSync(join(dir, 'audit-report.md')), 'expected a report'); + if (cliExit === 0 && verdict.startsWith('FAIL')) { + assert.ok(!result.stderr.includes('no readable verdict'), result.stderr); + assert.match(result.stderr, /reports FAIL/); + } + if (cliExit === 0 && !sentinel) + assert.match(result.stderr, /cut off before finishing/); + }); +} + +// The producer side. Every consumer above is pinned by executing the shipped +// text, but the literal the agent is told to write lives only in +// `_preamble.md` — so without this the producer could be renamed and the whole +// suite would stay green against a sentinel nothing writes. +test('the preamble tells the auditor to write the sentinel every reader waits for', () => { + const preamble = readFileSync( + join(repo, '.github/audit/_preamble.md'), + 'utf8', + ); + const written = [...preamble.matchAll(/^printf '[^']*' >> $/gm)]; + assert.equal( + written.length, + 1, + 'expected exactly one closing `printf` in the preamble', + ); + assert.match( + written[0][0], + new RegExp(SENTINEL.replace(/[-[\]{}()*+?.,\\^$|#]/g, '\\$&')), + ); + // The verdict line the reporting step reads, and the fail-closed value it + // opens as. + assert.match(preamble, /^printf 'VERDICT: INCONCLUSIVE\\n/m); + assert.match(preamble, /never buffered in your context/i); +}); + +// The consumer contract in PACKAGES.md is the job's check-run name, so the +// trigger that produces one per commit and the name it produces are both +// pinned here rather than left to review. +test('the workflow keeps the triggers and the job name a consumer reads', () => { + assert.match(workflow, /^on:\n(?:.*\n)*? {2}push:\n {4}branches: \[main\]$/m); + assert.match(workflow, /^ {2}schedule:\n {4}- cron: '51 4 \* \* \*'$/m); + assert.match(workflow, /^ {2}workflow_dispatch:$/m); + assert.match(workflow, /^ {2}security-audit:\n {4}name: security-audit$/m); + // A superseded commit on `main` still needs its own verdict. + assert.ok( + !/^concurrency:/m.test(workflow), + 'the audit must not cancel a superseded run', + ); +}); From 8f312a05a952c76e73abd679eb2afeb9347e97d3 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Tue, 22 Sep 2026 16:29:25 -0700 Subject: [PATCH 3/3] Hold the audit token in a main-only security-audit environment CLAUDE_CODE_OAUTH_TOKEN moves out of repository scope into the security-audit environment, whose deployment-branch policy admits only main. A workflow pushed on another branch, or a dispatch from one, never receives it. SECURITY.md audits the environment, its policy, and the ruleset that reserves main to pull requests. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/security-audit.yml | 6 ++++++ SECURITY.md | 1 + 2 files changed, 7 insertions(+) diff --git a/.github/workflows/security-audit.yml b/.github/workflows/security-audit.yml index 20fa3d8..9613288 100644 --- a/.github/workflows/security-audit.yml +++ b/.github/workflows/security-audit.yml @@ -29,6 +29,12 @@ jobs: security-audit: name: security-audit runs-on: ubuntu-latest + # CLAUDE_CODE_OAUTH_TOKEN lives in this environment, not at repository + # scope. Its deployment-branch policy admits only `main`, which the + # `Protect main` ruleset reserves to pull requests, so a workflow pushed + # on any other branch, or a dispatch from one, never receives the token. + environment: + name: security-audit # Generous: the audit runs the integration suite against real Postgres # containers as evidence, then reads the auth package adversarially. timeout-minutes: 40 diff --git a/SECURITY.md b/SECURITY.md index c655c2d..93518b0 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -76,6 +76,7 @@ Pinned by `pnpm packages:verify` in `.github/workflows/check.yml`. - **FAIL IF** `.github/workflows/check.yml` stops running `db:verify`, `test:scripts`, `test` and `packages:verify` on pushes to `main` and on every pull request, drops `persist-credentials: false`, or grants any permission beyond `contents: read`; inspect `.github/workflows/check.yml`. - **FAIL IF** `.github/workflows/security-audit.yml` is missing or disabled, loses its `push` to `main` trigger or its `security-audit` job name — a consumer reads that check run by name — lets the reporting step treat anything but an exact `VERDICT: PASS` line above a `` sentinel as passing, or drops the step that redacts secrets from the report and the transcript; inspect `.github/workflows/security-audit.yml`. +- **FAIL IF** the audit job stops declaring `environment: security-audit`, that environment's deployment-branch policy admits any ref but `main` (`gh api repos/diffplug/pgstencil/environments/security-audit/deployment-branch-policies`), or the ruleset on `main` stops requiring a pull request or blocking force-push and deletion (`gh api repos/diffplug/pgstencil/rules/branches/main`); the environment is what keeps `CLAUDE_CODE_OAUTH_TOKEN` from a workflow pushed on any other branch. Pinned by `pnpm test:scripts`, which runs the shipped reporting and redaction shell rather than a copy: `scripts/security-audit.test.mjs`.