feat: staged diff scanning, false-positive fixes, and a 4-9x faster scanner - #84
Draft
pixincreate wants to merge 20 commits into
Draft
feat: staged diff scanning, false-positive fixes, and a 4-9x faster scanner#84pixincreate wants to merge 20 commits into
pixincreate wants to merge 20 commits into
Conversation
KeyWatch's own test suite embeds fake example credentials (the AWS documentation example keys, dummy tokens in generated hook scripts). Exclude tests/ from self-scans so the pre-commit staged scan and manual scans of this repository report only real findings.
At entropy 3.0 the detector flagged long CamelCase identifiers (measured <= ~4.13 bits) throughout ordinary source code, flooding reports with LOW findings. Real base64 payloads measure >= ~4.27, so 4.2 separates the two. The stdin integration fixture loses the double-count of the AWS example key (entropy ~4.0), which the dedicated AWS detector still reports as HIGH. Also exclude detectors.toml from self-scans: the detector definitions themselves are secret-shaped strings.
… diff The pre-commit hook previously scanned whole staged files, so any pre-existing finding blocked commits even for deletion-only or identical-to-upstream changes. scan --staged parses 'git diff --cached -U0' in Rust, scans only added lines, and attributes findings to real file paths and post-image line numbers so --exclude globs and --baseline entries compose with hook scans. The git invocation is hardened against user config that would break diff parsing or silently hide findings: --no-color/color.ui=false (color.ui=always previously ANSI-wrapped every line, making the parser match nothing), --literal-pathspecs (a staged filename containing glob metacharacters was fnmatch-expanded and skipped), and explicit diff.mnemonicPrefix/noprefix/quotePath overrides (wrong prefixes broke path attribution). Files git renders as binary (e.g. '-diff' in .gitattributes) are surfaced in excluded_files instead of passing silently, non-UTF-8 content is decoded lossily instead of aborting the scan, and a git failure exits 2 so callers fail closed. Scan validation is reworked into one exhaustive match over (git_history, staged, stdin). Pre-existing fake fixtures in scanner unit tests get inline keywatch:ignore markers so self-scans stay clean.
Profiling a 36 MB corpus showed ~75% of scan time in the keyword prefilter: has_keywords lowercased the whole line once per detector (~119x per line) and re-lowercased every keyword on every call. Keywords are now lowercased once at Detector construction, each line is lowercased once into a reusable buffer, and all detector keywords are folded into one Aho-Corasick automaton whose single overlapping-search pass per line marks candidate detectors. has_keywords now expects pre-lowercased content. aho-corasick was already in the dependency tree via regex. Measured on the same corpus with identical findings: stdin stream scan 7.5s -> 0.83s (9x), directory scan 1.10s -> 0.27s (4x).
Exercising the baseline flow against public secret-corpus repos showed a feedback loop: the baseline file stores finding hashes that trigger detectors, so a baselined scan re-flagged its own baseline (46 -> 90 findings on trufflesecurity/test_keys) and every --update-baseline re-ingested them, growing the file indefinitely (90 -> 180 -> ...). The --baseline path is now appended to the exclude patterns as an escaped literal glob.
Config discovery previously checked only the first scan path's immediate directory, so a repository-root .keywatch.toml never applied to nested paths like proto/payment.proto. Discovery now walks ancestor directories, nearest config first. The walk stops at the first directory containing .git (the repository root) or at the home directory: .keywatch.toml can add rules, disable detectors, and exclude paths, so trusting one from an arbitrary writable ancestor (e.g. /tmp/.keywatch.toml above CI checkouts) would let it silently weaken every scan below it.
The pre-commit template previously listed staged files in bash and scanned each whole file, so pre-existing findings on unchanged lines blocked deletion-only and identical-to-upstream commits. The template is now a single 'key-watch scan --staged --exclude ...' call: added-line extraction, exclusion, and path attribution live in Rust, the per-file loop / awk hunk parser / symlink checks are gone, and a failing file listing can no longer fail open. Exit handling uses a case statement; any scanner exit above 1 (e.g. git failure) fails the hook closed. The pre-commit framework integration (.pre-commit-hooks.yaml) switches to the same command and stops passing filenames, which the staged scan already covers. hook install/uninstall messages abbreviate the home directory as ~, and hook unit tests pin core.hooksPath locally so a machine-wide global hooks path cannot leak into fixtures.
Output selection becomes a match over the finding count with a verbose guard, the severity gate uses matches! and i32::from instead of a boolean if/else.
…rdDetector '--volume "$PWD:/workspace:ro"' style docker/make snippets were flagged as HIGH Password findings. Uppercase 'PWD:' matches are allowlisted (the shell working-directory variable); lowercase 'pwd = ...' assignments are still detected.
Covers scan --staged semantics (pathspecs, binary handling, the multi-hunk secret caveat with pre-push as backstop), bounded walk-up config discovery, the core.hooksPath side effect and per-repo restore, and changelog entries for the fixes and the keyword prefilter performance work.
The latin-1 test fixture spells café as raw bytes (caf\xE9), which the spell checker read as a typo for 'calf'.
pixincreate
force-pushed
the
feat/staged-scanning
branch
from
August 25, 2026 16:22
ec64e45 to
deae734
Compare
pixincreate
added
bug
…workflow Following the gitleaks/.gitleaksignore and ggshield/.gitguardian.yaml convention of a committed, conventionally named ignore file: when --baseline is not passed, KeyWatch walks up from the scan target (bounded at the repository root or home directory, same as config discovery) for .keywatch-baseline.json. Hook scans therefore use a committed repo baseline with no configuration; --no-baseline-discovery opts out and --update-baseline creates the conventional file when none exists. The resolved baseline is excluded from scanning like an explicit one. Baseline fingerprints normalize a leading ./ so 'scan .' and 'scan nested/file' produce matching entries. Re-baselining is deliberately not automatic (neither gitleaks nor ggshield auto-update; doing so would silently accept new secrets): a workflow_dispatch-only update-baseline workflow regenerates the file and opens a pull request for review.
The self-exclusion used a literal glob built from the --baseline string, which only matched when the caller passed a relative path. A discovered baseline resolves to an absolute path while scanned files stay relative, so a full-tree scan read the baseline back and re-flagged every stored hash as a finding (a 1043-entry baseline reported 1043 LOW findings). Exclusion now compares canonicalized paths.
pixincreate
force-pushed
the
feat/staged-scanning
branch
from
August 25, 2026 18:17
f63d640 to
3549d48
Compare
Suppressing KeyWatch's own fake test fixtures with inline keywatch:ignore comments meant editing source purely to satisfy the scanner. A committed .keywatch-baseline.json does the same job without touching the code, which is the convention gitleaks and ggshield use for their own repositories. This also drops the blanket tests/* path exclusion: the baseline pins the exact fingerprint of each known fixture, so a newly introduced secret in tests/ is still reported, whereas an excluded path hides everything under it forever. Committing the baseline surfaced a gap in the staged scan, fixed here: the baseline file was skipped by path scans but not by scan --staged, so staging a baseline flagged all of its stored hashes as added lines and blocked the commit.
- both git-backed scan modes share one scan_git_output helper that owns spawning, streaming, reaping and status checking, replacing the duplicated kill/wait/early-return blocks - the keyword automaton no longer expect()s: if it cannot be built every keyword detector runs unconditionally, which is slower but can never miss a secret - exclude patterns are collected by iterating the flag directly, so an absent --exclude reads as 'no patterns' rather than a defaulted value - hook scope labels come from a const fn and $HOME is resolved once per process instead of on every path render
println! panics if the reader goes away, so 'key-watch scan . | head' aborted mid-report. Output now goes through a locked writer that treats a broken pipe as a normal end of output and propagates real I/O errors.
The Windows CI failures came from path spelling: 'scan .' records '.\secrets.txt' while the staged diff and explicit paths use forward slashes, so no baseline fingerprint ever matched and both baseline tests failed. Fingerprint paths now fold backslashes to forward slashes along with the leading './', which also makes a baseline generated on one platform usable on another. Exclude globs had the same gap: patterns are written with forward slashes, so 'target/**' never matched a scanned 'target\foo'. Paths are now matched in forward-slashed form. The new unit tests feed both spellings directly, so this stays covered on every platform rather than only on Windows CI.
A repository could disable the pre-commit hook entirely: a detectors.toml committed at its root replaces KeyWatch's detector set, and unlike pre-push the pre-commit template never passed --no-config-discovery. Reproduced end to end — a staged AWS secret was committed with no hook output. Both the generated hook and the pre-commit framework entries now use built-in detectors. Suppression still works through the baseline and inline markers, which the hook continues to honour, so this repository's own detectors.toml suppression moves from .keywatch.toml to the baseline. A '-diff' gitattribute made git emit only 'Binary files ... differ', so the added lines never reached the scanner and the file was reported clean. Undiffable paths are now read back from the index with git cat-file and scanned directly; genuinely binary blobs (NUL bytes) are still skipped, and excluded paths are not re-read. Base64Detector matched from 20 characters while its 4.2 entropy threshold drops 82% of random 20-character base64. Measurement shows base64 and CamelCase identifiers overlap in entropy below ~28 chars under every normalization tried, so the pattern floor moves to 28 where the two separate; known short credential formats have their own detectors with no entropy gate. Also fixes the CI push trigger, which pointed at 'main' while the default branch is 'master', so post-merge CI never ran and the Rust cache was never saved.
GenericKeyValueDetector treats the value side's quotes as optional, so 'let payment_method_token = card_token;' matched as key = value with the identifier 'card_token' clearing the 2.5 entropy gate — reported HIGH on a line containing no literal at all. RandomString had the same shape, flagging quoted snake_case serde attributes. Both now allowlist an all-lowercase snake_case value. Quoted literals end in a quote and generated secrets carry digits or capitals, so 'api_key = "sk_live_..."', 'API_KEY=abc123def456789' and 'token = api_key_2024' all still report.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The pre-commit hook scanned whole staged files, so pre-existing findings blocked unrelated commits (deletion-only changes included). This PR adds
scan --staged, which scans only the added lines ofgit diff --cachedand attributes findings to real file paths and line numbers, so--excludeand--baselinework with hook scans. The hook and the pre-commit framework integration now use it.Changes
scan --staged: staged-diff parsing moves from awk into Rust. The git invocation is pinned against user config that broke parsing:color.ui=alwayspreviously hid every finding, glob characters in filenames skipped files, and prefix/quotePath settings broke path attribution. Binary files are reported as excluded instead of silently passing, non-UTF-8 content is decoded lossily, and git failures fail the hook closed.scan --staged --exclude ...call replaces the bash file loop.$HOME) so a root.keywatch.tomlapplies to nested paths, without trusting config outside the scanned tree..keywatch-baseline.jsonis now auto-discovered (walking up to the repo root, like config), so hook scans use a committed repo baseline with zero configuration;--update-baselinecreates it when missing and--no-baseline-discoveryopts out. The baseline file itself is excluded from scans — it previously re-flagged its own stored hashes and grew on every update. Fingerprints normalize leading./soscan .andscan nested/fileentries match. A manual-triggerupdate-baselineworkflow regenerates the file via a reviewable PR (auto-updating on push would silently accept new secrets, which is why gitleaks and ggshield don't do it either).Base64Detectorentropy 3.0 → 4.2 (stops flagging long identifiers) andPasswordDetectorallowlists$PWD:volume mounts — both common false-positive sources.Tests
+-prefixed content, deleted and binary files, non-UTF-8 input, hunk line offsets), staged-scan integration against real git repos, config discovery boundaries, and a baseline self-scan regression test.color.ui=alwaysanddiff.mnemonicPrefix=true.