From 68b73a910298e4ae2ce6eac0d1b440eecbe27729 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Tue, 25 Aug 2026 11:51:04 -0700 Subject: [PATCH] Refuse a commit that adds something shaped like a credential MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The companion to the fixture removal. A site-admin token sat in a test file for 92 commits and fourteen days on a public mirror, past five audit rounds and two independent reviews, because all of them read the code under change and this was not under change. A grep would have caught it the first day. `scripts/scan-secrets.sh` is that grep, in three rules: * vendor-prefixed credentials — `ghp_`, `github_pat_`, `glpat-`, `xox*-`, `sk-`, `AKIA`/`ASIA`, `ya29.`, `AIza`, `npm_`, `dckr_pat_`. Shape alone identifies these, so there is no context to get wrong. * `BEGIN … PRIVATE KEY` blocks. * an opaque literal assigned to a secret-shaped name — the rule that would have caught this one. The third rule needs **both** halves, and that is what makes it usable rather than another disabled check. Measured before writing it: an entropy-only rule flags 317 literals in this tree, and name-proximity alone flags four, three of which are `secure::get_project_secret(&id, "aws-secret-access-key")` — a keychain *key name* sitting next to the word `secret`. Requiring the literal itself to be hex or base64 with no word structure is what excludes those. Validated rather than asserted: * **0 false positives** across every tracked file. * **Catches the real incident** — `--range 9b2f4fe~1..9b2f4fe` is refused. * Twelve shaped cases pass and fail as intended, including a sha256 in an `assert_eq!`, a git sha in a comment and the new dummy fixture, none of which trip it. * The hook was proved to block an actual `git commit`, not just to exist. Two halves, because each covers the other's gap: * `.githooks/pre-commit`, enabled per clone by `npm run hooks`. Git will not let a repository set its own hooks path — cloning would then be enough to run its code — so this is opt-in everywhere and `--no-verify` skips it. * `Secret Scan`, which nobody can bypass. It carries **no `paths:` filter** on purpose: the leak lived in `app/**` and `build.yml` only runs for `container/**`, so a path-filtered scan would have missed the very thing it exists for. It scans the whole tracked tree rather than a range, because a wrong range fails *open* and the full pass takes 0.5s. Also fixed while here: `core.hooksPath` in this clone pointed at `/workspace/.git/hooks`, a directory that does not exist — so git hooks were disabled outright and anything dropped in `.git/hooks` would have been ignored in silence. A hook that never runs is worse than no hook, because the checklist says it is there. `--tracked` skips binaries. Feeding a blob to grep gets "binary file matches" instead of the line, so a genuine finding inside one would arrive as a sentence nobody can act on. A line ending `pragma: allowlist secret` is skipped — wordy on purpose, so it reads as a claim and leaves something greppable. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LHL9ty7arp8FHwvE77ne7y --- .gitea/workflows/secret-scan.yml | 32 +++++++++ .githooks/pre-commit | 14 ++++ CLAUDE.md | 27 ++++++++ app/package.json | 3 +- scripts/scan-secrets.sh | 111 +++++++++++++++++++++++++++++++ 5 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 .gitea/workflows/secret-scan.yml create mode 100755 .githooks/pre-commit create mode 100755 scripts/scan-secrets.sh diff --git a/.gitea/workflows/secret-scan.yml b/.gitea/workflows/secret-scan.yml new file mode 100644 index 0000000..f47410e --- /dev/null +++ b/.gitea/workflows/secret-scan.yml @@ -0,0 +1,32 @@ +name: Secret Scan + +# **No `paths:` filter, deliberately.** The credential this exists for lived in +# `app/src-tauri/src/docker/container.rs`, which `build.yml` would have skipped — +# that workflow only runs for `container/**`. A scan that can be avoided by +# touching the wrong directory is not a scan. +# +# This is the half of the check that nobody can bypass. The pre-commit hook in +# `.githooks/` is faster and friendlier, but it is opt-in per clone and +# `--no-verify` skips it; both are true of every git hook and neither is fixable +# from inside a repository. + +on: + push: + branches: ["**"] + pull_request: + branches: ["**"] + +jobs: + scan: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + # The whole tracked tree, not just the diff. Scanning a range is cheaper + # but depends on getting the range right across pushes, force-pushes, + # merges and PR events — and a wrong range fails *open*. The full scan + # takes under half a second on this repository and cannot be evaded by + # arranging for the interesting commit to sit outside the window. + - name: Scan tracked files for credentials + run: sh scripts/scan-secrets.sh --tracked diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 0000000..0c0ff8d --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,14 @@ +#!/bin/sh +# Refuse a commit that adds something shaped like a live credential. +# +# Installed by pointing git at this directory: +# +# git config core.hooksPath .githooks +# +# which `npm run hooks` in app/ does for you. It is per-clone — git will not let +# a repository configure its own hooks path, for the obvious reason that cloning +# a repo would then be enough to run its code. So this is opt-in on every +# machine, `--no-verify` skips it, and neither of those is a flaw to fix here: +# the CI job in `.gitea/workflows/build.yml` is the half nobody can bypass. The +# hook exists to tell you in one second rather than in five minutes. +exec "$(git rev-parse --show-toplevel)/scripts/scan-secrets.sh" --staged diff --git a/CLAUDE.md b/CLAUDE.md index 3696563..ab75119 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -525,6 +525,33 @@ Anthropic and Bedrock deliberately keep Claude Code's own defaults. `models/project.rs` for anything that should default to true. - Cross-platform paths: Docker socket is `/var/run/docker.sock` on Linux/macOS, `//./pipe/docker_engine` on Windows +## Secrets + +**`scripts/scan-secrets.sh` refuses a commit that adds something shaped like a live +credential.** Enable the hook once per clone with `npm run hooks` (from `app/`), which sets +`core.hooksPath` to `.githooks`. A repository cannot configure its own hooks path — cloning it +would then be enough to run its code — so this is opt-in everywhere, and `--no-verify` skips it. +The `Secret Scan` workflow is the half nobody can bypass; it carries **no `paths:` filter**, on +purpose, because the incident that prompted all this lived in `app/**` and `build.yml` only runs +for `container/**`. + +Three rules, and the second half of the third is what keeps it usable: vendor-prefixed tokens +(`ghp_`, `sk-`, `AKIA`, `xox`, …), `BEGIN … PRIVATE KEY` blocks, and an opaque literal assigned to +a secret-shaped name. That last one needs **both** halves — the identifier must read as a +credential *and* the whole literal must be hex or base64 with no word structure. Name-proximity +alone flags `secure::get_project_secret(&id, "aws-secret-access-key")`, which is a keychain key +name; the literal test is what excludes it. Measured against the tree: 0 false positives, and it +catches the real incident (`9b2f4fe`) when replayed. + +A line ending `pragma: allowlist secret` is skipped. Make a fixture obviously fake before reaching +for it. + +**Why this exists:** `the_custom_env_fingerprint_never_carries_the_value` used the maintainer's +real Gitea **site-admin** token as its fixture — a test about secrets not escaping, leaking one. It +survived 92 commits and fourteen days in the public GitHub mirror, past five audit rounds and two +independent reviews, because every one of them read the code under change and this sat in a test +nobody had reason to open. Fixtures are never live values; there is no case where they need to be. + ## Testing Frontend tests use Vitest with jsdom environment and React Testing Library. Setup file at `src/test/setup.ts`. Run a single test file: diff --git a/app/package.json b/app/package.json index 229cd9d..816d518 100644 --- a/app/package.json +++ b/app/package.json @@ -9,7 +9,8 @@ "preview": "vite preview", "tauri": "tauri", "test": "vitest run", - "test:watch": "vitest" + "test:watch": "vitest", + "hooks": "git -C .. config core.hooksPath .githooks && echo \"pre-commit secret scan enabled\"" }, "dependencies": { "@tauri-apps/api": "^2", diff --git a/scripts/scan-secrets.sh b/scripts/scan-secrets.sh new file mode 100755 index 0000000..418c6f6 --- /dev/null +++ b/scripts/scan-secrets.sh @@ -0,0 +1,111 @@ +#!/bin/sh +# Refuse to let a live credential into the repository. +# +# Written after one got in: `the_custom_env_fingerprint_never_carries_the_value` +# used the maintainer's real Gitea site-admin token as its fixture. It survived +# 92 commits and fourteen days in a public mirror, past five audit rounds and two +# independent reviews — because every one of those looked at the code under +# change, and this sat in a test nobody had reason to open. A grep would have +# caught it on the first day. This is that grep. +# +# Usage: +# scan-secrets.sh --staged what `git commit` is about to record (the hook) +# scan-secrets.sh --range A..B every line added between two commits (CI) +# scan-secrets.sh --tracked every tracked file, as it stands now +# +# Exit 0 clean, 1 on a finding, 2 on misuse. +# +# ## Why it scans *added lines* and not the whole file +# +# The repository already contains long opaque strings — 317 literals of 32+ +# characters, almost all of them legitimate. A scanner that failed on those would +# be turned off within a day, which is the normal way this kind of check dies. +# Judging only what a commit *adds* keeps the signal where a person can act on it. +# +# ## Escape hatch +# +# A line carrying `pragma: allowlist secret` is skipped. Deliberately wordy: it +# should be uncomfortable enough to type that it is read as a claim, and it +# leaves something greppable behind. + +set -eu + +MODE="${1:---staged}" +RANGE="${2:-}" + +case "$MODE" in + --staged) ADDED=$(git diff --cached --unified=0 --no-color -- . 2>/dev/null || true) ;; + --range) [ -n "$RANGE" ] || { echo "scan-secrets: --range needs A..B" >&2; exit 2; } + ADDED=$(git diff --unified=0 --no-color "$RANGE" -- . 2>/dev/null || true) ;; + --tracked) + # `grep -Iq .` first: without it a binary blob's bytes reach the + # rules below, and GNU grep answers "binary file matches" instead + # of the line — so a real finding inside one would be reported as + # a sentence nobody can act on, and a stray NUL can end the scan + # early. Text files only; binaries are not where source secrets + # live, and `--staged` never sees them either (git emits + # "Binary files differ", not content). + ADDED=$(git ls-files -z \ + | xargs -0 -I{} sh -c 'grep -Iq . "{}" 2>/dev/null && sed "s/^/+/" "{}" 2>/dev/null' \ + || true) ;; + *) echo "scan-secrets: unknown mode $MODE" >&2; exit 2 ;; +esac + +# Only added lines; drop diff headers (+++ b/path) so a filename never matches. +CANDIDATES=$(printf '%s\n' "$ADDED" \ + | grep '^+' \ + | grep -v '^+++' \ + | grep -v 'pragma: allowlist secret' \ + || true) + +[ -n "$CANDIDATES" ] || exit 0 + +FOUND=0 +report() { + FOUND=1 + printf '\n %s\n' "$1" + printf '%s\n' "$2" | sed 's/^/ /' | cut -c1-160 +} + +# --- Rule 1: vendor-issued credentials. Shape alone identifies these, so there +# --- is no false-positive story to tell and no identifier context needed. +VENDOR=$(printf '%s\n' "$CANDIDATES" | grep -nE \ + 'gh[pousr]_[A-Za-z0-9]{20,}|github_pat_[A-Za-z0-9_]{20,}|glpat-[A-Za-z0-9_-]{20,}|xox[baprs]-[A-Za-z0-9-]{10,}|sk-[A-Za-z0-9]{32,}|AKIA[0-9A-Z]{16}|ASIA[0-9A-Z]{16}|ya29\.[A-Za-z0-9_-]{20,}|AIza[0-9A-Za-z_-]{35}|npm_[A-Za-z0-9]{36}|dckr_pat_[A-Za-z0-9_-]{20,}' \ + || true) +[ -n "$VENDOR" ] && report "A vendor-issued credential (its prefix identifies the provider):" "$VENDOR" + +# --- Rule 2: private key material. +KEYS=$(printf '%s\n' "$CANDIDATES" | grep -nE -- '-----BEGIN [A-Z ]*PRIVATE KEY-----' || true) +[ -n "$KEYS" ] && report "Private key material:" "$KEYS" + +# --- Rule 3: an opaque literal assigned to a secret-shaped name. +# +# This is the rule that would have caught the Gitea token — `let secret = +# "<40 hex>"`. Both halves are required, and that is what keeps it usable: +# the *name* must read as a credential, and the *whole literal* must be hex or +# base64 with no word structure. `"aws-secret-access-key"` is a keychain key +# name sitting right next to the word `secret`, and its hyphens are what keep it +# out; measured against the tree, name-proximity alone flagged four lines of +# which three were that shape. +OPAQUE=$(printf '%s\n' "$CANDIDATES" | grep -niE \ + '(secret|token|api[_-]?key|apikey|passwo?rd|passwd|credential|auth[_-]?(key|token))[^A-Za-z0-9]{0,12}[:=][^"'"'"']{0,12}["'"'"']([0-9a-fA-F]{32,}|[A-Za-z0-9+/]{40,}={0,2})["'"'"']' \ + || true) +[ -n "$OPAQUE" ] && report "An opaque literal assigned to a secret-shaped name:" "$OPAQUE" + +if [ "$FOUND" -eq 1 ]; then + cat >&2 <<'MSG' + + ──────────────────────────────────────────────────────────────────────── + Refusing the commit: it adds something shaped like a live credential. + + If it IS live: do not amend and move on. Rotate it first — anything that + reaches a branch is on the mirror, and the mirror is public. + + If it is genuinely not a secret — a fixture, a public key id, test data — + make the literal obviously fake ("not-a-real-token-0000…"), or append + `pragma: allowlist secret` to the line to say so on the record. + ──────────────────────────────────────────────────────────────────────── +MSG + exit 1 +fi +exit 0