From b8e7a3bdd21e8fcfdacfc5014e4463c4de9932fd Mon Sep 17 00:00:00 2001 From: Dennis Kim Date: Mon, 19 Oct 2026 13:48:00 -0400 Subject: [PATCH] feat(hooks): scan every commit for secrets with gitleaks The repo's pre-commit hook relied on a developer's global hook for a secret scan, so a fresh clone or a machine without one committed unscanned. It now runs `gitleaks git --pre-commit --staged --redact` itself and refuses the commit when gitleaks reports a finding or is not installed. - tests/pre_commit_secret_scan.rs commits in a scratch repo through the real hook with a stand-in gitleaks and global config nulled: a finding refuses, a clean scan commits and scans only the staged diff, a missing gitleaks refuses. All three fail against the previous hook. - CONTRIBUTING.md names gitleaks as a prerequisite of core.hooksPath. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HRWwk1oVv9oLFxjb9cSiCm --- CONTRIBUTING.md | 6 + .../tests/pre_commit_secret_scan.rs | 179 ++++++++++++++++++ scripts/hooks/pre-commit | 24 +++ 3 files changed, 209 insertions(+) create mode 100644 crates/devflow-cli/tests/pre_commit_secret_scan.rs diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 35df90a3..f2ab2723 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -35,6 +35,12 @@ Two things this hook does that are load-bearing: hooks directory wholesale, so without that shim the command above would silently switch off a global secret scanner. It is a no-op if you have no such hook. +- `pre-commit` runs its own secret scan, `gitleaks git --pre-commit --staged + --redact`, on every commit, so **install + [gitleaks](https://github.com/gitleaks/gitleaks#installing)** before + setting `core.hooksPath`: without it on `PATH` the hook refuses the commit + rather than skip the scan. A finding refuses the commit with the secret + redacted; a false positive goes in `.gitleaksignore` by fingerprint. - [`scripts/hooks/commit-msg`](scripts/hooks/commit-msg) enforces Conventional Commit formats (`feat:`, `fix:`, `chore:`, etc.) on every commit subject and warns if the subject exceeds 72 characters, keeping git history and automated diff --git a/crates/devflow-cli/tests/pre_commit_secret_scan.rs b/crates/devflow-cli/tests/pre_commit_secret_scan.rs new file mode 100644 index 00000000..ae5f7eba --- /dev/null +++ b/crates/devflow-cli/tests/pre_commit_secret_scan.rs @@ -0,0 +1,179 @@ +//! Behavioural tests for the gitleaks secret scan in `scripts/hooks/pre-commit`. +//! +//! Each test commits in a scratch repository whose `core.hooksPath` borrows +//! this checkout's `scripts/hooks`, with a stand-in `gitleaks` on `PATH` so the +//! result does not depend on whether the real one is installed (the CI image +//! has none). Global and system git config are nulled, so the hook's chain to +//! a machine-wide hook never runs and cannot mask the repo's own scan. + +use std::os::unix::fs::PermissionsExt as _; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +/// Directories the hook needs (git, bash, awk, grep, sed) and that must not +/// hold a real `gitleaks`, so the missing-scanner case really is missing. +const SYSTEM_PATH: &str = "/usr/bin:/bin"; + +fn repo_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../..") + .canonicalize() + .expect("resolve repo root") +} + +/// A stand-in `gitleaks` that appends its arguments to `args.log` beside it +/// and exits with `exit_code`. +fn fake_gitleaks(dir: &Path, exit_code: i32) -> PathBuf { + let bin = dir.join("bin"); + std::fs::create_dir_all(&bin).expect("create fake bin dir"); + let script = bin.join("gitleaks"); + std::fs::write( + &script, + format!( + "#!/bin/sh\nprintf '%s\\n' \"$*\" >> \"$(dirname \"$0\")/args.log\"\nexit {exit_code}\n" + ), + ) + .expect("write fake gitleaks"); + std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)) + .expect("chmod fake gitleaks"); + bin +} + +fn git(repo: &Path, path: &str, args: &[&str]) -> Output { + let mut cmd = Command::new("git"); + cmd.current_dir(repo) + .args(args) + .env("PATH", path) + .env("HOME", repo) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_NOSYSTEM", "1"); + for var in [ + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_INDEX_FILE", + "GIT_COMMON_DIR", + "GIT_CONFIG", + "GIT_PREFIX", + "GIT_OBJECT_DIRECTORY", + "GIT_ALTERNATE_OBJECT_DIRECTORIES", + ] { + cmd.env_remove(var); + } + cmd.output().expect("run git") +} + +/// A scratch repository on a feature branch, hooked to this checkout's hooks, +/// with one file staged. +fn staged_repo(path: &str) -> tempfile::TempDir { + let repo = tempfile::tempdir().expect("create scratch repo"); + let hooks = repo_root().join("scripts/hooks"); + for args in [ + vec!["init", "-q", "-b", "feature/secret-scan"], + vec!["config", "user.email", "test@example.com"], + vec!["config", "user.name", "test"], + vec![ + "config", + "core.hooksPath", + hooks.to_str().expect("utf-8 hooks path"), + ], + ] { + let out = git(repo.path(), path, &args); + assert!(out.status.success(), "git {args:?}: {}", describe(&out)); + } + std::fs::write(repo.path().join("file.txt"), "content\n").expect("write file"); + let out = git(repo.path(), path, &["add", "file.txt"]); + assert!(out.status.success(), "git add: {}", describe(&out)); + repo +} + +fn commit(repo: &Path, path: &str) -> Output { + git(repo, path, &["commit", "-q", "-m", "test: commit"]) +} + +fn has_commit(repo: &Path, path: &str) -> bool { + git(repo, path, &["rev-parse", "--verify", "-q", "HEAD"]) + .status + .success() +} + +fn describe(out: &Output) -> String { + format!( + "status {:?}, stdout {:?}, stderr {:?}", + out.status.code(), + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ) +} + +#[test] +fn a_commit_is_refused_when_gitleaks_reports_a_finding() { + let fake = tempfile::tempdir().expect("create fake dir"); + let path = format!("{}:{SYSTEM_PATH}", fake_gitleaks(fake.path(), 1).display()); + let repo = staged_repo(&path); + + let out = commit(repo.path(), &path); + + assert!( + !out.status.success(), + "the commit must be refused: {}", + describe(&out) + ); + assert!( + String::from_utf8_lossy(&out.stderr).contains("gitleaks refused the staged changes"), + "the refusal must name the scan: {}", + describe(&out) + ); + assert!(!has_commit(repo.path(), &path), "no commit may be written"); +} + +#[test] +fn a_clean_scan_commits_and_scans_only_the_staged_changes() { + let fake = tempfile::tempdir().expect("create fake dir"); + let bin = fake_gitleaks(fake.path(), 0); + let path = format!("{}:{SYSTEM_PATH}", bin.display()); + let repo = staged_repo(&path); + + let out = commit(repo.path(), &path); + + assert!( + out.status.success(), + "a clean scan must commit: {}", + describe(&out) + ); + assert!(has_commit(repo.path(), &path), "the commit must be written"); + let args = std::fs::read_to_string(bin.join("args.log")).expect("gitleaks was not run"); + let args = args.trim_end(); + assert!( + args.starts_with("git ") + && args.contains("--pre-commit") + && args.contains("--staged") + && args.contains("--redact"), + "gitleaks must scan the staged diff with secrets redacted, got {args:?}" + ); +} + +#[test] +fn a_commit_is_refused_when_gitleaks_is_not_installed() { + assert!( + !Path::new("/usr/bin/gitleaks").exists() && !Path::new("/bin/gitleaks").exists(), + "precondition: a gitleaks under {SYSTEM_PATH} would make this case vacuous" + ); + let repo = staged_repo(SYSTEM_PATH); + + let out = commit(repo.path(), SYSTEM_PATH); + + assert!( + !out.status.success(), + "the commit must be refused: {}", + describe(&out) + ); + assert!( + String::from_utf8_lossy(&out.stderr).contains("gitleaks is not on PATH"), + "the refusal must say the scanner is missing: {}", + describe(&out) + ); + assert!( + !has_commit(repo.path(), SYSTEM_PATH), + "no commit may be written" + ); +} diff --git a/scripts/hooks/pre-commit b/scripts/hooks/pre-commit index e7dda275..13bf5741 100755 --- a/scripts/hooks/pre-commit +++ b/scripts/hooks/pre-commit @@ -154,6 +154,30 @@ if [ ! -x "$worktree_guard" ]; then fi "$worktree_guard" --staged +# Secret scan: every commit's staged changes go through gitleaks. +# +# Until this block, a commit here was scanned only on machines whose global +# hooks happened to run gitleaks (the chain at the bottom of this file); a +# fresh clone, a contributor without one, or `GIT_CONFIG_GLOBAL=/dev/null` +# committed unscanned. A missing gitleaks refuses rather than warns, like the +# scanners above: a secret scan that silently stops running is the failure +# this block exists to close. `--redact` keeps a found secret out of the +# terminal and any log of it. A machine whose global hook also runs gitleaks +# scans twice; that costs well under a second. +# +# Exercised by crates/devflow-cli/tests/pre_commit_secret_scan.rs. +if ! command -v gitleaks >/dev/null 2>&1; then + echo "pre-commit: gitleaks is not on PATH; refusing to commit without a secret scan." >&2 + echo " Install it (https://github.com/gitleaks/gitleaks#installing), then commit again." >&2 + exit 1 +fi +if ! gitleaks git --pre-commit --staged --redact --no-banner --verbose; then + echo "pre-commit: gitleaks refused the staged changes (see its findings above)." >&2 + echo " Remove the secret from the staged files. If it is a false positive, add its" >&2 + echo " fingerprint to .gitleaksignore in the same commit." >&2 + exit 1 +fi + # Drop-in extension point: every file in scripts/hooks/pre-commit.d/ runs, in name # order, with this hook's arguments, and the first failure fails the hook. # develop ships none; a personal workspace branch adds its own checks here as