63 lines
3.8 KiB
Markdown
63 lines
3.8 KiB
Markdown
---
|
|
name: harden-precommit-secret-gate
|
|
description: "Fix or build a pre-commit secret-scan hook that actually blocks: the -z path hole, line-wide placeholder exemptions, inert doc placeholders, glued report lines, self-test fixture paths — each with a red-first bats case."
|
|
---
|
|
|
|
A pre-commit secret gate that fails open is worse than no gate: it teaches the operator that scanning happened. Four defect classes, each with a black-box test that goes red against the broken version (serverAppConfigs PR #70, command-center #59 — 4 holes closed same night, all red first).
|
|
|
|
## 1. Non-ASCII paths fail open
|
|
|
|
`git diff --cached --name-only` quotes non-ASCII paths (`"docs/\303\251.md"`), so `git diff` on that name returns **empty**, and a loop that `continue`s on an empty diff skips the file silently. Fail closed with NUL delimiting:
|
|
|
|
```bash
|
|
git diff --cached -z --name-only | while IFS= read -r -d '' f; do ...
|
|
```
|
|
|
|
Test: a tracked file whose name contains a non-ASCII character, holding a fake key on an added line, must be blocked.
|
|
|
|
## 2. Placeholder filters must be value-scoped
|
|
|
|
`if echo "$line" | grep -q "$placeholders"` exempts the whole line, so a comment word vouches for the credential beside it:
|
|
|
|
```python
|
|
token = "sk-…" # example config for the docs
|
|
```
|
|
|
|
Filter only the captured value (the `sk-…` group), never the line. Two directions to test: `key = "example"` stays allowed; `token = "sk-<real>" # example` is blocked.
|
|
|
|
## 3. Inert placeholders in documentation
|
|
|
|
A skill/README that *documents* a secret gate quotes the vendor's placeholder ids (`AKIA0000000000000EXAMPLE`, all-zero JWTs). Exempt them by value shape (all-zero / `EXAMPLE` suffix / RFC 2606 domain), or the machine tier stops committing — and the refusal usually lands on a heartbeat commit, where it looks like infrastructure failure, not policy. Also note: an unlanded fix means the checkout runs the **old** hook, so a landing can be blocked by the very bug the PR corrects; audit the staged diff and use the documented override (`ALLOW_SECRETS=1`, never `--no-verify`), recording the audit in the commit message.
|
|
|
|
## 4. Report-line assembly
|
|
|
|
`report+=(" $file: $line")` inside a loop that keeps only the last hit per file glues the next filename onto the previous line when a second hit exists — the report attributes the finding to the wrong file. Test a two-file staged set where both match, and assert each report line names exactly one existing path.
|
|
|
|
## 5. Self-test fixtures in the hook itself
|
|
|
|
The hook's own fixtures contain fake keys by necessity, so the gate would refuse any commit touching the hook or its tests (or a docs page quoting a key as documentation). Pin the bypass to specific paths and to added lines only, and print what was exempted:
|
|
|
|
```bash
|
|
case "$f" in .githooks/pre-commit|tests/secret-gate.bats)
|
|
echo "secret-gate: self-test fixtures exempt: $f" >&2; continue;; esac
|
|
```
|
|
|
|
## The scan rule
|
|
|
|
Scan **added content only** — `git diff --cached -U0 | grep '^+[^+]'` — or every commit touching a file with an old key in context gets blocked, and the operator's response to a gate that blocks everything is to override it always.
|
|
|
|
## Test harness (fixture repo, never the real checkout)
|
|
|
|
`git init` a temp repo, `git config core.hooksPath .githooks`, copy the hook in, `git add` a payload, then run it with the env overrides the hook honours (point its cache/notify paths at `$TMP` so tests never touch the user's config). Pin, in order:
|
|
|
|
| Case | Expect |
|
|
|---|---|
|
|
| fake key on an added line | exit 1, message names the file |
|
|
| `key = "example"` | pass |
|
|
| real key + `# example` comment | exit 1 |
|
|
| non-ASCII filename with a key | exit 1 |
|
|
| two files with hits | report names both |
|
|
| self-test fixtures | pass, with the exempt line printed |
|
|
|
|
`bats -r` after each edit. `--no-verify` is forbidden on Frank's boxes: the hook IS the control.
|