feat: add the leaks and map commands - #26
Merged
Merged
Conversation
Two deterministic subcommands for v1.15.0. Neither calls a provider, uses
the cache, or costs anything; both consume the commit and path filters
from ADR-0035, so a narrowing learned on a review transfers to both.
`commitbrief leaks` — credential audit (ADR-0036)
The pre-send scanner is a gate: it sees the added lines of the one diff
about to be sent. That shape cannot answer "is there a key in my tree
right now?" or "did anyone ever commit one?" — and a key committed then
removed is still in the history, still reachable in every clone and fork.
Both halves run by default, each with its own off-switch, so a positional
range narrows history without silently disabling the tree. The worktree
half enumerates with `git ls-files -z`: it yields exactly the tracked set
(an untracked, gitignored .env is where a secret is supposed to live and
cannot leak through git), respects .gitignore for free, and sidesteps
ignore.Matcher having no isDir entry point, which would make directory
pruning during a walk subtly wrong. The history half reuses SelectCommits
and scans added lines only, per commit, mapping hunk offsets to real
post-image line numbers and attributing each hit to commit/author/date —
which is what decides whether a key still needs rotating.
internal/guard keeps its API and its leaf status. CompileUserPatterns
returns an unexported type, so the compiled set is captured in a closure
rather than named as a struct field.
Three decisions worth recording:
- No third exit code. --fail-on defaults to `any` here, so a hit exits 1
out of the box; --fail-on none reports without failing.
- --json reuses the locked schema v1 with meta.provider "builtin", not a
new scan schema. That is the whole point: `leaks --json | guard
--from-json -` gates a merge with no new plumbing.
- Finding.Snippet stays empty. The scanner now reads whole files, so its
own report must not become a second copy of the secret. Two tests
assert the secret never reaches the output.
Adds the repo's first content-based binary detection (NUL byte in the
first 8 KiB) plus a 5 MiB cap, both counted and reported. Honors the
review's ignore layers, so a key inside vendor/** is not reported — a
documented blind spot, not an oversight.
`commitbrief map` — commit graph (ADR-0037)
The filters can select a non-contiguous commit set from anywhere in
history, and the only feedback was a count, so a wrong filter silently
reviewed the wrong code. `map` draws the DAG with matching commits
highlighted and the rest dimmed as context.
That needs two metadata walks: one with the predicate dropped but the
range kept, so the lanes stay topologically correct (a graph of only the
matches is a list, not a graph), and one through SelectCommits unchanged,
so the highlight is exactly what a review would pick up rather than an
approximation. --branches gives a topology summary instead.
Zero new dependencies — a graph library would land on the render layer
and need its own ADR. internal/graph owns pure lane assignment (testable
against hand-written DAGs; parents outside the walk close their lane) and
internal/render/graph.go owns appearance only. No --json: a graph schema
would be a second semver-locked contract, the same reason ADR-0020
declined it for summary.
Supporting changes: CommitMeta gains Parents from %P; ui.TerminalWidth
and ui.Clip are extracted from progress.go and shared; --max-commits and
--merges are ordinary bounds on commands that always walk history rather
than modifiers with nothing to modify.
Also rewrites `commitbrief list`'s built-in reference, which was several
releases stale — it never listed commit, guard, mcp, remote pr, doctor,
providers, config, install-hook or upgrade.
Promotes the CHANGELOG to 1.15.0 and repairs the compare links, which
still pointed at v1.13.0 because v1.14.0 shipped without its own row.
Docs: ADR-0036, ADR-0037, contracts/{cli-surface,secret-patterns,
json-schema-v1}, architecture/{overview,system-map}, PRD, README,
CHANGELOG, man pages.
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
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.
Two deterministic subcommands completing the v1.15.0 line. Neither calls a provider, uses the cache, or costs anything; both consume the ADR-0035 commit and path filters, so a narrowing learned on a review transfers to both.
commitbrief leaks— credential audit (ADR-0036)The pre-send scanner is a gate: it sees the added lines of the one diff about to be sent. That shape structurally cannot answer the two questions people actually ask — "is there a key in my tree right now?" and "did anyone ever commit one?" A key that was committed and later removed is still in the history, still reachable in every clone and fork.
Both halves run by default, each with its own off-switch, so a positional range narrows history without silently disabling the tree.
git ls-files -zrather thanfilepath.WalkDir: it yields exactly the tracked set (an untracked, gitignored.envis where a secret is supposed to live and cannot leak through git), respects.gitignorefor free, and sidestepsignore.Matcherhaving noisDir=trueentry point — which would make directory pruning during a walk subtly wrong.SelectCommitsand scans added lines only, per commit, mapping hunk offsets to real post-image line numbers and attributing each hit to commit/author/date — which is what decides whether a key still needs rotating.internal/guardkeeps its API and its leaf status:CompileUserPatternsreturns an unexported type, so the compiled set is captured in a closure rather than named as a struct field.Three decisions worth review attention:
--fail-ondefaults toanyhere, so a hit exits 1 out of the box;--fail-on nonereports without failing. The two-code contract stays intact.--jsonreuses locked schema v1meta.provider: "builtin". Not a new scan schema — that is the whole point, since it makesleaks --json | guard --from-json -gate a merge with no new plumbing.Finding.Snippetstays emptyAdds the repo's first content-based binary detection (NUL byte in the first 8 KiB) plus a 5 MiB cap, both counted and reported.
commitbrief map— commit graph (ADR-0037)The filters can select a non-contiguous commit set from anywhere in history, and the only feedback was a count (
Commits (matched): 12). Which twelve? A wrong filter silently reviewed the wrong code.mapdraws the DAG with matching commits highlighted and the rest dimmed as context.That needs two metadata walks: one with the predicate dropped but the range kept, so the lanes stay topologically correct (a graph of only the matches is a list, not a graph), and one through
SelectCommitsunchanged, so the highlight is exactly what a review would pick up rather than an approximation of it.--branchesgives a topology summary instead.Zero new dependencies — a graph library would land on the render layer and need its own ADR.
internal/graphowns pure lane assignment (testable against hand-written DAGs; parents outside the walk close their lane rather than holding a column open) andinternal/render/graph.goowns appearance only. No--json: a graph schema would be a second semver-locked contract, the same reason ADR-0020 declined it forsummary.Supporting changes
CommitMetagainsParentsfrom%P(additive;RangeCommitsleaves it zero).ui.TerminalWidthandui.Clipextracted fromprogress.go, which held the repo's onlyterm.GetSizecall inline, and now shared.--max-commits/--mergesare ordinary bounds on commands that always walk history, rather than modifiers with nothing to modify.commitbrief list's built-in reference, which was several releases stale — it never listedcommit,guard,mcp,remote pr,doctor,providers,config,install-hookorupgrade.Verification
make checkgreen (fmt, vet, lint, test, release-check, i18n-check, spdx-check, gosec).make manpageregenerated.Verified end to end against a scratch repo with a planted-then-removed AWS key and against this repo:
grep -cfor the planted key returns 0 in both plain and--jsonoutputleaks --json | guard --from-json -→ gate BLOCKED, exit 1map --text sandboxhighlights matches against dimmed context;map --branchesrenders ahead/behindDocs
ADR-0036, ADR-0037 + index rows;
contracts/{cli-surface,secret-patterns,json-schema-v1};architecture/{overview,system-map}; PRD; README; CHANGELOG; man pages. Two new wiki pages (Leaks-command,Map-command) plusHome/_Sidebar/Filtering/Secret-scanner/Guard-commandcross-links — the wiki is a separate repo and needs its own push.