From 797d652a28f18afe2a730e43e98183abe869a5a0 Mon Sep 17 00:00:00 2001 From: Joris Wouter Jonkers Date: Mon, 14 Sep 2026 11:03:53 +0200 Subject: [PATCH] feat: run the secret scan locally, not only in CI scripts/lint-secrets.ts wraps gitleaks the way lint-manifests.ts wraps kubeconform: the binary comes from GITLEAKS or the PATH, and a machine without it skips loudly instead of passing quietly. `npm run verify` runs it, and the secret-scan CI job now runs the same script against the pinned binary it downloads, so one command and one set of patterns serve both. --- .github/workflows/ci.yml | 6 +- README.md | 2 +- docs/architecture.md | 2 +- docs/requirements.md | 3 +- package.json | 3 +- scripts/lint-secrets.ts | 68 ++++++++++++ test/secret-scan-contract.test.ts | 169 ++++++++++++++++++++++++++++++ vitest.config.ts | 32 +++--- 8 files changed, 264 insertions(+), 21 deletions(-) create mode 100644 scripts/lint-secrets.ts create mode 100644 test/secret-scan-contract.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8c4cca0..29ae86f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -204,7 +204,9 @@ # Binary, not gitleaks-action: the action needs a paid licence for org # repos. Pinned version, per - # docs/adr/deferred/0048-class-b-pinning.md. + # docs/adr/deferred/0048-class-b-pinning.md. scripts/lint-secrets.ts is + # the one command and one set of patterns `npm run verify` also runs; + # this step only supplies the pinned binary it downloads. - 'name': 'Secret scan' 'shell': 'bash' 'env': @@ -214,7 +216,7 @@ curl --fail --silent --show-error --location \ "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" \ | tar -xz gitleaks - ./gitleaks detect --no-git --redact --source . --exit-code 1 + GITLEAKS="${PWD}/gitleaks" npm run lint:secrets 'pipeline-complete': 'name': 'Pipeline Complete' diff --git a/README.md b/README.md index 8acc7fb..18ee136 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ npm run verify # lint, format, typecheck, ADR contract, tests + coverage `npm run lint:adrs` alone runs the decision-record contract, and `npm test` runs the suite without enforcing coverage. `npm run test:coverage` (part of `npm run verify`) enforces the ratchet in `vitest.config.ts`: statements -97.88%, branches 90.27%, functions 100%, lines 97.69%. +97.94%, branches 90.72%, functions 100%, lines 97.76%. ## Conventions diff --git a/docs/architecture.md b/docs/architecture.md index 7f3d94c..ea3b83d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -211,7 +211,7 @@ proves the two never drift apart. | tests | `npm run test:coverage` | behaviour, plus the coverage ratchet | | package contents | `node scripts/check-package-contents.ts` | `npm pack` shipping a file outside `docs/adr/` and `spec/`, the boundary the package's `files` field states but does not enforce on its own | | actionlint | a pinned `actionlint` binary | invalid workflow syntax, an undefined `${{ }}` expression, a shellcheck finding inside a `run:` step | -| secret scan | a pinned `gitleaks` binary | a committed secret matching the default ruleset, or this repository's own allowlist entries | +| secret scan | `npm run lint:secrets` | a committed secret matching the default ruleset, or this repository's own allowlist entries | Decisions, links, manifests, requirements and docs share one CI job, `contracts`: all five check a document against a rule rather than code diff --git a/docs/requirements.md b/docs/requirements.md index bea2eeb..7a40476 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -20,7 +20,7 @@ test file and holds at least one test; ids are unique; the count this document states matches the number of rows it holds; and every id cited anywhere in the tracked tree resolves to a row here. -This ledger holds **12** rows. The compiler's behaviours join it as they land. +This ledger holds **13** rows. The compiler's behaviours join it as they land. | id | a contributor or a consumer can rely on | proved by | |---|---|---| @@ -36,3 +36,4 @@ This ledger holds **12** rows. The compiler's behaviours join it as they land. | REQ-010 | A gate's npm script and the CI job that runs it land in the same pull request, so neither can drift from the other unnoticed | [test/pipeline-wiring.test.ts](../test/pipeline-wiring.test.ts) | | REQ-011 | Every script, path, coverage number and Node version README.md and CONTRIBUTING.md name matches the repository they describe | [test/docs-contract.test.ts](../test/docs-contract.test.ts) | | REQ-012 | A pull request's title, body and every commit in it carry no agent attribution: no Co-Authored-By trailer naming a coding agent, no "generated with" banner naming one, no link back to an agent session | [test/pr-title-contract.test.ts](../test/pr-title-contract.test.ts) | +| REQ-013 | `npm run verify` runs the same secret scan CI runs, failing on a committed secret rather than only after a push | [test/secret-scan-contract.test.ts](../test/secret-scan-contract.test.ts) | diff --git a/package.json b/package.json index 213da26..989813a 100644 --- a/package.json +++ b/package.json @@ -20,12 +20,13 @@ "lint:boundaries": "node scripts/lint-boundaries.ts", "lint:requirements": "node scripts/lint-requirements.ts", "lint:docs": "node scripts/lint-docs.ts", + "lint:secrets": "node scripts/lint-secrets.ts", "format": "prettier --write .", "format:check": "prettier --check .", "typecheck": "tsc --noEmit", "test": "vitest run", "test:coverage": "vitest run --coverage", - "verify": "npm run lint && npm run format:check && npm run typecheck && npm run lint:adrs && npm run lint:links && npm run lint:manifests && npm run lint:requirements && npm run lint:docs && npm run lint:boundaries && npm run test:coverage" + "verify": "npm run lint && npm run format:check && npm run typecheck && npm run lint:adrs && npm run lint:links && npm run lint:manifests && npm run lint:requirements && npm run lint:docs && npm run lint:secrets && npm run lint:boundaries && npm run test:coverage" }, "devDependencies": { "@eslint/js": "10.0.1", diff --git a/scripts/lint-secrets.ts b/scripts/lint-secrets.ts new file mode 100644 index 0000000..e8585e1 --- /dev/null +++ b/scripts/lint-secrets.ts @@ -0,0 +1,68 @@ +// The secret scan CI runs, wrapped so `npm run verify` catches a leaked +// credential on the machine that wrote it rather than after a push. +// +// gitleaks scans the tree against its default ruleset plus this repository's +// own .gitleaks.toml allowlist (--no-git: the working tree, not git history, +// the same source CI's checkout scans). A machine without the binary skips +// loudly rather than passing quietly, the same shape as the other gates that +// shell out to a pinned external tool. +// +// A library first: tests call lintSecrets() in-process with a stand-in +// binary, and `node scripts/lint-secrets.ts [root]` is the command, which +// takes the binary from GITLEAKS, or from the PATH. +import { spawnSync } from "node:child_process"; +import { join } from "node:path"; +import { isEntrypoint } from "./lib/entrypoint.ts"; +import { processOutput, type GateOutput } from "./lib/output.ts"; + +const REPOSITORY = join(import.meta.dirname, ".."); + +/** Scan `root` for committed secrets with the gitleaks at `bin`. */ +export function lintSecrets( + root: string, + bin: string, + output: GateOutput, +): number { + const probe = spawnSync(bin, ["version"], { encoding: "utf8" }); + if (probe.error) { + output.out( + `secret scan: SKIPPED because ${bin} is not on PATH. ` + + "CI installs a pinned release; set GITLEAKS to run it locally.\n", + ); + return 0; + } + + const run = spawnSync( + bin, + [ + "detect", + "--no-git", + "--redact", + "--verbose", + "--source", + root, + "--exit-code", + "1", + ], + { encoding: "utf8" }, + ); + if (run.error) { + output.err(`secret scan: could not run ${bin}: ${run.error.message}\n`); + return 1; + } + output.out(run.stdout); + output.err(run.stderr); + return run.status ?? 1; +} + +/** Scan the tree named by argv[0], or this repository. */ +export function main( + argv: readonly string[], + env: NodeJS.ProcessEnv = process.env, + output: GateOutput = processOutput, +): number { + return lintSecrets(argv[0] ?? REPOSITORY, env.GITLEAKS ?? "gitleaks", output); +} + +if (isEntrypoint(import.meta.url, process.argv[1])) + process.exitCode = main(process.argv.slice(2)); diff --git a/test/secret-scan-contract.test.ts b/test/secret-scan-contract.test.ts new file mode 100644 index 0000000..119fdec --- /dev/null +++ b/test/secret-scan-contract.test.ts @@ -0,0 +1,169 @@ +// The secret scan gate, executed. +// +// lint-secrets.ts wraps gitleaks for two reasons: it skips loudly when the +// binary is absent rather than passing quietly, and it fails the build when +// the binary fails. Both are what make it a gate rather than a script, and +// neither is proven by running it against a clean tree. A stand-in binary +// stands in for gitleaks throughout, the same way manifest-contract.test.ts +// stands in for kubeconform: a real scanner is never started here, so no +// planted-credential fixture needs to exist in this tracked file at all, and +// none does. +// +// REQ-013 (docs/requirements.md): `npm run verify` runs the same secret scan +// CI runs, and fails on a finding rather than only after a push. +import { spawnSync } from "node:child_process"; +import { chmodSync, mkdtempSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it, vi } from "vitest"; +import { lintSecrets, main } from "../scripts/lint-secrets.ts"; +import { collect } from "./support/collect.ts"; +import { temporary } from "./setup.ts"; + +const REPOSITORY = join(import.meta.dirname, ".."); + +/** A stand-in gitleaks that answers `version`, then echoes its scan args and exits `code`. */ +function gitleaks(code: number): string { + const bin = join(mkdtempSync(join(temporary(), "bin-")), "gitleaks"); + writeFileSync( + bin, + "#!/bin/sh\n" + + 'if [ "$1" = "version" ]; then echo "8.30.1"; exit 0; fi\n' + + 'echo "fake gitleaks finding File:$6"\n' + + `exit ${code}\n`, + ); + chmodSync(bin, 0o755); + return bin; +} + +/** + * A stand-in that answers `version` and then deletes itself, so the probe + * spawns fine but the scan invocation right after it cannot: `spawnSync` + * fails to start the process at all and sets `error` rather than `status`, + * the same case package-contents-contract.test.ts covers for `npm` with a + * missing `cwd`. There is no `cwd` to break here, since `bin` is an explicit + * path rather than a name looked up on PATH, so the binary removes itself + * between the two calls instead. + */ +function vanishingGitleaks(): string { + const bin = join(mkdtempSync(join(temporary(), "bin-")), "gitleaks"); + writeFileSync(bin, '#!/bin/sh\nrm -- "$0"\necho "8.30.1"\nexit 0\n'); + chmodSync(bin, 0o755); + return bin; +} + +/** + * A stand-in that answers `version`, then kills itself with a signal on the + * scan call rather than exiting, so `spawnSync` reports `status: null` and no + * `error`: the one way a process ends without an exit code. + */ +function signalledGitleaks(): string { + const bin = join(mkdtempSync(join(temporary(), "bin-")), "gitleaks"); + writeFileSync( + bin, + "#!/bin/sh\n" + + 'if [ "$1" = "version" ]; then echo "8.30.1"; exit 0; fi\n' + + "kill -TERM $$\nsleep 5\n", + ); + chmodSync(bin, 0o755); + return bin; +} + +describe("the secret scan", () => { + it("skips loudly and passes when the binary is absent", () => { + const output = collect(); + const absent = join(temporary(), "definitely-not-here"); + expect(lintSecrets(temporary(), absent, output)).toBe(0); + expect(output.text()).toMatch(/SKIPPED/); + expect(output.text()).toMatch(/GITLEAKS/); + }); + + it("passes when gitleaks passes, handing it the source root", () => { + const output = collect(); + const root = temporary(); + expect(lintSecrets(root, gitleaks(0), output)).toBe(0); + expect(output.text()).toMatch(`fake gitleaks finding File:${root}`); + }); + + it("fails the build and names the file when gitleaks finds something", () => { + const output = collect(); + const root = temporary(); + expect(lintSecrets(root, gitleaks(1), output)).toBe(1); + expect(output.text()).toMatch(`File:${root}`); + }); + + it("fails loudly when gitleaks passes the probe but cannot then be run", () => { + const output = collect(); + expect(lintSecrets(temporary(), vanishingGitleaks(), output)).toBe(1); + expect(output.text()).toMatch(/secret scan: could not run/); + }); + + it("fails the build when the scan is killed by a signal, leaving no exit status", () => { + expect(lintSecrets(temporary(), signalledGitleaks(), collect())).toBe(1); + }); +}); + +describe("the command", () => { + it("takes the binary from GITLEAKS", () => { + expect(main([temporary()], { GITLEAKS: gitleaks(3) }, collect())).toBe(3); + }); + + it("scans this repository when no root is given", () => { + expect(main([], { GITLEAKS: gitleaks(0) }, collect())).toBe(0); + }); + + it("falls back to the literal gitleaks name on PATH when GITLEAKS is unset", () => { + const dir = mkdtempSync(join(temporary(), "path-bin-")); + const bin = join(dir, "gitleaks"); + writeFileSync( + bin, + "#!/bin/sh\n" + + 'if [ "$1" = "version" ]; then echo "8.30.1"; exit 0; fi\n' + + "exit 0\n", + ); + chmodSync(bin, 0o755); + vi.stubEnv("PATH", `${dir}:${process.env.PATH ?? ""}`); + expect(main([temporary()], {}, collect())).toBe(0); + }); + + it("runs when Node starts the script, which is how CI runs it", () => { + // GITLEAKS names a binary that is not there, so the gate has to skip. + // Not an empty PATH: an empty PATH means the current directory, and CI + // downloads gitleaks into the directory the tests run from. + const absent = join(temporary(), "definitely-not-here"); + const run = spawnSync( + process.execPath, + [join(REPOSITORY, "scripts", "lint-secrets.ts"), temporary()], + { encoding: "utf8", env: { PATH: "", GITLEAKS: absent } }, + ); + expect(run.status).toBe(0); + expect(run.stdout).toMatch( + /secret scan: SKIPPED because .* is not on PATH/, + ); + }); +}); + +describe("the entrypoint guard", () => { + // This file is small enough that its own bottom-of-file guard, left + // uncovered as every other gate leaves its own, would be a large enough + // share of the file to pull the suite under the ratchet, the same reason + // check-package-contents.ts's guard test gives. Reloading the module with + // `process.argv` set to its own path drives the guard for real, in this + // process, so v8 sees the line the subprocess test above proves but + // coverage otherwise never reaches. GITLEAKS is stubbed to an absent path + // so the run is the deterministic skip, not a real scan of this tree. + it("runs main and sets process.exitCode when Node starts this module", async () => { + const modulePath = join(REPOSITORY, "scripts", "lint-secrets.ts"); + const originalArgv = process.argv; + const originalExitCode = process.exitCode; + process.argv = [process.argv[0] ?? "node", modulePath]; + vi.stubEnv("GITLEAKS", join(temporary(), "definitely-not-here")); + vi.resetModules(); + try { + await import("../scripts/lint-secrets.ts"); + expect(process.exitCode).toBe(0); + } finally { + process.argv = originalArgv; + process.exitCode = originalExitCode; + } + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index e86d3c9..f453d74 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -22,23 +22,25 @@ export default defineConfig({ // A ratchet, per docs/adr/architecture/0101-coverage-is-a-ratchet.md: // set from what the suite reaches, and only ever raised. // - // Measured 2026-09-14, after the pull request attribution check - // (scripts/check-pr-title.ts) landed. Its two `?? ""` capture-group - // fallbacks were replaced by a cast each, the same way the docs - // contract gate's were: a `.+` group cannot be absent once the outer - // regex has matched, and once `messages` is filtered to non-empty - // strings, splitting one always yields a first element. Like the other - // gates, it left only its bottom-of-file entrypoint guard uncovered. - // Two runs of one tree, identical both times: statements 508/519, - // branches 260/288, functions 82/82, lines 467/478. What is left - // uncovered elsewhere is mostly the one-line command guard at the - // bottom of each other gate and the branches for a tool that cannot be - // started at all. + // Measured 2026-09-14, after the local secret scan + // (scripts/lint-secrets.ts) landed. Its file is small enough that the + // bottom-of-file entrypoint guard every other gate leaves uncovered + // would, left the same way, pull the suite under the previous + // threshold; test/secret-scan-contract.test.ts's "the entrypoint guard" + // covers it in-process instead, the way + // test/package-contents-contract.test.ts already does for + // check-package-contents.ts, and its two `??` fallbacks (the default + // binary name, a signal-killed scan's null exit status) each get a + // fixture of their own. Two runs of one tree, identical both times: + // statements 523/534, branches 274/302, functions 84/84, lines + // 482/493. What is left uncovered elsewhere is mostly the one-line + // command guard at the bottom of each other gate and the branches for a + // tool that cannot be started at all. thresholds: { - statements: 97.88, - branches: 90.27, + statements: 97.94, + branches: 90.72, functions: 100, - lines: 97.69, + lines: 97.76, }, }, },