From 66a8743dc01f2e51d5f75efeb2ceba247ba33f9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammet=20=C5=9Eafak?= Date: Sun, 26 Jul 2026 10:19:33 +0300 Subject: [PATCH] feat: add the leaks and map commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CHANGELOG.md | 49 +++- README.md | 45 +++- internal/cli/cli_test.go | 2 +- internal/cli/commitfilter.go | 19 +- internal/cli/integration_test.go | 253 ++++++++++++++++++ internal/cli/leaks.go | 325 +++++++++++++++++++++++ internal/cli/list.go | 109 +++++--- internal/cli/map.go | 216 ++++++++++++++++ internal/cli/root.go | 2 + internal/git/log.go | 4 +- internal/git/refs.go | 257 ++++++++++++++++++ internal/git/refs_test.go | 90 +++++++ internal/git/select.go | 10 +- internal/git/select_test.go | 11 +- internal/graph/layout.go | 235 +++++++++++++++++ internal/graph/layout_test.go | 262 +++++++++++++++++++ internal/i18n/messages.en.yml | 15 ++ internal/i18n/messages.tr.yml | 15 ++ internal/leaks/binary.go | 38 +++ internal/leaks/history.go | 112 ++++++++ internal/leaks/leaks.go | 169 ++++++++++++ internal/leaks/leaks_test.go | 429 +++++++++++++++++++++++++++++++ internal/leaks/worktree.go | 141 ++++++++++ internal/render/graph.go | 325 +++++++++++++++++++++++ internal/render/graph_test.go | 214 +++++++++++++++ internal/ui/progress.go | 32 +-- internal/ui/width.go | 56 ++++ man/commitbrief-leaks.1 | 199 ++++++++++++++ man/commitbrief-map.1 | 192 ++++++++++++++ man/commitbrief.1 | 2 +- 30 files changed, 3759 insertions(+), 69 deletions(-) create mode 100644 internal/cli/leaks.go create mode 100644 internal/cli/map.go create mode 100644 internal/git/refs.go create mode 100644 internal/git/refs_test.go create mode 100644 internal/graph/layout.go create mode 100644 internal/graph/layout_test.go create mode 100644 internal/leaks/binary.go create mode 100644 internal/leaks/history.go create mode 100644 internal/leaks/leaks.go create mode 100644 internal/leaks/leaks_test.go create mode 100644 internal/leaks/worktree.go create mode 100644 internal/render/graph.go create mode 100644 internal/render/graph_test.go create mode 100644 internal/ui/width.go create mode 100644 man/commitbrief-leaks.1 create mode 100644 man/commitbrief-map.1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 301069c..dfe0a21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,43 @@ and the project adheres to [Semantic Versioning 2.0.0](https://semver.org/spec/v ## [Unreleased] +## [1.15.0] - 2026-07-26 + ### Added +- **`commitbrief leaks` — audit the working tree and git history for committed + credentials (ADR-0036).** The pre-send secret scanner is a gate: it only ever sees + the one diff about to be sent, so it cannot answer "is there a key in my tree right + now?" or "did anyone ever commit one?" — and a key that was committed and later + removed is still in the history, still reachable in every clone and fork. `leaks` + answers both with the same eight built-in patterns plus your + `guard.secret_patterns`, deterministically: no provider call, no cache, no cost. + Both halves run by default (`--no-worktree` / `--no-history` switch either off). + The working-tree half reads every **tracked** file whole — untracked, gitignored + files like `.env` are deliberately out of scope, since that is where a secret is + supposed to live and it cannot leak through git. The history half scans the **added + lines** of the commits ADR-0035's filters select, attributing each hit to its + commit, author and date, because that is what decides whether a key still needs + rotating. Bounded by `--max-commits` (default 200), and truncation is always + reported. + Exits 1 on any hit so it gates CI out of the box; `--fail-on none` reports without + failing. `--json` emits the existing schema v1 with `meta.provider: "builtin"`, so + `commitbrief leaks --json | commitbrief guard --from-json -` enforces a + `.commitbrief/policy.yml` budget with no new plumbing. + Findings carry a file, a line and the pattern names — **never the matched text**. + Two limits are documented rather than hidden: it honors the ignore layers, so a key + inside `vendor/**` is not reported, and it is regex-only, so a high-entropy blob + with no recognizable prefix is invisible. +- **`commitbrief map` — the commit graph, and what your filter actually selected + (ADR-0037).** The commit filters can pick a non-contiguous set from anywhere in the + history, and the only feedback was a count ("12 commits matched") — so a wrong + filter silently reviewed the wrong code. `map` draws the DAG with matching commits + highlighted and the rest dimmed as context, which makes a filter checkable before + you pay for a review. `--branches` switches to a branch topology summary: where each + branch sits relative to the base and how far ahead/behind. Deterministic, always + exits 0 — a viewer, not a gate. Rows clip to the terminal rather than wrapping, and + colour plus box-drawing fall back together, so a pipe or `--color=never` yields + plain ASCII. No new dependencies: lane assignment is a pure function in a new + `internal/graph` package. - **Commit-level filters: `--author`, `--committer`, `--start-date`, `--end-date`, `--text` (ADR-0035).** Review a *set of commits* rather than a single diff. `git diff` has no author/date/message options — those are @@ -62,6 +98,15 @@ and the project adheres to [Semantic Versioning 2.0.0](https://semver.org/spec/v The version check runs **only** when you invoke the command: there is no automatic update check and no telemetry. +### Changed +- `commitbrief list`'s built-in command reference was several releases stale — it + never listed `commit`, `guard`, `mcp`, `remote pr`, `doctor`, `providers`, `config`, + `install-hook` or `upgrade`. Rewritten to cover the whole surface, including the + path and commit filters. +- `--max-commits` and `--merges` are usage errors on a review when no commit filter is + set (nothing to modify), but ordinary bounds on `leaks` and `map`, which always walk + history. + ### Fixed - `commitbrief remote pr` now applies `--file` / `--dir` on the **posting** path too. Only the `--no-post` path honored them, so a narrowed run that @@ -2032,7 +2077,9 @@ Anthropic provider. - Initial-commit `CommitDiff` via `go-git` returns `ErrUnsupported` and is handled by the CLI fallback (ADR-0002 mitigation). -[Unreleased]: https://github.com/CommitBrief/commitbrief/compare/v1.13.0...HEAD +[Unreleased]: https://github.com/CommitBrief/commitbrief/compare/v1.15.0...HEAD +[1.15.0]: https://github.com/CommitBrief/commitbrief/compare/v1.14.0...v1.15.0 +[1.14.0]: https://github.com/CommitBrief/commitbrief/compare/v1.13.0...v1.14.0 [1.13.0]: https://github.com/CommitBrief/commitbrief/compare/v1.12.0...v1.13.0 [1.12.0]: https://github.com/CommitBrief/commitbrief/compare/v1.11.0...v1.12.0 [1.11.0]: https://github.com/CommitBrief/commitbrief/compare/v1.10.0...v1.11.0 diff --git a/README.md b/README.md index 86e412c..dff151c 100644 --- a/README.md +++ b/README.md @@ -234,6 +234,19 @@ commitbrief --committer carol --merges # committer identity; keep me commitbrief --author alice --start-date 2026-06-01 --dir internal # all combinable commitbrief diff main..develop --author alice # bound the walk to a range +# Audit for committed credentials — deterministic, no provider call +commitbrief leaks # tracked files + last 200 commits +commitbrief leaks --no-history # working tree only, fast +commitbrief leaks main..HEAD --no-worktree # exactly that range +commitbrief leaks --author alice --start-date 2026-01-01 +commitbrief leaks --json | commitbrief guard --from-json - # gate CI on it + +# See the commit graph — and exactly what a filter selects +commitbrief map # the DAG, newest first +commitbrief map --author alice # matches highlighted, rest dimmed +commitbrief map --branches # ahead/behind the base branch +commitbrief map main..develop --max-commits 50 + # Plain-language change digest (read-only; no findings) commitbrief summary # what's staged, grouped by area commitbrief summary main...develop # a range; uses the commit messages in it @@ -918,7 +931,37 @@ matching rules (exact path or gitignore-style glob), and exclusion is applied last, so it always wins. `commitbrief dry-run` reports how many commits matched and how many files each -layer removed. +layer removed. `commitbrief map` shows *which* commits a filter selects — matches +highlighted, everything else dimmed as context — which is the fastest way to check a +filter before paying for a review. + +## Finding committed credentials + +The pre-send secret scanner is a gate: it sees the one diff about to be sent. It +cannot tell you whether a key is sitting in your tree right now, or whether one was +committed and later removed — and a removed key is still in the history, still +reachable in every clone. + +`commitbrief leaks` answers both, with the same pattern set and no provider call: + +```sh +commitbrief leaks # tracked files + the last 200 commits +commitbrief leaks --patterns # what it looks for (built-ins + yours) +commitbrief leaks --fail-on none # report without failing the build +``` + +It exits 1 on any hit, so it gates CI out of the box, and `--json` emits schema v1 — +so `commitbrief leaks --json | commitbrief guard --from-json -` enforces a +`.commitbrief/policy.yml` budget with no extra plumbing. + +Findings report a **file, a line and the pattern names** — never the matched text. The +scanner reads whole files, so its own report must not become a second copy of the +secret. + +Two limits worth knowing: it honors the ignore layers above, so a key inside +`vendor/**` is not reported; and it is regex-only, so a high-entropy blob with no +recognizable prefix is invisible. It is a targeted check, not a general-purpose +secret scanner. ## Building from source diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 4d14871..03f4d3c 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -11,7 +11,7 @@ import ( func TestRootCommandHasSubcommands(t *testing.T) { root := newRootCmd() - want := []string{"cache", "commit", "compress", "config", "diff", "doctor", "dry-run", "guard", "init", "install-hook", "list", "mcp", "providers", "remote", "setup", "summary", "upgrade"} + want := []string{"cache", "commit", "compress", "config", "diff", "doctor", "dry-run", "guard", "init", "install-hook", "leaks", "list", "map", "mcp", "providers", "remote", "setup", "summary", "upgrade"} got := []string{} for _, c := range root.Commands() { // cobra adds `help` and `completion` automatically; filter to ours. diff --git a/internal/cli/commitfilter.go b/internal/cli/commitfilter.go index 94902f2..45b1bc3 100644 --- a/internal/cli/commitfilter.go +++ b/internal/cli/commitfilter.go @@ -40,6 +40,23 @@ const commitFilterFlags = "--author/--committer/--start-date/--end-date/--text" // positional args the walk defaults to HEAD — the implicit history walk that // makes `commitbrief --author alice` work on its own. func buildCommitFilter(cat *i18n.Catalog, scope reviewScopeFlags, diffArgs []string) (git.CommitFilter, error) { + return commitFilterFor(cat, scope, diffArgs, false) +} + +// buildWalkFilter is buildCommitFilter for commands that walk history +// unconditionally — `leaks` and `map`. For them `--merges` and `--max-commits` +// are ordinary knobs rather than modifiers with nothing to modify, so the +// "modifier used alone" rejection does not apply. +// +// The distinction is real: on a review, `--max-commits 50` alone means the +// user expected a commit walk they never actually asked for, and silently +// reviewing the staged index instead would be wrong. On `leaks`, there is +// always a walk to bound. +func buildWalkFilter(cat *i18n.Catalog, diffArgs []string) (git.CommitFilter, error) { + return commitFilterFor(cat, reviewScopeFlags{}, diffArgs, true) +} + +func commitFilterFor(cat *i18n.Catalog, scope reviewScopeFlags, diffArgs []string, alwaysWalks bool) (git.CommitFilter, error) { f := git.CommitFilter{ Authors: trimAll(global.authors), Committers: trimAll(global.committers), @@ -60,7 +77,7 @@ func buildCommitFilter(cat *i18n.Catalog, scope reviewScopeFlags, diffArgs []str global.startDate, global.endDate)) } - if !f.Active() { + if !f.Active() && !alwaysWalks { // A modifier on its own can't do anything. Say so instead of running // a review that silently ignored a flag the user typed. if global.merges || global.maxCommits > 0 { diff --git a/internal/cli/integration_test.go b/internal/cli/integration_test.go index 6dd1f9a..5ea1d75 100644 --- a/internal/cli/integration_test.go +++ b/internal/cli/integration_test.go @@ -2306,3 +2306,256 @@ func TestExcludeFiltersRejectedByCommitCommand(t *testing.T) { t.Fatal("commit must reject the path denylist for the same reason it rejects --file/--dir") } } + +// ---------- commitbrief leaks (ADR-0036) ---------- + +// leakKey is a well-known AWS documentation placeholder that matches the +// built-in pattern. It is not a real credential. +const leakKey = "AKIAIOSFODNN7EXAMPLE" + +func TestLeaksFindsWorktreeSecret(t *testing.T) { + e := newCLIEnv(t) + writeFile(t, filepath.Join(e.repoRoot, "conf.yml"), "key: "+leakKey+"\n") + gitCmd(t, e.repoRoot, "add", "conf.yml") + gitCmd(t, e.repoRoot, "commit", "-q", "-m", "add conf") + + err := e.run("leaks", "--no-history") + if err == nil { + t.Fatal("a found credential must fail the run so CI gates on it") + } + out := e.out.String() + if !strings.Contains(out, "conf.yml:1") { + t.Errorf("expected the file:line of the hit; got:\n%s", truncate(out, 600)) + } + if !strings.Contains(out, "AWS Access Key") { + t.Errorf("expected the pattern name; got:\n%s", truncate(out, 600)) + } +} + +// The invariant that must never regress: the scanner reads whole files, so its +// own report must not become a second copy of the secret. +func TestLeaksNeverEchoesTheSecret(t *testing.T) { + e := newCLIEnv(t) + writeFile(t, filepath.Join(e.repoRoot, "conf.yml"), "key: "+leakKey+"\n") + gitCmd(t, e.repoRoot, "add", "conf.yml") + gitCmd(t, e.repoRoot, "commit", "-q", "-m", "add conf") + + _ = e.run("leaks", "--no-history", "--fail-on", "none") + combined := e.out.String() + e.errOut.String() + if !strings.Contains(combined, "conf.yml") { + t.Fatalf("expected a finding to check against; got:\n%s", truncate(combined, 600)) + } + if strings.Contains(combined, leakKey) { + t.Fatalf("the matched secret leaked into the report:\n%s", truncate(combined, 600)) + } +} + +func TestLeaksJSONNeverEchoesTheSecret(t *testing.T) { + e := newCLIEnv(t) + writeFile(t, filepath.Join(e.repoRoot, "conf.yml"), "key: "+leakKey+"\n") + gitCmd(t, e.repoRoot, "add", "conf.yml") + gitCmd(t, e.repoRoot, "commit", "-q", "-m", "add conf") + + _ = e.run("leaks", "--no-history", "--json", "--fail-on", "none") + out := e.out.String() + if strings.Contains(out, leakKey) { + t.Fatalf("the matched secret leaked into --json output:\n%s", truncate(out, 600)) + } + // Snippet would be the natural place for it to reappear; it must stay + // omitted (omitempty) rather than carrying the matching line. + if strings.Contains(out, `"snippet"`) { + t.Errorf("snippet must never be populated for a credential finding:\n%s", truncate(out, 600)) + } +} + +// The reason the history half exists: a secret that was committed and then +// removed is still reachable in every clone. +func TestLeaksFindsSecretRemovedFromWorktree(t *testing.T) { + e := newCLIEnv(t) + writeFile(t, filepath.Join(e.repoRoot, "conf.yml"), "key: "+leakKey+"\n") + gitCmd(t, e.repoRoot, "add", "conf.yml") + gitCmd(t, e.repoRoot, "commit", "-q", "-m", "add conf") + writeFile(t, filepath.Join(e.repoRoot, "conf.yml"), "key: REDACTED\n") + gitCmd(t, e.repoRoot, "add", "conf.yml") + gitCmd(t, e.repoRoot, "commit", "-q", "-m", "scrub") + + // The working tree is clean... + if err := e.run("leaks", "--no-history"); err != nil { + t.Fatalf("the working tree is clean, so this must pass: %v\n%s", + err, truncate(e.out.String(), 400)) + } + + // ...but the history still carries it. + e2 := newCLIEnv(t) + writeFile(t, filepath.Join(e2.repoRoot, "conf.yml"), "key: "+leakKey+"\n") + gitCmd(t, e2.repoRoot, "add", "conf.yml") + gitCmd(t, e2.repoRoot, "commit", "-q", "-m", "add conf") + writeFile(t, filepath.Join(e2.repoRoot, "conf.yml"), "key: REDACTED\n") + gitCmd(t, e2.repoRoot, "add", "conf.yml") + gitCmd(t, e2.repoRoot, "commit", "-q", "-m", "scrub") + + if err := e2.run("leaks", "--no-worktree"); err == nil { + t.Fatalf("the history half must find the removed key; got:\n%s", + truncate(e2.out.String(), 600)) + } +} + +func TestLeaksCleanRepoExitsZero(t *testing.T) { + e := newCLIEnv(t) + if err := e.run("leaks"); err != nil { + t.Fatalf("a clean repo must exit 0: %v\n%s", err, truncate(e.out.String(), 400)) + } + if !strings.Contains(e.out.String(), "No credentials found") { + t.Errorf("expected the all-clear line; got:\n%s", truncate(e.out.String(), 400)) + } +} + +func TestLeaksFailOnNoneReportsWithoutFailing(t *testing.T) { + e := newCLIEnv(t) + writeFile(t, filepath.Join(e.repoRoot, "conf.yml"), "key: "+leakKey+"\n") + gitCmd(t, e.repoRoot, "add", "conf.yml") + gitCmd(t, e.repoRoot, "commit", "-q", "-m", "add conf") + + if err := e.run("leaks", "--no-history", "--fail-on", "none"); err != nil { + t.Fatalf("--fail-on none must report without failing: %v", err) + } + if !strings.Contains(e.out.String(), "conf.yml") { + t.Errorf("the finding should still be reported; got:\n%s", truncate(e.out.String(), 400)) + } +} + +func TestLeaksHonorsPathFilters(t *testing.T) { + e := newCLIEnv(t) + writeFile(t, filepath.Join(e.repoRoot, "src", "a.yml"), "key: "+leakKey+"\n") + gitCmd(t, e.repoRoot, "add", "src/a.yml") + gitCmd(t, e.repoRoot, "commit", "-q", "-m", "add src") + + if err := e.run("leaks", "--no-history", "--exclude-dir", "src"); err != nil { + t.Fatalf("--exclude-dir should remove the only hit: %v\n%s", + err, truncate(e.out.String(), 400)) + } +} + +func TestLeaksPatternsListsEffectiveSet(t *testing.T) { + e := newCLIEnv(t) + if err := e.run("leaks", "--patterns"); err != nil { + t.Fatalf("--patterns: %v", err) + } + for _, want := range []string{"AWS Access Key", "JWT", "PEM Private Key"} { + if !strings.Contains(e.out.String(), want) { + t.Errorf("--patterns should list %q; got:\n%s", want, e.out.String()) + } + } +} + +func TestLeaksRejectsBothHalvesDisabled(t *testing.T) { + e := newCLIEnv(t) + if err := e.run("leaks", "--no-worktree", "--no-history"); err == nil { + t.Fatal("disabling both halves leaves nothing to scan and must error") + } +} + +func TestLeaksRejectsMalformedDate(t *testing.T) { + e := newCLIEnv(t) + if err := e.run("leaks", "--start-date", "06-2026"); err == nil { + t.Fatal("a malformed --start-date must fail fast") + } +} + +// --max-commits alone is a usage error on a review (nothing to modify) but an +// ordinary bound here, because leaks always walks history. +func TestLeaksAcceptsMaxCommitsAlone(t *testing.T) { + e := newCLIEnv(t) + if err := e.run("leaks", "--max-commits", "5"); err != nil { + t.Fatalf("--max-commits alone is valid for a command that always walks: %v", err) + } +} + +// ---------- commitbrief map (ADR-0037) ---------- + +func TestMapDrawsCommitGraph(t *testing.T) { + e := newCLIEnv(t) + gitCmd(t, e.repoRoot, "commit", "-q", "-m", "second commit") + + if err := e.run("map"); err != nil { + t.Fatalf("map: %v\nstderr:\n%s", err, e.errOut.String()) + } + out := e.out.String() + if !strings.Contains(out, "second commit") { + t.Errorf("expected the commit subject in the graph; got:\n%s", truncate(out, 600)) + } + if !strings.Contains(out, "initial") { + t.Errorf("expected the whole history; got:\n%s", truncate(out, 600)) + } +} + +func TestMapHighlightsFilterMatches(t *testing.T) { + // The point of the command: show WHICH commits a filter selects and what + // they sat between. + e := newCLIEnv(t) + gitCmd(t, e.repoRoot, "commit", "-q", "-m", "feat: payments gateway") + + if err := e.run("map", "--text", "payments"); err != nil { + t.Fatalf("map --text: %v\nstderr:\n%s", err, e.errOut.String()) + } + out := e.out.String() + if !strings.Contains(out, "matches the filter") { + t.Errorf("a filtered graph must print its legend; got:\n%s", truncate(out, 600)) + } + // Both the match and the unmatched context commit are drawn. + if !strings.Contains(out, "payments gateway") || !strings.Contains(out, "initial") { + t.Errorf("expected match plus surrounding context; got:\n%s", truncate(out, 600)) + } +} + +func TestMapBranchesShowsTopology(t *testing.T) { + e := newCLIEnv(t) + gitCmd(t, e.repoRoot, "commit", "-q", "-m", "second") + gitCmd(t, e.repoRoot, "branch", "feature/x") + + if err := e.run("map", "--branches"); err != nil { + t.Fatalf("map --branches: %v\nstderr:\n%s", err, e.errOut.String()) + } + out := e.out.String() + if !strings.Contains(out, "main") || !strings.Contains(out, "feature/x") { + t.Errorf("expected both branches; got:\n%s", truncate(out, 600)) + } + if !strings.Contains(out, "base") { + t.Errorf("the base branch should be labelled; got:\n%s", truncate(out, 600)) + } +} + +func TestMapRejectsJSONAndMarkdown(t *testing.T) { + // A graph JSON would be a new semver-locked schema; rejecting beats + // emitting something that isn't what the flag promised. + e := newCLIEnv(t) + if err := e.run("map", "--json"); err == nil { + t.Fatal("map --json must be rejected") + } + e2 := newCLIEnv(t) + if err := e2.run("map", "--markdown"); err == nil { + t.Fatal("map --markdown must be rejected") + } +} + +func TestMapRejectsFindingsFlags(t *testing.T) { + e := newCLIEnv(t) + if err := e.run("map", "--fail-on", "critical"); err == nil { + t.Fatal("map emits no findings, so --fail-on must be rejected") + } +} + +func TestMapBranchesRejectsCommitFilters(t *testing.T) { + e := newCLIEnv(t) + if err := e.run("map", "--branches", "--author", "alice"); err == nil { + t.Fatal("--branches lists branches, which a commit filter cannot narrow") + } +} + +func TestMapExitsZeroOnSuccess(t *testing.T) { + // map is a viewer, never a gate. + e := newCLIEnv(t) + if err := e.run("map", "--max-commits", "1"); err != nil { + t.Fatalf("map must exit 0 on a successful render: %v", err) + } +} diff --git a/internal/cli/leaks.go b/internal/cli/leaks.go new file mode 100644 index 0000000..495f92b --- /dev/null +++ b/internal/cli/leaks.go @@ -0,0 +1,325 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package cli + +import ( + "errors" + "fmt" + "sort" + "strings" + + "github.com/spf13/cobra" + + "github.com/CommitBrief/commitbrief/internal/git" + "github.com/CommitBrief/commitbrief/internal/guard" + "github.com/CommitBrief/commitbrief/internal/leaks" + "github.com/CommitBrief/commitbrief/internal/render" +) + +// `commitbrief leaks` (ADR-0036) — the standalone credential audit. +// +// The pre-send scanner in internal/guard is a gate on one diff. This is an +// audit of the repository: whole files in the working tree, plus the added +// lines of historical commits. Deterministic — the same eight built-in regexes +// plus any ADR-0024 user patterns, no provider call, no cache, no cost. +// +// Both halves run by default and each has its own off-switch, so a positional +// range narrows the history half without silently disabling the tree. + +type leaksFlags struct { + noWorktree bool + noHistory bool + patterns bool +} + +// leaksDefaultMaxCommits bounds the history half of a bare run. Scanning every +// commit of a long-lived repo is minutes of work; 200 covers "recent work" +// without an explicit opt-in, and truncation is always reported. +const leaksDefaultMaxCommits = 200 + +func newLeaksCmd() *cobra.Command { + var f leaksFlags + + cmd := &cobra.Command{ + Use: "leaks [...]", + Short: "Scan the working tree and git history for committed credentials", + Long: "Report credential-shaped content in the repository: every tracked file " + + "in the working tree, plus the lines added by historical commits.\n\n" + + "This is the audit counterpart to the pre-send secret scanner, which only " + + "ever sees the one diff about to be reviewed. A secret that was committed " + + "and later removed is still in the history — and still reachable by anyone " + + "who clones the repo — so the history half is what finds it.\n\n" + + "Both halves run by default; --no-worktree and --no-history switch either " + + "off. The commit filters (--author, --start-date, --end-date, --text) narrow " + + "which commits the history half reads, and the path filters narrow which " + + "files either half reads.\n\n" + + "Deterministic: the same regex set the review path uses, no provider call, " + + "no cost. Findings report a file, a line and the pattern names that " + + "matched — never the matched text, so the report itself cannot leak.\n\n" + + "Exits 1 when anything is found, so it gates CI out of the box; pass " + + "--fail-on none to report without failing.", + Args: cobra.ArbitraryArgs, + RunE: func(cmd *cobra.Command, args []string) error { + return runLeaks(cmd, f, args) + }, + } + flags := cmd.Flags() + flags.BoolVar(&f.noWorktree, "no-worktree", false, "skip the working-tree half") + flags.BoolVar(&f.noHistory, "no-history", false, "skip the commit-history half") + flags.BoolVar(&f.patterns, "patterns", false, "list the effective pattern set (built-ins + configured) and exit") + return cmd +} + +func runLeaks(cmd *cobra.Command, f leaksFlags, args []string) error { + app, err := resolveContext(true) + if err != nil { + return err + } + if f.noWorktree && f.noHistory { + return errors.New(app.Catalog.T("leaks.nothing_to_scan")) + } + + userPatterns := toUserSecretPatterns(app.Config.Guard.SecretPatterns) + if f.patterns { + return listLeakPatterns(cmd, userPatterns) + } + + opts := leaks.Options{ + Patterns: userPatterns, + Matcher: buildMatcher(app.RepoRoot), + Files: global.files, + Dirs: global.dirs, + ExcludeFiles: global.excludeFiles, + ExcludeDirs: global.excludeDirs, + } + + ctx := cmd.Context() + var result leaks.Result + + if !f.noWorktree { + tree, sErr := leaks.ScanWorktree(ctx, app.RepoRoot, opts) + if sErr != nil { + return leaksError(app, sErr) + } + result = result.Merge(tree) + } + + if !f.noHistory { + filter, fErr := buildWalkFilter(app.Catalog, args) + if fErr != nil { + return fErr + } + if filter.MaxCommits == 0 { + filter.MaxCommits = leaksDefaultMaxCommits + } + sel, selErr := git.SelectCommits(ctx, app.RepoRoot, filter) + if selErr != nil { + return selErr + } + hist, sErr := leaks.ScanHistory(ctx, app.RepoRoot, sel, opts) + if sErr != nil { + return leaksError(app, sErr) + } + result = result.Merge(hist) + } + result.Sort() + + findings := leakFindings(result, userPatterns) + + if global.json { + return emitLeaksJSON(cmd, app, findings, result) + } + if err := writeLeaksReport(cmd, app, result, findings); err != nil { + return err + } + return leaksGate(app, findings) +} + +// leaksGate turns findings into the exit code. +// +// A scanner that exits 0 on a hit is useless in CI, but the exit-code contract +// has room for exactly two values — so instead of inventing a third, `leaks` +// defaults --fail-on to `any` and reuses the existing severity vocabulary. +// `--fail-on none` reports without failing. The report is written first, so +// the user gets the findings *and* the non-zero exit (the doctor/guard shape). +func leaksGate(app *appContext, findings []render.Finding) error { + raw := strings.TrimSpace(global.failOn) + if raw == "" { + raw = "any" + } + policy, err := parseFailOn(raw) + if err != nil { + return err + } + if !policy.enabled || len(findings) == 0 { + return nil + } + + thresholdRank := severityRank[policy.threshold] + matches := 0 + for _, f := range findings { + rank, ok := severityRank[f.Severity] + if !ok { + continue + } + if policy.anyMode || rank <= thresholdRank { + matches++ + } + } + if matches == 0 { + return nil + } + // A short, distinct message: the report above already stated the count and + // what to do about it, so repeating it verbatim as the error line would + // just print the same sentence twice. + return errors.New(app.Catalog.T("leaks.gate_failed", matches)) +} + +// leakFindings converts scan findings into the locked schema-v1 Finding shape, +// so `leaks --json` can be piped straight into `commitbrief guard --from-json`. +// +// Snippet is deliberately left empty: populating it would echo the secret and +// break the ADR-0007 invariant that the scanner never becomes a leak vector. +func leakFindings(res leaks.Result, userPatterns []guard.UserSecretPattern) []render.Finding { + out := make([]render.Finding, 0, len(res.Findings)) + for _, f := range res.Findings { + title := strings.Join(f.Patterns, ", ") + out = append(out, render.Finding{ + Severity: leakSeverity(f, userPatterns), + File: f.File, + Line: f.Line, + Title: title, + Description: leakDescription(f), + Suggestion: "Treat this credential as compromised: rotate it at the provider, " + + "then remove it from the repository. For a hit in history, rotation is the " + + "only reliable fix — rewriting history does not reach clones or forks that " + + "already have the commit.", + }) + } + return out +} + +// leakSeverity is critical unless every matching pattern is one we cannot +// assert criticality for (a JWT, which is often an expired fixture token, or a +// user-supplied house pattern). +func leakSeverity(f leaks.Finding, userPatterns []guard.UserSecretPattern) render.Severity { + for _, p := range f.Patterns { + if !leaks.IsHighNotCritical(p, userPatterns) { + return render.SeverityCritical + } + } + return render.SeverityHigh +} + +func leakDescription(f leaks.Finding) string { + patterns := strings.Join(f.Patterns, ", ") + if !f.FromHistory() { + return fmt.Sprintf("%s matched in the working tree at %s:%d.", patterns, f.File, f.Line) + } + return fmt.Sprintf("%s matched in commit %s by %s, at %s:%d. The line may no longer be "+ + "in the working tree, but it remains in the repository's history.", + patterns, f.Short, f.Author, f.File, f.Line) +} + +// emitLeaksJSON writes the schema-v1 document. meta names a "builtin" provider +// rather than a model vendor: the scan is deterministic and cost-free, and the +// alternative — a second semver-locked schema — buys nothing a consumer wants +// (ADR-0036). +func emitLeaksJSON(cmd *cobra.Command, app *appContext, findings []render.Finding, res leaks.Result) error { + w, closer, err := openOutput(cmd) + if err != nil { + return err + } + defer closer() + + if err := render.JSON(w, render.Payload{ + Findings: findings, + Meta: render.Meta{ + Provider: "builtin", + Model: "secret-scan", + Lang: app.Lang.Code, + Files: res.FilesScanned, + LinesAdded: 0, + LinesRemoved: 0, + }, + }); err != nil { + return err + } + return leaksGate(app, findings) +} + +// writeLeaksReport renders the human view: a coverage line, then one line per +// finding. Debug-grade tabular output, English, matching dry-run and cache +// stats. +func writeLeaksReport(cmd *cobra.Command, app *appContext, res leaks.Result, findings []render.Finding) error { + w := cmd.OutOrStdout() + + if !global.quiet { + if _, err := fmt.Fprintln(w, app.Catalog.T("leaks.scanned", + res.FilesScanned, res.CommitsScanned)); err != nil { + return fmt.Errorf("leaks: write: %w", err) + } + if res.Skipped > 0 { + // Never silent: a scan that passed over half the repo must say so, + // or "no findings" is misleading. + _, _ = fmt.Fprintln(w, app.Catalog.T("leaks.skipped", res.Skipped)) + } + if res.Truncated { + _, _ = fmt.Fprintln(w, app.Catalog.T("leaks.truncated", leaksCommitLimit())) + } + } + + if len(findings) == 0 { + _, err := fmt.Fprintln(w, app.Catalog.T("leaks.clean")) + return err + } + + if _, err := fmt.Fprintln(w); err != nil { + return fmt.Errorf("leaks: write: %w", err) + } + for i, f := range findings { + where := fmt.Sprintf("%s:%d", f.File, f.Line) + line := fmt.Sprintf(" %-9s %-40s %s", strings.ToUpper(string(f.Severity)), where, f.Title) + if origin := res.Findings[i]; origin.FromHistory() { + line += fmt.Sprintf(" (%s, %s)", origin.Short, origin.Author) + } + if _, err := fmt.Fprintln(w, line); err != nil { + return fmt.Errorf("leaks: write: %w", err) + } + } + _, err := fmt.Fprintln(w, "\n"+app.Catalog.T("leaks.found", len(findings))) + return err +} + +// listLeakPatterns implements --patterns: the effective set, so a user can +// confirm their guard.secret_patterns actually loaded. +func listLeakPatterns(cmd *cobra.Command, userPatterns []guard.UserSecretPattern) error { + extra, err := guard.CompileUserPatterns(userPatterns) + if err != nil { + return err + } + names := guard.AllPatternNames(extra) + sort.Strings(names) + + w := cmd.OutOrStdout() + for _, n := range names { + if _, err := fmt.Fprintln(w, n); err != nil { + return fmt.Errorf("leaks: write: %w", err) + } + } + return nil +} + +// leaksError wraps a scan failure. An invalid guard.secret_patterns regex is +// by far the most likely cause, and it deserves the same localized message the +// review path gives it. +func leaksError(app *appContext, err error) error { + return errors.New(app.Catalog.T("guard.secret_patterns.invalid", err.Error())) +} + +func leaksCommitLimit() int { + if global.maxCommits > 0 { + return global.maxCommits + } + return leaksDefaultMaxCommits +} diff --git a/internal/cli/list.go b/internal/cli/list.go index 7f0852e..1ba0b75 100644 --- a/internal/cli/list.go +++ b/internal/cli/list.go @@ -28,7 +28,7 @@ commitbrief diff main feature # review feature vs main commitbrief diff main...feature # PR-style three-dot diff ` + "```" + ` -Narrow any scope with ` + "`--file`" + ` / ` + "`--dir`" + ` (repeatable, see below). +Narrow any scope with the path and commit filters (see below). ## Summary @@ -42,47 +42,107 @@ commitbrief summary HEAD~3 HEAD # digest the last three commits Read-only, plain text (no findings); each line is grouped by logical area and attributed to the short commit hash(es) for a range. Use ` + "`-o`" + ` to write to a file. -## Setup and rules +## Commit message ` + "```" + ` -commitbrief setup [--local] # provider + API key wizard -commitbrief init # write COMMITBRIEF.md to the repo +commitbrief commit # suggest a message for the staged diff, then commit +commitbrief commit -t conventional # plain | conventional | conventional+body | gitmoji | subject+body +commitbrief commit -g 3 # offer 3 alternatives to choose from +commitbrief commit --yes # commit the first suggestion, no prompt ` + "```" + ` +The only command that writes to git, and only after confirmation. + ## Inspection ` + "```" + ` -commitbrief dry-run # build prompt and report; no API call -commitbrief list # this reference +commitbrief dry-run # build prompt and report; no API call +commitbrief map # draw the commit graph +commitbrief map --branches # branch topology: ahead/behind the base +commitbrief leaks # scan tree + history for credentials +commitbrief list # this reference +commitbrief doctor # pipeline health check +` + "```" + ` + +` + "`map`" + ` and ` + "`leaks`" + ` are deterministic: no provider call, no cost. With a +commit filter set, ` + "`map`" + ` highlights the matching commits and dims the rest. +` + "`leaks`" + ` exits 1 when it finds anything (` + "`--fail-on none`" + ` to report only). + +## CI and automation + +` + "```" + ` +commitbrief --fail-on critical # exit 1 on a critical finding +commitbrief guard # gate on .commitbrief/policy.yml +commitbrief leaks --json | commitbrief guard --from-json - +commitbrief mcp # MCP server over stdio (agent review gate) +commitbrief install-hook # install a git hook +commitbrief remote pr 42 # review a GitHub PR +` + "```" + ` + +## Setup and rules + +` + "```" + ` +commitbrief setup [--local] # provider + API key wizard +commitbrief init # write COMMITBRIEF.md to the repo +commitbrief providers list|use|test # list/switch/ping providers +commitbrief config show|get|set # inspect/edit merged config ` + "```" + ` ## Maintenance ` + "```" + ` -commitbrief compress # shrink COMMITBRIEF.md losslessly -commitbrief cache clear # remove every cached LLM response for this repo -commitbrief cache prune [flags] # drop old/excess entries; defaults --keep-last 500 --older-than 7d +commitbrief compress # shrink COMMITBRIEF.md losslessly +commitbrief cache clear # remove every cached LLM response for this repo +commitbrief cache prune [flags] # drop old/excess entries; defaults --keep-last 500 --older-than 7d +commitbrief cache stats|inspect # cache footprint / one entry +commitbrief upgrade [--check] # check for and install a newer CommitBrief ` + "```" + ` ## Global flags -- ` + "`--json`" + ` — machine-readable JSON output +- ` + "`--json`" + ` — machine-readable JSON output (schema v1) - ` + "`--markdown`" + ` — plain markdown, no ANSI - ` + "`-o, --output `" + ` — write to file instead of stdout - ` + "`--no-cache`" + ` — bypass cache read and write -- ` + "`-f, --file `" + ` — narrow review to this file (repeatable) -- ` + "`-d, --dir `" + ` — narrow review to files under this directory (repeatable) -- ` + "`--copy`" + ` — copy findings (severity, path, title, description) to the system clipboard -- ` + "`-y, --yes`" + ` — auto-confirm prompts +- ` + "`--copy`" + ` — copy findings to the system clipboard +- ` + "`-y, --yes`" + ` — auto-confirm prompts (never the secret scan or cost preflight) - ` + "`-v, --verbose`" + ` — show token/cost/latency footer - ` + "`-q, --quiet`" + ` — suppress info messages on stderr - ` + "`--lang `" + ` — override output language -- ` + "`--provider `" + ` — override configured provider -- ` + "`--model `" + ` — override configured model +- ` + "`--provider `" + ` / ` + "`--model `" + ` — override the backend +- ` + "`--cli claude|gemini|codex`" + ` — use a local CLI tool as the backend - ` + "`--color `" + ` — auto | always | never +- ` + "`--fail-on `" + ` — exit 1 at/above this severity (critical…info, any, none) +- ` + "`--min-severity `" + ` — hide lower severities from the rendered output +- ` + "`--show-prompt`" + ` — print the exact prompt that would be sent, then exit ## Filtering +### Path filters + +- ` + "`-f, --file `" + ` — review only these files (repeatable) +- ` + "`-d, --dir `" + ` — review only files under these dirs (repeatable) +- ` + "`--exclude-file `" + ` — skip these files (repeatable) +- ` + "`--exclude-dir `" + ` — skip these dirs (repeatable) + +A value with ` + "`*`" + `, ` + "`?`" + ` or ` + "`[`" + ` is a gitignore-style glob. Exclusions are +applied last, so ` + "`--dir internal --exclude-dir internal/cli`" + ` reviews everything +under ` + "`internal/`" + ` except ` + "`internal/cli`" + `. + +### Commit filters + +- ` + "`--author `" + ` / ` + "`--committer `" + ` — name or email (repeatable) +- ` + "`--start-date`" + ` / ` + "`--end-date `" + ` — both ends inclusive +- ` + "`--text `" + ` — commit message, or a branch name +- ` + "`--max-commits `" + ` — cap the selection (default 200) +- ` + "`--merges`" + ` — keep merge commits (excluded by default) + +Different kinds are AND'd, multiple values of one kind are OR'd. Setting any of +them selects a set of commits instead of the index, so they replace +` + "`--staged`" + `/` + "`--unstaged`" + ` rather than combining with them. + +### Ignore layers + Three layers, applied in order. Later layers win, so a ` + "`!pattern`" + ` in ` + "`.commitbriefignore`" + ` can revert a built-in exclusion: @@ -94,21 +154,8 @@ Three layers, applied in order. Later layers win, so a ` + "`!pattern`" + ` in 3. **` + "`COMMITBRIEF.md`" + ` semantic filter** — interpreted by the LLM (file-level decisions happen above; this is the natural-language layer). -Example ` + "`.commitbriefignore`" + `: - -` + "```" + `gitignore -# generated migrations -db/migrations/*.sql - -# vendored docs -docs/vendor/** - -# but please review go.sum despite the built-in default -!go.sum -` + "```" + ` - -` + "`commitbrief dry-run --staged`" + ` reports how many files each layer -removed. +` + "`commitbrief dry-run`" + ` reports how many commits matched and how many files +each layer removed. ` func newListCmd() *cobra.Command { diff --git a/internal/cli/map.go b/internal/cli/map.go new file mode 100644 index 0000000..139445e --- /dev/null +++ b/internal/cli/map.go @@ -0,0 +1,216 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package cli + +import ( + "context" + "errors" + "fmt" + "io" + "time" + + "github.com/spf13/cobra" + + "github.com/CommitBrief/commitbrief/internal/git" + "github.com/CommitBrief/commitbrief/internal/graph" + "github.com/CommitBrief/commitbrief/internal/render" + "github.com/CommitBrief/commitbrief/internal/ui" +) + +// `commitbrief map` (ADR-0037) — a deterministic view of the commit graph. +// +// No provider call, no cache, no cost. Two jobs: +// +// 1. Show how commits and branches actually relate, which `dry-run`'s counts +// cannot. +// 2. Make the ADR-0035 commit filters *visible*. `dry-run` reports that 12 +// commits matched; `map --author alice` shows WHICH twelve and what they +// sat between, with the rest dimmed as context. +// +// It is a viewer, never a gate: a successful render always exits 0. + +type mapFlags struct { + branches bool + all bool +} + +// mapDefaultMaxCommits bounds the default DAG height. A graph taller than a +// few screens stops being a visualisation, and the walk itself costs a `git +// log` over that many commits. +const mapDefaultMaxCommits = 200 + +func newMapCmd() *cobra.Command { + var f mapFlags + + cmd := &cobra.Command{ + Use: "map [...]", + Short: "Draw the commit graph, highlighting what a filter selected", + Long: "Render the commit DAG for a range (or HEAD's history) as a lane graph: " + + "one row per commit with its branch/tag labels, subject, author and age.\n\n" + + "The commit filters apply on top, and that is the point: with " + + "--author / --start-date / --end-date / --text set, matching commits are " + + "highlighted and the rest are drawn as dimmed context, so you can see " + + "exactly what a filter selects before spending a review on it.\n\n" + + "--branches switches to a branch topology summary — where each branch " + + "sits relative to the base, and how far ahead/behind.\n\n" + + "Read-only and deterministic: no provider call, no cache, no cost.", + Args: cobra.ArbitraryArgs, + RunE: func(cmd *cobra.Command, args []string) error { + return runMap(cmd, f, args) + }, + } + flags := cmd.Flags() + flags.BoolVar(&f.branches, "branches", false, "show a branch topology summary instead of the commit graph") + flags.BoolVar(&f.all, "all", false, "walk every branch, not just the given range (commit graph only)") + return cmd +} + +func runMap(cmd *cobra.Command, f mapFlags, args []string) error { + app, err := resolveContext(true) + if err != nil { + return err + } + + // map draws a graph, not findings. Rejecting the findings-oriented output + // flags up front beats emitting something that isn't what the flag + // promised. A graph JSON would be a new semver-locked schema; that is + // deferred (ADR-0037), not silently approximated. + if global.json || global.markdown { + return errors.New(app.Catalog.T("map.flag_conflict_format")) + } + if global.failOn != "" || global.minSeverity != "" || global.suggestCommit { + return errors.New(app.Catalog.T("map.flag_conflict_review")) + } + + w, closer, err := openOutput(cmd) + if err != nil { + return err + } + defer closer() + + // Colour and box-drawing travel together: a terminal that refused ANSI is + // also the one most likely to mangle U+2502, so both fall back at once. + styled := ui.ColorEnabled(w, ui.ParseColorMode(global.color)) + opts := render.GraphOptions{ + Color: styled, + Unicode: styled, + Width: ui.TerminalWidth(w), + } + + if f.branches { + return runMapBranches(cmd.Context(), app, w, opts) + } + return runMapGraph(cmd.Context(), cmd, app, f, args, w, opts) +} + +// runMapGraph is the default DAG view. +func runMapGraph(ctx context.Context, cmd *cobra.Command, app *appContext, f mapFlags, args []string, w io.Writer, opts render.GraphOptions) error { + // The commit filter does double duty: its rev range bounds the walk, and + // its predicate decides which rows are highlighted. + // buildWalkFilter, not buildCommitFilter: map always walks history, so + // `map --max-commits 20` is an ordinary bound rather than a modifier with + // nothing to modify. + filter, err := buildWalkFilter(app.Catalog, args) + if err != nil { + return err + } + filtered := filter.Active() + + // Two walks when a filter is active. The first drops the *predicate* but + // keeps the range, so the graph still shows the surrounding commits and + // the lanes stay topologically correct — a graph of only the matches would + // be a list, not a graph. The second is the real filter, and reusing + // SelectCommits for it means the highlight is exactly what a review would + // pick up, never an approximation of it. + topology := filter + topology.Authors = nil + topology.Committers = nil + topology.Text = "" + topology.Since = time.Time{} + topology.Until = time.Time{} + topology.MaxCommits = mapCommitLimit() + if f.all { + topology.Rev = []string{"--all"} + } + // Merges are structure, not noise, in a graph: hiding them would leave + // lanes that fork and never rejoin. + topology.Merges = true + + all, err := git.SelectCommits(ctx, app.RepoRoot, topology) + if err != nil { + return err + } + if len(all.Commits) == 0 { + infof("%s", app.Catalog.T("map.no_commits")) + return nil + } + + var matched map[string]bool + if filtered { + sel, sErr := git.SelectCommits(ctx, app.RepoRoot, filter) + if sErr != nil { + return sErr + } + matched = make(map[string]bool, len(sel.Commits)) + for _, c := range sel.Commits { + matched[c.Hash] = true + } + infof("%s", app.Catalog.T("filter.commit.selected", len(sel.Commits), all.Walked)) + } + opts.Filtered = filtered + + // Ref labels are decoration; a repo that cannot enumerate them still gets + // its graph. + refs, _ := git.RefsByCommit(ctx, app.RepoRoot) + + nodes := make([]graph.Commit, 0, len(all.Commits)) + for _, c := range all.Commits { + nodes = append(nodes, graph.Commit{Hash: c.Hash, Parents: c.Parents}) + } + laid := graph.Layout(nodes, matched) + + rows := make([]render.GraphRow, 0, len(laid)) + for i, row := range laid { + rows = append(rows, render.GraphRow{ + Row: row, + Commit: all.Commits[i], + Refs: refs[all.Commits[i].Hash], + }) + } + if rErr := render.Graph(w, rows, opts); rErr != nil { + return rErr + } + // The truncation caveat is about the view, not part of it, so it goes to + // stderr and a redirected stdout stays a clean graph. + if all.Truncated { + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), app.Catalog.T("map.truncated", mapCommitLimit())) + } + return nil +} + +// runMapBranches is the `--branches` topology view. +func runMapBranches(ctx context.Context, app *appContext, w io.Writer, opts render.GraphOptions) error { + // The commit filters select commits; a branch list has none to select. + // Rejecting beats rendering an identical view that ignored the flag. + if commitFiltersRequested() { + return errors.New(app.Catalog.T("map.branches_flag_conflict", commitFilterFlags)) + } + branches, err := git.BranchTopology(ctx, app.RepoRoot, "") + if err != nil { + return err + } + if len(branches) == 0 { + infof("%s", app.Catalog.T("map.no_branches")) + return nil + } + return render.BranchTopology(w, branches, opts) +} + +// mapCommitLimit resolves the row cap from the shared --max-commits flag, +// falling back to the built-in default. +func mapCommitLimit() int { + if global.maxCommits > 0 { + return global.maxCommits + } + return mapDefaultMaxCommits +} diff --git a/internal/cli/root.go b/internal/cli/root.go index c4daa28..abe9fe8 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -193,6 +193,8 @@ func newRootCmd() *cobra.Command { newMCPCmd(), newGuardCmd(), newUpgradeCmd(), + newMapCmd(), + newLeaksCmd(), ) return cmd } diff --git a/internal/git/log.go b/internal/git/log.go index 604e407..e03fa7c 100644 --- a/internal/git/log.go +++ b/internal/git/log.go @@ -24,9 +24,11 @@ import ( // Body, Files. The identity/date fields stay zero — the manifest never // needed them. // - SelectCommits (the commit-level filters) sets every field, because -// author/committer/date matching happens on this struct. +// author/committer/date matching happens on this struct and the commit +// graph (ADR-0037) needs Parents to lay out its lanes. type CommitMeta struct { Hash string // full 40-hex hash; empty for RangeCommits records + Parents []string // full parent hashes (%P); empty for RangeCommits records Short string // abbreviated hash, e.g. "a1b2c3d" Author string // author name (%an) AuthorEmail string // author email (%ae) diff --git a/internal/git/refs.go b/internal/git/refs.go new file mode 100644 index 0000000..33a61db --- /dev/null +++ b/internal/git/refs.go @@ -0,0 +1,257 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package git + +import ( + "context" + "fmt" + "os/exec" + "strconv" + "strings" + "time" +) + +// Branch topology (ADR-0037) — the input for `commitbrief map --branches`. +// +// Read-only, `git for-each-ref` + `git rev-list --left-right --count`. Like the +// rest of internal/git's newer surface these are package-level functions beside +// the Repo interface rather than methods on it: go-git implements none of them, +// so widening the interface would only add ErrUnsupported stubs (ADR-0035 §E). + +// maxBranches bounds how many refs a topology view will describe. A repo with +// thousands of stale remote branches would otherwise spend a `git rev-list` per +// ref to render a screen nobody can read. +const maxBranches = 200 + +// Branch is one ref plus its position relative to the base branch. +type Branch struct { + Name string // short ref name, e.g. "main" or "origin/feature/x" + Hash string // commit the ref points at + Remote bool // true for refs/remotes/* + Subject string // the tip commit's subject + Author string // the tip commit's author name + Date time.Time // the tip commit's author date + Ahead int // commits on this ref that the base lacks + Behind int // commits on the base that this ref lacks + IsBase bool // this ref IS the base; Ahead/Behind are 0 by definition +} + +// BranchTopology returns every local and remote-tracking branch, each measured +// against base. A base of "" is resolved with DefaultBranch. +// +// Ahead/behind are computed per ref with `git rev-list --left-right --count +// base...ref`, which is symmetric-difference counting — exactly what "3 ahead, +// 12 behind" means to a human. Refs are ordered base-first, then local, then +// remote, then by name, so the output is deterministic. +func BranchTopology(ctx context.Context, repoRoot, base string) ([]Branch, error) { + bin, err := exec.LookPath("git") + if err != nil { + return nil, ErrNoGitCLI + } + if strings.TrimSpace(base) == "" { + base = DefaultBranch(ctx, repoRoot) + } + + out, err := runGit(ctx, bin, repoRoot, []string{ + "for-each-ref", + "--format=%(refname:short)" + fieldSepLiteral + + "%(objectname)" + fieldSepLiteral + + "%(contents:subject)" + fieldSepLiteral + + "%(authorname)" + fieldSepLiteral + + "%(authordate:iso-strict)" + fieldSepLiteral + + "%(refname)", + "refs/heads", "refs/remotes", + }) + if err != nil { + return nil, err + } + + branches := make([]Branch, 0, 16) + for _, line := range strings.Split(out, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + fields := strings.Split(line, fieldSepLiteral) + if len(fields) < 6 { + continue + } + name := strings.TrimSpace(fields[0]) + // origin/HEAD is a symbolic alias for another branch, not a line of + // work of its own — listing it would double-count. Checked against the + // FULL refname because the short form is bare "origin". + if name == "" || isSymbolicHEAD(strings.TrimSpace(fields[5])) { + continue + } + b := Branch{ + Name: name, + Hash: strings.TrimSpace(fields[1]), + Subject: strings.TrimSpace(fields[2]), + Author: strings.TrimSpace(fields[3]), + Remote: strings.HasPrefix(strings.TrimSpace(fields[5]), "refs/remotes/"), + IsBase: name == base, + } + if ts, perr := time.Parse(time.RFC3339, strings.TrimSpace(fields[4])); perr == nil { + b.Date = ts + } + branches = append(branches, b) + if len(branches) >= maxBranches { + break + } + } + + for i := range branches { + if branches[i].IsBase { + continue + } + ahead, behind, cErr := aheadBehind(ctx, bin, repoRoot, base, branches[i].Name) + if cErr != nil { + // An unrelated history (no merge base) makes the comparison + // meaningless rather than fatal — leave the counts at zero and + // keep rendering the rest of the topology. + continue + } + branches[i].Ahead, branches[i].Behind = ahead, behind + } + + sortBranches(branches) + return branches, nil +} + +// aheadBehind counts the symmetric difference between base and ref. +// `--left-right --count` prints "\t": the left side is what +// base has and ref doesn't, the right side the reverse. +func aheadBehind(ctx context.Context, bin, repoRoot, base, ref string) (ahead, behind int, err error) { + out, err := runGit(ctx, bin, repoRoot, []string{ + "rev-list", "--left-right", "--count", base + "..." + ref, + }) + if err != nil { + return 0, 0, err + } + fields := strings.Fields(out) + if len(fields) < 2 { + return 0, 0, fmt.Errorf("git rev-list --count %s...%s: unexpected output %q", base, ref, out) + } + behind, err = strconv.Atoi(fields[0]) + if err != nil { + return 0, 0, err + } + ahead, err = strconv.Atoi(fields[1]) + if err != nil { + return 0, 0, err + } + return ahead, behind, nil +} + +// sortBranches orders base first, then locals, then remotes, then by name — +// so the base branch anchors the top of the view and a run is reproducible. +func sortBranches(branches []Branch) { + rank := func(b Branch) int { + switch { + case b.IsBase: + return 0 + case !b.Remote: + return 1 + default: + return 2 + } + } + for i := 1; i < len(branches); i++ { + for j := i; j > 0; j-- { + a, b := branches[j-1], branches[j] + if rank(a) < rank(b) || (rank(a) == rank(b) && a.Name <= b.Name) { + break + } + branches[j-1], branches[j] = b, a + } + } +} + +// RefsByCommit maps a commit hash to the short names of every branch and tag +// pointing at it, for the graph's `(main, v1.2.0)` labels. +// +// Best-effort: a repo with no refs, or a git that refuses the query, yields an +// empty map rather than an error — labels are decoration, and a graph without +// them is still a graph. +func RefsByCommit(ctx context.Context, repoRoot string) (map[string][]string, error) { + bin, err := exec.LookPath("git") + if err != nil { + return nil, ErrNoGitCLI + } + out, err := runGit(ctx, bin, repoRoot, []string{ + "for-each-ref", + "--format=%(objectname)" + fieldSepLiteral + + "%(refname:short)" + fieldSepLiteral + + "%(refname)", + "refs/heads", "refs/remotes", "refs/tags", + }) + if err != nil { + return nil, err + } + refs := make(map[string][]string) + for _, line := range strings.Split(out, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + fields := strings.Split(line, fieldSepLiteral) + if len(fields) < 3 { + continue + } + hash, name, full := strings.TrimSpace(fields[0]), strings.TrimSpace(fields[1]), strings.TrimSpace(fields[2]) + if hash == "" || name == "" || isSymbolicHEAD(full) { + continue + } + refs[hash] = append(refs[hash], name) + } + return refs, nil +} + +// isSymbolicHEAD reports whether a full refname is a symbolic HEAD alias such +// as refs/remotes/origin/HEAD. The check MUST use the full refname: the short +// form of refs/remotes/origin/HEAD is bare "origin", so filtering on the short +// name lets it through and the graph grows a phantom "origin" label. +func isSymbolicHEAD(fullRef string) bool { + return strings.HasSuffix(fullRef, "/HEAD") +} + +// DefaultBranch resolves the repo's base branch: the target of +// refs/remotes/origin/HEAD when it exists, else the first of main/master/trunk +// that does, else the current HEAD. It never fails — a topology view with an +// imperfect base is far more useful than an error. +func DefaultBranch(ctx context.Context, repoRoot string) string { + bin, err := exec.LookPath("git") + if err != nil { + return "main" + } + if out, sErr := runGit(ctx, bin, repoRoot, []string{ + "symbolic-ref", "--short", "refs/remotes/origin/HEAD", + }); sErr == nil { + // "origin/main" → "main": the local branch is the useful comparison + // point, and it is what a user means by "the base". + if name := strings.TrimSpace(out); name != "" { + return strings.TrimPrefix(name, "origin/") + } + } + for _, candidate := range []string{"main", "master", "trunk"} { + if _, vErr := runGit(ctx, bin, repoRoot, []string{ + "rev-parse", "--verify", "--quiet", candidate, + }); vErr == nil { + return candidate + } + } + if out, hErr := runGit(ctx, bin, repoRoot, []string{ + "rev-parse", "--abbrev-ref", "HEAD", + }); hErr == nil { + if name := strings.TrimSpace(out); name != "" && name != "HEAD" { + return name + } + } + return "HEAD" +} + +// fieldSepLiteral is the raw US byte. for-each-ref has no %x1f escape (that is +// a `git log` pretty-format feature), so the separator goes in literally — +// which is fine here because, unlike --format on log, for-each-ref does not +// reject a format that lacks a `%`. +const fieldSepLiteral = "\x1f" diff --git a/internal/git/refs_test.go b/internal/git/refs_test.go new file mode 100644 index 0000000..e50ea9d --- /dev/null +++ b/internal/git/refs_test.go @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package git + +import ( + "context" + "testing" +) + +func TestBranchTopologyReportsAheadBehind(t *testing.T) { + r := newFilterRepo(t) + branches, err := BranchTopology(context.Background(), r.dir, "main") + if err != nil { + t.Fatalf("BranchTopology: %v", err) + } + byName := map[string]Branch{} + for _, b := range branches { + byName[b.Name] = b + } + + base, ok := byName["main"] + if !ok { + t.Fatalf("main missing from %v", branchNames(branches)) + } + if !base.IsBase || base.Ahead != 0 || base.Behind != 0 { + t.Errorf("base branch must report itself as base with zero counts: %#v", base) + } + + // payments/stripe forked off HEAD~1 and added one commit, while main went + // on to add one of its own. + feat, ok := byName["payments/stripe"] + if !ok { + t.Fatalf("payments/stripe missing from %v", branchNames(branches)) + } + if feat.Ahead != 1 { + t.Errorf("ahead = %d, want 1", feat.Ahead) + } + if feat.Behind != 2 { + t.Errorf("behind = %d, want 2", feat.Behind) + } + if feat.Subject != "chore: bump sdk" { + t.Errorf("subject = %q, want the tip commit's subject", feat.Subject) + } + if feat.Author != "Bob" { + t.Errorf("author = %q, want Bob", feat.Author) + } + if feat.Date.IsZero() { + t.Error("tip date should be populated") + } +} + +func TestBranchTopologyOrdersBaseFirst(t *testing.T) { + r := newFilterRepo(t) + r.git(t, nil, "branch", "aaa-early", "HEAD") + branches, err := BranchTopology(context.Background(), r.dir, "main") + if err != nil { + t.Fatalf("BranchTopology: %v", err) + } + if len(branches) == 0 || !branches[0].IsBase { + t.Fatalf("base branch must sort first, got %v", branchNames(branches)) + } +} + +func TestBranchTopologySkipsRemoteHEADAlias(t *testing.T) { + r := newFilterRepo(t) + branches, err := BranchTopology(context.Background(), r.dir, "main") + if err != nil { + t.Fatalf("BranchTopology: %v", err) + } + for _, b := range branches { + if b.Name == "origin/HEAD" { + t.Fatalf("origin/HEAD is a symbolic alias and must not be listed") + } + } +} + +func TestDefaultBranchFallsBackToMain(t *testing.T) { + r := newFilterRepo(t) + if got := DefaultBranch(context.Background(), r.dir); got != "main" { + t.Fatalf("DefaultBranch = %q, want main", got) + } +} + +func branchNames(branches []Branch) []string { + out := make([]string, 0, len(branches)) + for _, b := range branches { + out = append(out, b.Name) + } + return out +} diff --git a/internal/git/select.go b/internal/git/select.go index b40e189..f49b62c 100644 --- a/internal/git/select.go +++ b/internal/git/select.go @@ -150,6 +150,7 @@ const selectRecordFormat = "--format=" + fmtRecordSep + "%cn" + fmtFieldSep + "%ce" + fmtFieldSep + "%aI" + fmtFieldSep + + "%P" + fmtFieldSep + "%s" + fmtFieldSep + "%b" + fmtFieldSep @@ -159,7 +160,7 @@ const showRecordFormat = "--format=" + fmtRecordSep // selectRecordFields is how many US-separated fields selectRecordFormat plus // the --name-status block produce. -const selectRecordFields = 10 +const selectRecordFields = 11 // FilteredDiff selects the commits matching f and returns their concatenated // patches together with the selection metadata. It is read-only: `git log`, @@ -543,14 +544,15 @@ func parseSelectCommits(out string) []CommitMeta { AuthorEmail: strings.TrimSpace(fields[3]), Committer: strings.TrimSpace(fields[4]), CommitterEmail: strings.TrimSpace(fields[5]), - Subject: strings.TrimSpace(fields[7]), - Body: strings.TrimSpace(fields[8]), + Parents: strings.Fields(fields[7]), + Subject: strings.TrimSpace(fields[8]), + Body: strings.TrimSpace(fields[9]), } if ts, err := time.Parse(time.RFC3339, strings.TrimSpace(fields[6])); err == nil { c.Date = ts } if len(fields) == selectRecordFields { - c.Files = parseNameStatus(fields[9]) + c.Files = parseNameStatus(fields[10]) } commits = append(commits, c) } diff --git a/internal/git/select_test.go b/internal/git/select_test.go index b9c4912..36844e9 100644 --- a/internal/git/select_test.go +++ b/internal/git/select_test.go @@ -427,7 +427,9 @@ func TestParseSelectCommitsSkipsMalformed(t *testing.T) { strings.Join([]string{ "1111111111111111111111111111111111111111", "1111111", "Alice", "alice@example.com", "Alice", "alice@example.com", - "2026-01-10T12:00:00+00:00", "feat: x", "body", + "2026-01-10T12:00:00+00:00", + "2222222222222222222222222222222222222222 3333333333333333333333333333333333333333", + "feat: x", "body", }, logFieldSep) + logFieldSep + "\nM\tx.go\n" out := logRecordSep + "deadbeef" + good // first record has no field separators @@ -438,6 +440,13 @@ func TestParseSelectCommitsSkipsMalformed(t *testing.T) { if got[0].Author != "Alice" || got[0].Date.IsZero() || !equalStrings(got[0].Files, []string{"x.go"}) { t.Fatalf("record not parsed as expected: %#v", got[0]) } + // %P is space-separated, so a merge commit yields both parents. + if len(got[0].Parents) != 2 { + t.Fatalf("expected 2 parents, got %#v", got[0].Parents) + } + if got[0].Subject != "feat: x" || got[0].Body != "body" { + t.Fatalf("parents field must not shift subject/body: %#v", got[0]) + } } func TestChunk(t *testing.T) { diff --git a/internal/graph/layout.go b/internal/graph/layout.go new file mode 100644 index 0000000..b1f6b00 --- /dev/null +++ b/internal/graph/layout.go @@ -0,0 +1,235 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +// Package graph lays a commit list out onto terminal columns ("lanes") so a +// renderer can draw the DAG the way `git log --graph` does (ADR-0037). +// +// It is deliberately git-free and render-free: the input is an ordered commit +// list with parent hashes, the output is one Row per commit carrying the glyph +// for every lane column. That keeps the tricky part — lane bookkeeping — a pure +// function that can be tested against hand-written DAGs, and leaves colour, +// unicode-vs-ASCII, and width clipping entirely to the caller. +package graph + +// Commit is the minimal shape Layout needs. It mirrors the fields of +// git.CommitMeta that matter for topology, so callers can adapt without this +// package importing internal/git (keeping it a leaf, like internal/tokens). +type Commit struct { + Hash string + Parents []string +} + +// Glyph is what occupies one lane column on one row. +type Glyph uint8 + +const ( + // GlyphEmpty is an unused column. + GlyphEmpty Glyph = iota + // GlyphCommit is the commit's own marker; exactly one per row. + GlyphCommit + // GlyphVertical is a lane passing straight through this row. + GlyphVertical + // GlyphFork is a lane branching out to the right (a second parent leaving + // the commit's lane). + GlyphFork + // GlyphMerge is a lane folding back in to the left (a lane whose commit + // has been reached and that now rejoins). + GlyphMerge +) + +// Row is one rendered line: the commit, its lane index, and the glyph for each +// lane column. Cells is exactly Width() wide. +type Row struct { + Commit Commit + Lane int + Matched bool + Cells []Glyph +} + +// Width returns how many lane columns this row occupies. +func (r Row) Width() int { return len(r.Cells) } + +// Layout assigns each commit a lane and produces one Row per commit, in input +// order (which the caller is expected to have sorted newest-first, as git log +// does). +// +// The algorithm is the standard one: `lanes` holds, per column, the hash that +// column is currently waiting to draw. When a commit is reached, it takes the +// leftmost lane already waiting for it; its first parent inherits that lane and +// every additional parent claims a new one (a fork). Any *other* lane also +// waiting for this commit is released — that is a merge point, where two lines +// of development converge. +// +// matched marks which commits satisfied the caller's filter; a nil map means +// "everything matches", which is what an unfiltered run wants. +// +// Commits whose parents are outside the input set (a truncated walk, or a +// range that starts mid-history) simply close their lane — the graph shows the +// boundary rather than inventing edges to commits it was never given. +func Layout(commits []Commit, matched map[string]bool) []Row { + if len(commits) == 0 { + return nil + } + // present bounds the DAG to what we were actually given, so a parent edge + // pointing outside the walk closes its lane instead of holding a column + // open forever. + present := make(map[string]struct{}, len(commits)) + for _, c := range commits { + present[c.Hash] = struct{}{} + } + + var lanes []string // per column: the hash that column is waiting for; "" = free + rows := make([]Row, 0, len(commits)) + + for _, c := range commits { + lane := indexOf(lanes, c.Hash) + if lane < 0 { + // A commit nothing is waiting for: a branch tip, or the first + // commit of the walk. It opens its own lane. + lane = firstFree(lanes) + if lane == len(lanes) { + lanes = append(lanes, "") + } + lanes[lane] = c.Hash + } + + // Every other lane waiting for this same commit is a line of + // development converging here. Record them, then release them. + var merging []int + for i, waiting := range lanes { + if i != lane && waiting == c.Hash { + merging = append(merging, i) + lanes[i] = "" + } + } + + parents := knownParents(c.Parents, present) + + // Rebind this commit's lane to its first parent, then give every + // additional parent a lane of its own. + var forking []int + if len(parents) == 0 { + lanes[lane] = "" // root commit (or a truncated boundary): lane closes + } else { + lanes[lane] = parents[0] + for _, p := range parents[1:] { + // A parent already being waited for needs no new column — the + // two lines simply share it from here down. + if indexOf(lanes, p) >= 0 { + continue + } + f := firstFree(lanes) + if f == len(lanes) { + lanes = append(lanes, "") + } + lanes[f] = p + forking = append(forking, f) + } + } + + rows = append(rows, Row{ + Commit: c, + Lane: lane, + Matched: isMatched(matched, c.Hash), + Cells: cellsFor(lanes, lane, merging, forking), + }) + } + + // Pad every row to the widest one so a renderer can index columns without + // bounds-checking each row. + width := 0 + for _, r := range rows { + if len(r.Cells) > width { + width = len(r.Cells) + } + } + for i := range rows { + for len(rows[i].Cells) < width { + rows[i].Cells = append(rows[i].Cells, GlyphEmpty) + } + } + return rows +} + +// cellsFor renders one row's columns from the lane table as it stands *after* +// the commit was processed. The commit's own column shows the commit marker; +// lanes that merged into it this row show a merge glyph even though they are +// already released; lanes opened by a fork show a fork glyph; everything else +// still occupied is a pass-through. +func cellsFor(lanes []string, lane int, merging, forking []int) []Glyph { + cells := make([]Glyph, len(lanes)) + for i, waiting := range lanes { + if waiting != "" { + cells[i] = GlyphVertical + } + } + for _, i := range merging { + if i < len(cells) { + cells[i] = GlyphMerge + } + } + for _, i := range forking { + if i < len(cells) { + cells[i] = GlyphFork + } + } + if lane < len(cells) { + cells[lane] = GlyphCommit + } + return cells +} + +// knownParents drops parent edges pointing outside the walked set, so a +// truncated history closes its lanes instead of waiting for commits that will +// never arrive. Duplicates are dropped too (a merge of a commit with itself is +// malformed, but git has produced stranger things). +func knownParents(parents []string, present map[string]struct{}) []string { + if len(parents) == 0 { + return nil + } + out := make([]string, 0, len(parents)) + seen := make(map[string]struct{}, len(parents)) + for _, p := range parents { + if p == "" { + continue + } + if _, ok := present[p]; !ok { + continue + } + if _, dup := seen[p]; dup { + continue + } + seen[p] = struct{}{} + out = append(out, p) + } + return out +} + +func indexOf(lanes []string, hash string) int { + for i, l := range lanes { + if l == hash { + return i + } + } + return -1 +} + +// firstFree returns the leftmost released column, or len(lanes) when a new one +// must be appended. Reusing released columns is what keeps the graph narrow +// instead of drifting right with every merge. +func firstFree(lanes []string) int { + for i, l := range lanes { + if l == "" { + return i + } + } + return len(lanes) +} + +// isMatched treats a nil map as "no filter is active", so an unfiltered graph +// highlights every commit rather than none. +func isMatched(matched map[string]bool, hash string) bool { + if matched == nil { + return true + } + return matched[hash] +} diff --git a/internal/graph/layout_test.go b/internal/graph/layout_test.go new file mode 100644 index 0000000..d23f166 --- /dev/null +++ b/internal/graph/layout_test.go @@ -0,0 +1,262 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package graph + +import ( + "strings" + "testing" +) + +// c builds a commit from a hash and its parents, keeping the DAG literals in +// these tests readable. +func c(hash string, parents ...string) Commit { + return Commit{Hash: hash, Parents: parents} +} + +// render draws the lane columns as text so a test failure shows the shape that +// was produced, not a slice of integers. It mirrors what the real renderer does +// but stays deliberately dumb — this package is about lane assignment, and the +// glyph-to-rune mapping lives with the renderer. +func render(rows []Row) string { + var sb strings.Builder + for _, r := range rows { + for _, g := range r.Cells { + switch g { + case GlyphCommit: + sb.WriteByte('*') + case GlyphVertical: + sb.WriteByte('|') + case GlyphFork: + sb.WriteByte('\\') + case GlyphMerge: + sb.WriteByte('/') + default: + sb.WriteByte(' ') + } + } + sb.WriteByte(' ') + sb.WriteString(r.Commit.Hash) + sb.WriteByte('\n') + } + return sb.String() +} + +func lanes(rows []Row) []int { + out := make([]int, len(rows)) + for i, r := range rows { + out[i] = r.Lane + } + return out +} + +func equalInts(a, b []int) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func TestLayoutEmpty(t *testing.T) { + if got := Layout(nil, nil); got != nil { + t.Fatalf("empty input should yield nil, got %#v", got) + } + if got := Layout([]Commit{}, nil); got != nil { + t.Fatalf("empty slice should yield nil, got %#v", got) + } +} + +func TestLayoutLinearHistoryStaysInOneLane(t *testing.T) { + rows := Layout([]Commit{ + c("a", "b"), + c("b", "c"), + c("c"), + }, nil) + + if len(rows) != 3 { + t.Fatalf("expected 3 rows, got %d", len(rows)) + } + if !equalInts(lanes(rows), []int{0, 0, 0}) { + t.Errorf("linear history must not widen; lanes = %v\n%s", lanes(rows), render(rows)) + } + for i, r := range rows { + if r.Width() != 1 { + t.Errorf("row %d width = %d, want 1\n%s", i, r.Width(), render(rows)) + } + if r.Cells[0] != GlyphCommit { + t.Errorf("row %d should mark its own commit\n%s", i, render(rows)) + } + } +} + +func TestLayoutRootCommitClosesItsLane(t *testing.T) { + // The last commit has no parents; nothing should still be pending. + rows := Layout([]Commit{c("a", "b"), c("b")}, nil) + last := rows[len(rows)-1] + if last.Cells[0] != GlyphCommit { + t.Fatalf("root commit should occupy its lane\n%s", render(rows)) + } +} + +func TestLayoutForkAndMerge(t *testing.T) { + // m is a merge of a (first parent) and b (second parent); both descend + // from r. + // + // m + // |\ + // a b + // |/ + // r + rows := Layout([]Commit{ + c("m", "a", "b"), + c("a", "r"), + c("b", "r"), + c("r"), + }, nil) + + if !equalInts(lanes(rows), []int{0, 0, 1, 0}) { + t.Fatalf("unexpected lane assignment %v\n%s", lanes(rows), render(rows)) + } + // The merge row must open a second column for the second parent. + if rows[0].Width() < 2 || rows[0].Cells[1] != GlyphFork { + t.Errorf("merge commit should fork a lane for its second parent\n%s", render(rows)) + } + // Both sides converge on r, so the second lane folds back in. + if rows[3].Cells[1] != GlyphMerge { + t.Errorf("converging lane should be marked as merging\n%s", render(rows)) + } +} + +func TestLayoutReleasedLaneIsReused(t *testing.T) { + // Two independent branch tips, the first of which terminates before the + // second one appears. The freed column must be reused rather than the + // graph drifting rightwards. + rows := Layout([]Commit{ + c("a"), // tip 1, root — opens and immediately closes lane 0 + c("b", "c"), // tip 2 — should reuse lane 0 + c("c"), + }, nil) + + if !equalInts(lanes(rows), []int{0, 0, 0}) { + t.Fatalf("freed lane should be reused; lanes = %v\n%s", lanes(rows), render(rows)) + } + for _, r := range rows { + if r.Width() != 1 { + t.Fatalf("graph should stay one column wide\n%s", render(rows)) + } + } +} + +func TestLayoutOctopusMergeOpensALanePerExtraParent(t *testing.T) { + rows := Layout([]Commit{ + c("m", "a", "b", "d"), + c("a", "r"), + c("b", "r"), + c("d", "r"), + c("r"), + }, nil) + + if rows[0].Width() != 3 { + t.Fatalf("a 3-parent merge needs 3 lanes, got %d\n%s", rows[0].Width(), render(rows)) + } + if rows[0].Cells[1] != GlyphFork || rows[0].Cells[2] != GlyphFork { + t.Errorf("both extra parents should fork\n%s", render(rows)) + } + if !equalInts(lanes(rows), []int{0, 0, 1, 2, 0}) { + t.Errorf("unexpected octopus lanes %v\n%s", lanes(rows), render(rows)) + } +} + +func TestLayoutParentOutsideTheWalkClosesTheLane(t *testing.T) { + // A truncated walk: `a`'s parent was never handed to Layout. The lane must + // close at the boundary instead of staying open for a commit that will + // never arrive (which would leave a dangling column down the whole graph). + rows := Layout([]Commit{c("a", "missing")}, nil) + + if len(rows) != 1 { + t.Fatalf("expected 1 row, got %d", len(rows)) + } + if rows[0].Width() != 1 || rows[0].Cells[0] != GlyphCommit { + t.Fatalf("boundary commit should occupy exactly its own lane\n%s", render(rows)) + } +} + +func TestLayoutSharedSecondParentDoesNotOpenADuplicateLane(t *testing.T) { + // Both parents of m are already-pending or identical targets; the graph + // must not allocate a column for a hash it is already waiting on. + rows := Layout([]Commit{ + c("m", "a", "a"), + c("a"), + }, nil) + + if rows[0].Width() != 1 { + t.Fatalf("duplicate parent should not widen the graph\n%s", render(rows)) + } +} + +func TestLayoutMatchedMarking(t *testing.T) { + commits := []Commit{c("a", "b"), c("b", "c"), c("c")} + + // A nil map means no filter is active — everything is "matched" so an + // unfiltered graph is not drawn entirely as context. + for i, r := range Layout(commits, nil) { + if !r.Matched { + t.Errorf("row %d: nil matched map should mark every commit", i) + } + } + + rows := Layout(commits, map[string]bool{"b": true}) + want := []bool{false, true, false} + for i, r := range rows { + if r.Matched != want[i] { + t.Errorf("row %d (%s): Matched = %v, want %v", i, r.Commit.Hash, r.Matched, want[i]) + } + } +} + +func TestLayoutRowsAreUniformWidth(t *testing.T) { + // The renderer indexes columns without bounds checks, so every row must be + // padded to the widest. + rows := Layout([]Commit{ + c("m", "a", "b"), + c("a", "r"), + c("b", "r"), + c("r"), + }, nil) + + width := rows[0].Width() + for i, r := range rows { + if r.Width() != width { + t.Fatalf("row %d width = %d, want %d (all rows padded)\n%s", + i, r.Width(), width, render(rows)) + } + } +} + +func TestLayoutExactlyOneCommitGlyphPerRow(t *testing.T) { + rows := Layout([]Commit{ + c("m", "a", "b"), + c("a", "r"), + c("b", "r"), + c("r"), + }, nil) + + for i, r := range rows { + n := 0 + for _, g := range r.Cells { + if g == GlyphCommit { + n++ + } + } + if n != 1 { + t.Errorf("row %d has %d commit glyphs, want exactly 1\n%s", i, n, render(rows)) + } + if r.Cells[r.Lane] != GlyphCommit { + t.Errorf("row %d: Lane %d does not hold the commit glyph\n%s", i, r.Lane, render(rows)) + } + } +} diff --git a/internal/i18n/messages.en.yml b/internal/i18n/messages.en.yml index 031e713..0c859e0 100644 --- a/internal/i18n/messages.en.yml +++ b/internal/i18n/messages.en.yml @@ -296,3 +296,18 @@ upgrade.err_tool_missing: "%s install detected, but %q was not found on PATH" upgrade.verify_failed: "warning: could not re-run the upgraded binary to verify it: %v" upgrade.verify_mismatch: "warning: the upgraded binary reports %q, not the expected %q" upgrade.verify_shadowed: "warning: %q was upgraded, but %q comes first on PATH and will run instead" + +map.flag_conflict_format: "map draws a graph, not findings: --json / --markdown don't apply. Use --output to write the graph to a file." +map.flag_conflict_review: "map emits no findings, so --fail-on, --min-severity and --suggest-commit have nothing to act on." +map.branches_flag_conflict: "map --branches lists branches, so %s can't narrow it. Drop --branches to see the filtered commit graph." +map.no_commits: "No commits to draw." +map.no_branches: "No branches found." +map.truncated: "⚠ graph truncated at %d commits; pass --max-commits to draw more." + +leaks.scanned: "Scanned %d file(s) across %d commit(s)." +leaks.skipped: "Skipped %d file(s): binary or over the size cap." +leaks.truncated: "⚠ history truncated at %d commits; pass --max-commits to scan further back." +leaks.clean: "No credentials found." +leaks.found: "%d possible credential(s) found. Rotate them — a key in git history stays reachable in every existing clone." +leaks.nothing_to_scan: "--no-worktree and --no-history together leave nothing to scan." +leaks.gate_failed: "failing: %d credential finding(s). Pass --fail-on none to report without failing." diff --git a/internal/i18n/messages.tr.yml b/internal/i18n/messages.tr.yml index 65fe098..df229eb 100644 --- a/internal/i18n/messages.tr.yml +++ b/internal/i18n/messages.tr.yml @@ -295,3 +295,18 @@ upgrade.err_tool_missing: "%s kurulumu tespit edildi ama %q PATH'te bulunamadı" upgrade.verify_failed: "uyarı: güncellenen binary doğrulama için çalıştırılamadı: %v" upgrade.verify_mismatch: "uyarı: güncellenen binary %q bildiriyor, beklenen %q değil" upgrade.verify_shadowed: "uyarı: %q güncellendi, ama PATH'te önce %q geliyor ve onun yerine o çalışacak" + +map.flag_conflict_format: "map graf çizer, finding üretmez: --json / --markdown geçerli değil. Grafı dosyaya yazmak için --output kullanın." +map.flag_conflict_review: "map finding üretmez; --fail-on, --min-severity ve --suggest-commit üzerinde işlem yapacak bir şey yok." +map.branches_flag_conflict: "map --branches branch listeler; bu yüzden %s ile daraltılamaz. Filtrelenmiş commit grafiği için --branches'i kaldırın." +map.no_commits: "Çizilecek commit yok." +map.no_branches: "Branch bulunamadı." +map.truncated: "⚠ graf %d commit'te kesildi; daha fazlası için --max-commits kullanın." + +leaks.scanned: "%d dosya, %d commit tarandı." +leaks.skipped: "%d dosya atlandı: ikili ya da boyut sınırının üstünde." +leaks.truncated: "⚠ geçmiş %d commit'te kesildi; daha geriye gitmek için --max-commits kullanın." +leaks.clean: "Kimlik bilgisi bulunamadı." +leaks.found: "%d olası kimlik bilgisi bulundu. Bunları döndürün (rotate) — git geçmişindeki bir anahtar mevcut tüm klonlarda erişilebilir kalır." +leaks.nothing_to_scan: "--no-worktree ve --no-history birlikte kullanılınca taranacak bir şey kalmıyor." +leaks.gate_failed: "başarısız: %d kimlik bilgisi bulgusu. Hata vermeden raporlamak için --fail-on none kullanın." diff --git a/internal/leaks/binary.go b/internal/leaks/binary.go new file mode 100644 index 0000000..9911d5a --- /dev/null +++ b/internal/leaks/binary.go @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package leaks + +import "bytes" + +// DefaultMaxFileBytes caps how large a file may be before the scanner passes +// over it. The patterns all target short, structured tokens; a file this size +// is a database dump or a bundled asset, and running eight regexes over every +// line of it costs far more than it can plausibly find. +const DefaultMaxFileBytes = 5 << 20 // 5 MiB + +// binarySniffBytes is how much of a file is inspected for the NUL byte that +// marks it as non-text. This is the same heuristic git itself uses, and 8 KiB +// is enough to catch every real binary format's header. +const binarySniffBytes = 8 << 10 + +// isBinary reports whether content looks like a binary blob. +// +// The repo had no content-based binary detection before this: diff.FileDiff's +// Binary flag is parsed out of git's own "Binary files …" sentinel, which is +// unavailable when reading a file straight off disk. A NUL byte in the first +// 8 KiB is the standard, cheap test — valid UTF-8 text never contains one. +func isBinary(content []byte) bool { + head := content + if len(head) > binarySniffBytes { + head = head[:binarySniffBytes] + } + return bytes.IndexByte(head, 0) >= 0 +} + +// maxFileBytes resolves the effective size cap. +func maxFileBytes(opts Options) int64 { + if opts.MaxFileBytes > 0 { + return opts.MaxFileBytes + } + return DefaultMaxFileBytes +} diff --git a/internal/leaks/history.go b/internal/leaks/history.go new file mode 100644 index 0000000..b3966ba --- /dev/null +++ b/internal/leaks/history.go @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package leaks + +import ( + "context" + + "github.com/CommitBrief/commitbrief/internal/diff" + "github.com/CommitBrief/commitbrief/internal/git" +) + +// ScanHistory reports credential-shaped content in the ADDED lines of the +// selected commits. +// +// Added lines only, deliberately: a secret that was committed and later +// removed is still in the history — reachable by anyone who clones the repo — +// and finding exactly that is the point of scanning history at all. Scanning +// context lines instead would re-report the same key once per commit that +// touched the file near it. +// +// Each commit's patch is fetched and scanned separately rather than as one +// concatenated blob, because a finding is only actionable with attribution: +// which commit introduced it, by whom, and when decides whether the key still +// needs rotating. +func ScanHistory(ctx context.Context, repoRoot string, sel git.Selection, opts Options) (Result, error) { + scan, err := newScanner(opts) + if err != nil { + return Result{}, err + } + + res := Result{Truncated: sel.Truncated} + matcher := opts.Matcher + + for _, commit := range sel.Commits { + patch, pErr := git.PatchesFor(ctx, repoRoot, []git.CommitMeta{commit}) + if pErr != nil { + return Result{}, pErr + } + res.CommitsScanned++ + if patch == "" { + continue + } + + parsed, parseErr := diff.Parse(git.Diff{Content: patch, Origin: git.OriginFiltered}) + if parseErr != nil { + // A commit whose patch will not parse (an exotic mode change, a + // malformed submodule entry) should not sink the whole scan. + continue + } + if matcher != nil { + parsed = diff.Filter(parsed, matcher) + } + parsed, parseErr = diff.KeepPaths(parsed, opts.Files, opts.Dirs) + if parseErr != nil { + return Result{}, parseErr + } + parsed, parseErr = diff.DropPaths(parsed, opts.ExcludeFiles, opts.ExcludeDirs) + if parseErr != nil { + return Result{}, parseErr + } + + for _, f := range parsed.Files { + if f.Binary { + res.Skipped++ + continue + } + res.FilesScanned++ + for _, hit := range scanAddedLines(f, scan) { + hit.Commit = commit.Hash + hit.Short = commit.Short + hit.Author = commit.Author + hit.Date = commit.Date + res.Findings = append(res.Findings, hit) + } + } + } + + res.Sort() + return res, nil +} + +// scanAddedLines scans one file's added lines, translating each hunk offset +// into a real line number in that commit's post-image. +// +// The mapping matters: guard's SecretMatch.Line counts lines within the diff +// *string*, which is meaningless once the diff is gone. Walking the hunks and +// tracking NewStart gives a number that actually points at the file as that +// commit left it — the number a reader needs to go look. +func scanAddedLines(f diff.FileDiff, scan scanFunc) []Finding { + var out []Finding + for _, h := range f.Hunks { + line := h.NewStart + for _, l := range h.Lines { + switch l.Kind { + case diff.LineDel: + // Deleted lines do not exist in the post-image, so they do not + // advance the new-side counter. + continue + case diff.LineAdd: + if matches := scan(l.Text); len(matches) > 0 { + out = append(out, Finding{ + File: f.Path, + Line: line, + Patterns: matches[0].Patterns, + }) + } + } + line++ + } + } + return out +} diff --git a/internal/leaks/leaks.go b/internal/leaks/leaks.go new file mode 100644 index 0000000..203f6ef --- /dev/null +++ b/internal/leaks/leaks.go @@ -0,0 +1,169 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +// Package leaks scans a repository for credential-shaped content (ADR-0036). +// +// It is the standalone counterpart to the pre-send scanner in internal/guard. +// That one is a gate: it sees the added lines of the one diff about to be sent +// to a provider, and its job is to stop the send. This one is an audit: it +// reads whole files in the working tree and the added lines of arbitrary +// historical commits, and its job is to tell you what is there. +// +// The pattern set is not duplicated — guard owns it, including the ADR-0024 +// user extensions, and this package drives it over two new inputs. guard stays +// a leaf (it must not import internal/git); the orchestration lives here. +// +// # The redaction invariant +// +// A Finding records a file, a line number and the pattern names that matched. +// It NEVER records the matched text. That is the ADR-0007 invariant, restated +// in engineering/standards/security.md, and it matters more here than in guard +// because this package reads whole files and writes a report the user is +// likely to paste somewhere. There is a test asserting the secret never +// reaches the output; keep it. +package leaks + +import ( + "sort" + "time" + + "github.com/CommitBrief/commitbrief/internal/guard" + "github.com/CommitBrief/commitbrief/internal/ignore" +) + +// Finding is one credential-shaped hit. +type Finding struct { + // File is the repo-relative, slash-normalized path. + File string + // Line is 1-based: the line in the file for a worktree hit, or the line in + // the commit's post-image for a history hit. + Line int + // Patterns are the alphabetised names of every pattern that matched. + Patterns []string + + // Commit attribution, empty for a worktree hit. Knowing who introduced a + // key and when is what decides whether it still needs rotating. + Commit string + Short string + Author string + Date time.Time +} + +// FromHistory reports whether this hit came from a commit rather than the +// current working tree. +func (f Finding) FromHistory() bool { return f.Commit != "" } + +// Options are the inputs shared by both scan halves. +type Options struct { + // Patterns is the compiled user-pattern set from guard.CompileUserPatterns. + // The built-ins always run regardless; this is additive only (ADR-0024). + Patterns []guard.UserSecretPattern + + // Matcher is the composed ignore layer set (built-ins + .commitbriefignore). + // nil means no ignore filtering. + Matcher *ignore.Matcher + + // Files/Dirs narrow to these paths; ExcludeFiles/ExcludeDirs remove from + // the result. Same semantics as the review's --file/--dir/--exclude-*. + Files []string + Dirs []string + ExcludeFiles []string + ExcludeDirs []string + + // MaxFileBytes caps how large a file may be before it is skipped; 0 uses + // DefaultMaxFileBytes. + MaxFileBytes int64 +} + +// Result is one half's outcome. +type Result struct { + Findings []Finding + + // FilesScanned counts files actually read (worktree) or file entries + // examined (history). + FilesScanned int + // CommitsScanned is 0 for the worktree half. + CommitsScanned int + // Skipped counts files passed over as binary or oversized. Reported, never + // silent — a scanner that quietly ignores half a repo is worse than none. + Skipped int + // Truncated is set when the commit walk hit its cap. + Truncated bool +} + +// Merge folds another Result into this one, concatenating findings and summing +// the counters. Used to combine the worktree and history halves into one report. +func (r Result) Merge(other Result) Result { + return Result{ + Findings: append(append([]Finding{}, r.Findings...), other.Findings...), + FilesScanned: r.FilesScanned + other.FilesScanned, + CommitsScanned: r.CommitsScanned + other.CommitsScanned, + Skipped: r.Skipped + other.Skipped, + Truncated: r.Truncated || other.Truncated, + } +} + +// Sort orders findings deterministically: worktree hits before history hits, +// then by path, then by line, then by commit. Two runs over an unchanged repo +// must produce byte-identical reports, or diffing two scans is useless. +func (r *Result) Sort() { + sort.SliceStable(r.Findings, func(i, j int) bool { + a, b := r.Findings[i], r.Findings[j] + if a.FromHistory() != b.FromHistory() { + return !a.FromHistory() + } + if a.File != b.File { + return a.File < b.File + } + if a.Line != b.Line { + return a.Line < b.Line + } + return a.Commit < b.Commit + }) +} + +// scanFunc scans arbitrary text and returns the matching lines. +// +// This exists because guard.CompileUserPatterns returns an *unexported* type: +// a caller outside guard can hold the value (type inference) but cannot name +// it, so it can never become a struct field or an explicit parameter. Wrapping +// the compiled set in a closure captures it without naming it — and keeps +// guard's API untouched, which is the point (its narrow surface is what keeps +// it a leaf package). +type scanFunc func(content string) []guard.SecretMatch + +// newScanner compiles the user patterns once and returns a scanner over the +// effective set (built-ins ++ user, built-ins winning on a name collision — +// ADR-0024). An invalid user regex fails here, before any file is read. +func newScanner(opts Options) (scanFunc, error) { + extra, err := guard.CompileUserPatterns(opts.Patterns) + if err != nil { + return nil, err + } + return func(content string) []guard.SecretMatch { + return guard.ScanTextWith(content, extra) + }, nil +} + +// patternSeverityHigh names the built-in patterns that do NOT warrant +// `critical`. A JWT is the one built-in with a real false-positive rate — +// expired or sample tokens are common in fixtures and docs — so it is reported +// a notch lower rather than being dropped or crying wolf. +var patternSeverityHigh = map[string]struct{}{ + "JWT": {}, +} + +// IsHighNotCritical reports whether a pattern name should be surfaced as +// `high` rather than `critical`. User patterns (ADR-0024) are also `high`: +// they are somebody's house format and this package cannot assert how bad a +// hit really is. +func IsHighNotCritical(pattern string, userPatterns []guard.UserSecretPattern) bool { + if _, ok := patternSeverityHigh[pattern]; ok { + return true + } + for _, p := range userPatterns { + if p.Name == pattern { + return true + } + } + return false +} diff --git a/internal/leaks/leaks_test.go b/internal/leaks/leaks_test.go new file mode 100644 index 0000000..1a01af8 --- /dev/null +++ b/internal/leaks/leaks_test.go @@ -0,0 +1,429 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package leaks + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/CommitBrief/commitbrief/internal/git" + "github.com/CommitBrief/commitbrief/internal/guard" + "github.com/CommitBrief/commitbrief/internal/ignore" +) + +// A syntactically valid AWS key that matches the built-in pattern. It is a +// well-known documentation placeholder, not a real credential. +const fakeAWSKey = "AKIAIOSFODNN7EXAMPLE" + +type repo struct{ dir string } + +func newRepo(t *testing.T) *repo { + t.Helper() + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not on PATH") + } + r := &repo{dir: t.TempDir()} + r.git(t, "init", "-q", "-b", "main") + r.git(t, "config", "user.name", "Test") + r.git(t, "config", "user.email", "test@example.com") + r.git(t, "config", "commit.gpgsign", "false") + return r +} + +func (r *repo) git(t *testing.T, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = r.dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } +} + +func (r *repo) write(t *testing.T, rel, content string) { + t.Helper() + path := filepath.Join(r.dir, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func (r *repo) commit(t *testing.T, msg string, paths ...string) { + t.Helper() + args := append([]string{"add"}, paths...) + r.git(t, args...) + r.git(t, "commit", "-q", "-m", msg) +} + +func mustScanTree(t *testing.T, r *repo, opts Options) Result { + t.Helper() + res, err := ScanWorktree(context.Background(), r.dir, opts) + if err != nil { + t.Fatalf("ScanWorktree: %v", err) + } + return res +} + +func files(res Result) []string { + out := make([]string, 0, len(res.Findings)) + for _, f := range res.Findings { + out = append(out, fmt.Sprintf("%s:%d", f.File, f.Line)) + } + return out +} + +// ---------- worktree ---------- + +func TestScanWorktreeFindsTrackedSecret(t *testing.T) { + r := newRepo(t) + r.write(t, "config.yml", "key: "+fakeAWSKey+"\n") + r.write(t, "clean.go", "package app\n") + r.commit(t, "initial", "config.yml", "clean.go") + + res := mustScanTree(t, r, Options{}) + if len(res.Findings) != 1 { + t.Fatalf("expected 1 finding, got %v", files(res)) + } + f := res.Findings[0] + if f.File != "config.yml" || f.Line != 1 { + t.Errorf("finding = %s:%d, want config.yml:1", f.File, f.Line) + } + if len(f.Patterns) == 0 || f.Patterns[0] != "AWS Access Key" { + t.Errorf("patterns = %v, want [AWS Access Key]", f.Patterns) + } + if f.FromHistory() { + t.Error("a worktree hit must not claim commit attribution") + } + if res.FilesScanned != 2 { + t.Errorf("FilesScanned = %d, want 2", res.FilesScanned) + } +} + +// The invariant that matters most: the scanner reads whole files, so it must +// never echo what it found. See engineering/standards/security.md. +func TestScanWorktreeNeverRecordsTheSecret(t *testing.T) { + r := newRepo(t) + r.write(t, "config.yml", "key: "+fakeAWSKey+"\n") + r.commit(t, "initial", "config.yml") + + res := mustScanTree(t, r, Options{}) + if len(res.Findings) == 0 { + t.Fatal("expected a finding to check") + } + dump := fmt.Sprintf("%#v", res) + if strings.Contains(dump, fakeAWSKey) { + t.Fatalf("the matched secret leaked into the Result:\n%s", dump) + } +} + +func TestScanWorktreeIgnoresUntrackedFiles(t *testing.T) { + // An untracked, usually-gitignored .env is where a secret is SUPPOSED to + // live. Flagging it would be noise, and it cannot leak through git. + r := newRepo(t) + r.write(t, "clean.go", "package app\n") + r.commit(t, "initial", "clean.go") + r.write(t, ".env", "AWS_KEY="+fakeAWSKey+"\n") + + res := mustScanTree(t, r, Options{}) + if len(res.Findings) != 0 { + t.Fatalf("untracked files must not be scanned, got %v", files(res)) + } +} + +func TestScanWorktreeHonorsIgnoreLayers(t *testing.T) { + r := newRepo(t) + r.write(t, "vendor/lib.go", "key := \""+fakeAWSKey+"\"\n") + r.write(t, "app.go", "package app\n") + r.commit(t, "initial", "vendor/lib.go", "app.go") + + res := mustScanTree(t, r, Options{Matcher: ignore.Builtin()}) + if len(res.Findings) != 0 { + t.Fatalf("vendor/** is a built-in ignore; got %v", files(res)) + } + + // Without the matcher the same file is reported — proving the layer, not + // some other filter, is what excluded it. + if got := mustScanTree(t, r, Options{}); len(got.Findings) != 1 { + t.Fatalf("without the ignore layer the hit should surface, got %v", files(got)) + } +} + +func TestScanWorktreeHonorsPathFilters(t *testing.T) { + r := newRepo(t) + r.write(t, "src/a.yml", "key: "+fakeAWSKey+"\n") + r.write(t, "docs/b.yml", "key: "+fakeAWSKey+"\n") + r.commit(t, "initial", "src/a.yml", "docs/b.yml") + + res := mustScanTree(t, r, Options{Dirs: []string{"src"}}) + if len(res.Findings) != 1 || res.Findings[0].File != "src/a.yml" { + t.Fatalf("--dir should narrow to src/, got %v", files(res)) + } + + res = mustScanTree(t, r, Options{ExcludeDirs: []string{"docs"}}) + if len(res.Findings) != 1 || res.Findings[0].File != "src/a.yml" { + t.Fatalf("--exclude-dir should drop docs/, got %v", files(res)) + } +} + +func TestScanWorktreeSkipsBinaryAndOversizedFiles(t *testing.T) { + r := newRepo(t) + // A NUL byte in the first 8 KiB marks the file binary; the key that + // follows must not be reported. + r.write(t, "blob.bin", "\x00\x01\x02 "+fakeAWSKey+"\n") + r.write(t, "big.txt", strings.Repeat("x", 64)+"\n"+fakeAWSKey+"\n") + r.commit(t, "initial", "blob.bin", "big.txt") + + res := mustScanTree(t, r, Options{MaxFileBytes: 32}) + if len(res.Findings) != 0 { + t.Fatalf("binary and oversized files must be skipped, got %v", files(res)) + } + if res.Skipped != 2 { + t.Errorf("Skipped = %d, want 2 (skips are reported, never silent)", res.Skipped) + } +} + +func TestScanWorktreeUserPatterns(t *testing.T) { + r := newRepo(t) + r.write(t, "svc.conf", "token = INT-0123456789\n") + r.commit(t, "initial", "svc.conf") + + if res := mustScanTree(t, r, Options{}); len(res.Findings) != 0 { + t.Fatalf("the house format should not match a built-in, got %v", files(res)) + } + + res := mustScanTree(t, r, Options{ + Patterns: []guard.UserSecretPattern{{Name: "Internal Token", Regex: `INT-[0-9]{10}`}}, + }) + if len(res.Findings) != 1 || res.Findings[0].Patterns[0] != "Internal Token" { + t.Fatalf("user pattern should match, got %v", res.Findings) + } +} + +func TestScanWorktreeInvalidUserPatternFailsBeforeReadingAnything(t *testing.T) { + r := newRepo(t) + r.write(t, "a.txt", "hello\n") + r.commit(t, "initial", "a.txt") + + _, err := ScanWorktree(context.Background(), r.dir, Options{ + Patterns: []guard.UserSecretPattern{{Name: "Broken", Regex: "([unclosed"}}, + }) + if err == nil { + t.Fatal("an invalid user regex must fail the scan, not be skipped") + } + if !strings.Contains(err.Error(), "Broken") { + t.Errorf("error should name the offending pattern; got %v", err) + } +} + +func TestScanWorktreeCleanRepo(t *testing.T) { + r := newRepo(t) + r.write(t, "a.go", "package app\n") + r.commit(t, "initial", "a.go") + + res := mustScanTree(t, r, Options{}) + if len(res.Findings) != 0 { + t.Fatalf("clean repo should yield no findings, got %v", files(res)) + } +} + +// ---------- history ---------- + +func selectAll(t *testing.T, r *repo) git.Selection { + t.Helper() + sel, err := git.SelectCommits(context.Background(), r.dir, git.CommitFilter{ + Text: "", Rev: []string{"HEAD"}, MaxCommits: 50, + }) + if err != nil { + t.Fatalf("SelectCommits: %v", err) + } + // CommitFilter.Active() is false with no predicate, but SelectCommits + // still walks — which is what this helper wants. + return sel +} + +// The whole reason to scan history: a secret that was committed and later +// removed is still reachable in the repo. +func TestScanHistoryFindsRemovedSecret(t *testing.T) { + r := newRepo(t) + r.write(t, "config.yml", "key: "+fakeAWSKey+"\n") + r.commit(t, "add config", "config.yml") + r.write(t, "config.yml", "key: REDACTED\n") + r.commit(t, "scrub the key", "config.yml") + + // It is gone from the tree... + if res := mustScanTree(t, r, Options{}); len(res.Findings) != 0 { + t.Fatalf("the working tree is clean; got %v", files(res)) + } + + // ...but not from the history. + res, err := ScanHistory(context.Background(), r.dir, selectAll(t, r), Options{}) + if err != nil { + t.Fatalf("ScanHistory: %v", err) + } + if len(res.Findings) != 1 { + t.Fatalf("expected the historical hit, got %v", files(res)) + } + f := res.Findings[0] + if f.File != "config.yml" || f.Line != 1 { + t.Errorf("finding = %s:%d, want config.yml:1", f.File, f.Line) + } + if !f.FromHistory() || f.Short == "" || f.Author != "Test" || f.Date.IsZero() { + t.Errorf("history hit needs full attribution, got %#v", f) + } +} + +func TestScanHistoryNeverRecordsTheSecret(t *testing.T) { + r := newRepo(t) + r.write(t, "config.yml", "key: "+fakeAWSKey+"\n") + r.commit(t, "add config", "config.yml") + + res, err := ScanHistory(context.Background(), r.dir, selectAll(t, r), Options{}) + if err != nil { + t.Fatalf("ScanHistory: %v", err) + } + if len(res.Findings) == 0 { + t.Fatal("expected a finding to check") + } + if dump := fmt.Sprintf("%#v", res); strings.Contains(dump, fakeAWSKey) { + t.Fatalf("the matched secret leaked into the Result:\n%s", dump) + } +} + +func TestScanHistoryReportsRealFileLineNumbers(t *testing.T) { + // guard's SecretMatch.Line counts lines within the diff string, which is + // meaningless once the diff is gone. The reported number must point at the + // file as that commit left it. + r := newRepo(t) + r.write(t, "app.conf", "alpha\nbeta\ngamma\ndelta\n") + r.commit(t, "initial", "app.conf") + r.write(t, "app.conf", "alpha\nbeta\ngamma\ndelta\nkey = "+fakeAWSKey+"\n") + r.commit(t, "append key", "app.conf") + + res, err := ScanHistory(context.Background(), r.dir, selectAll(t, r), Options{}) + if err != nil { + t.Fatalf("ScanHistory: %v", err) + } + if len(res.Findings) != 1 { + t.Fatalf("expected 1 finding, got %v", files(res)) + } + if res.Findings[0].Line != 5 { + t.Errorf("Line = %d, want 5 (the real line in the post-image)", res.Findings[0].Line) + } +} + +func TestScanHistoryHonorsPathFilters(t *testing.T) { + r := newRepo(t) + r.write(t, "src/a.yml", "key: "+fakeAWSKey+"\n") + r.write(t, "docs/b.yml", "key: "+fakeAWSKey+"\n") + r.commit(t, "initial", "src/a.yml", "docs/b.yml") + + res, err := ScanHistory(context.Background(), r.dir, selectAll(t, r), Options{Dirs: []string{"src"}}) + if err != nil { + t.Fatalf("ScanHistory: %v", err) + } + if len(res.Findings) != 1 || res.Findings[0].File != "src/a.yml" { + t.Fatalf("--dir should narrow the history scan too, got %v", files(res)) + } +} + +func TestScanHistoryEmptySelection(t *testing.T) { + r := newRepo(t) + r.write(t, "a.go", "package app\n") + r.commit(t, "initial", "a.go") + + res, err := ScanHistory(context.Background(), r.dir, git.Selection{}, Options{}) + if err != nil { + t.Fatalf("ScanHistory: %v", err) + } + if len(res.Findings) != 0 || res.CommitsScanned != 0 { + t.Fatalf("an empty selection scans nothing, got %#v", res) + } +} + +func TestScanHistoryPropagatesTruncation(t *testing.T) { + r := newRepo(t) + r.write(t, "a.go", "package app\n") + r.commit(t, "initial", "a.go") + + res, err := ScanHistory(context.Background(), r.dir, + git.Selection{Truncated: true}, Options{}) + if err != nil { + t.Fatalf("ScanHistory: %v", err) + } + if !res.Truncated { + t.Error("a truncated selection must stay truncated in the result") + } +} + +// ---------- shared ---------- + +func TestResultMergeAndSort(t *testing.T) { + tree := Result{ + Findings: []Finding{{File: "b.txt", Line: 2}}, + FilesScanned: 3, + Skipped: 1, + } + hist := Result{ + Findings: []Finding{{File: "a.txt", Line: 1, Commit: "abc"}}, + FilesScanned: 2, + CommitsScanned: 4, + Truncated: true, + } + + merged := tree.Merge(hist) + if merged.FilesScanned != 5 || merged.CommitsScanned != 4 || merged.Skipped != 1 { + t.Errorf("counters not summed: %#v", merged) + } + if !merged.Truncated { + t.Error("truncation must survive a merge") + } + + merged.Sort() + // Worktree hits sort before history hits regardless of path, so the report + // leads with what is on disk right now. + if merged.Findings[0].FromHistory() { + t.Errorf("worktree findings should sort first, got %#v", merged.Findings) + } +} + +func TestIsHighNotCritical(t *testing.T) { + user := []guard.UserSecretPattern{{Name: "Internal Token", Regex: "x"}} + + if !IsHighNotCritical("JWT", nil) { + t.Error("JWT is the one built-in with a real false-positive rate; want high") + } + if IsHighNotCritical("AWS Access Key", nil) { + t.Error("AWS Access Key should stay critical") + } + if !IsHighNotCritical("Internal Token", user) { + t.Error("user patterns cannot be asserted critical; want high") + } + if IsHighNotCritical("Internal Token", nil) { + t.Error("an unknown pattern with no user set should stay critical") + } +} + +func TestIsBinary(t *testing.T) { + if !isBinary([]byte("abc\x00def")) { + t.Error("a NUL byte marks the content binary") + } + if isBinary([]byte("plain text\nwith unicode ✓\n")) { + t.Error("UTF-8 text must not be classified binary") + } + if isBinary(nil) { + t.Error("empty content is not binary") + } + // The sniff only inspects the head, so a NUL far past the window is missed + // by design — that is the documented, git-standard trade-off. + tail := append([]byte(strings.Repeat("a", binarySniffBytes+10)), 0) + if isBinary(tail) { + t.Error("the sniff should only inspect the first 8 KiB") + } +} diff --git a/internal/leaks/worktree.go b/internal/leaks/worktree.go new file mode 100644 index 0000000..72c86eb --- /dev/null +++ b/internal/leaks/worktree.go @@ -0,0 +1,141 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package leaks + +import ( + "bytes" + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/CommitBrief/commitbrief/internal/diff" +) + +// ScanWorktree reads every tracked file in the working tree and reports the +// credential-shaped lines in it. +// +// Enumeration is `git ls-files -z`, not filepath.WalkDir, for three reasons: +// it yields exactly the tracked set (an untracked, gitignored .env is where a +// secret is *supposed* to live, so flagging it would be noise); it respects +// .gitignore for free; and it sidesteps ignore.Matcher having no isDir=true +// entry point, which would make directory pruning during a walk subtly wrong. +// +// Unlike guard's diff scan this reads whole files — every line, not just added +// ones — because the question here is "what is in my tree right now", not +// "what am I about to send". +func ScanWorktree(ctx context.Context, repoRoot string, opts Options) (Result, error) { + scan, err := newScanner(opts) + if err != nil { + return Result{}, err + } + paths, err := trackedFiles(ctx, repoRoot) + if err != nil { + return Result{}, err + } + + limit := maxFileBytes(opts) + var res Result + + for _, rel := range paths { + if !keepPath(rel, opts) { + continue + } + abs := filepath.Join(repoRoot, filepath.FromSlash(rel)) + + info, statErr := os.Lstat(abs) + if statErr != nil { + // The index can list a file the tree no longer has (a deletion + // staged elsewhere, a race with another process). Skip it rather + // than failing the whole scan. + continue + } + // A symlink's target may sit outside the repo entirely; following it + // would scan files the user never asked about. + if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() { + continue + } + if info.Size() > limit { + res.Skipped++ + continue + } + + content, readErr := os.ReadFile(abs) + if readErr != nil { + continue + } + if isBinary(content) { + res.Skipped++ + continue + } + + res.FilesScanned++ + for _, m := range scan(string(content)) { + res.Findings = append(res.Findings, Finding{ + File: rel, + Line: m.Line, + Patterns: m.Patterns, + }) + } + } + + res.Sort() + return res, nil +} + +// trackedFiles lists the repo's tracked paths, NUL-separated so a path +// containing a newline (legal on POSIX) cannot split a record. +func trackedFiles(ctx context.Context, repoRoot string) ([]string, error) { + bin, err := exec.LookPath("git") + if err != nil { + return nil, fmt.Errorf("leaks: %w", err) + } + cmd := exec.CommandContext(ctx, bin, "ls-files", "-z") + cmd.Dir = repoRoot + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if runErr := cmd.Run(); runErr != nil { + msg := strings.TrimSpace(stderr.String()) + if msg == "" { + msg = runErr.Error() + } + return nil, fmt.Errorf("git ls-files: %s", msg) + } + + raw := strings.Split(stdout.String(), "\x00") + paths := make([]string, 0, len(raw)) + for _, p := range raw { + if p = strings.TrimSpace(p); p != "" { + paths = append(paths, p) + } + } + return paths, nil +} + +// keepPath applies the ignore layers and the path allow/denylists to one +// repo-relative path. +// +// It reuses diff.KeepPaths / diff.DropPaths by wrapping the path in a +// single-entry diff, so the glob semantics are byte-identical to what +// --file/--dir/--exclude-* do on a review (ADR-0026). Reimplementing the +// matcher here is exactly how the two would drift apart. +func keepPath(rel string, opts Options) bool { + parts := strings.Split(rel, "/") + if opts.Matcher != nil && opts.Matcher.MatchParts(parts) { + return false + } + probe := diff.Diff{Files: []diff.FileDiff{{Path: rel, PathParts: parts}}} + + kept, err := diff.KeepPaths(probe, opts.Files, opts.Dirs) + if err != nil || len(kept.Files) == 0 { + return false + } + kept, err = diff.DropPaths(kept, opts.ExcludeFiles, opts.ExcludeDirs) + if err != nil || len(kept.Files) == 0 { + return false + } + return true +} diff --git a/internal/render/graph.go b/internal/render/graph.go new file mode 100644 index 0000000..d9bd00e --- /dev/null +++ b/internal/render/graph.go @@ -0,0 +1,325 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package render + +import ( + "fmt" + "io" + "strings" + "time" + + "github.com/charmbracelet/lipgloss" + + "github.com/CommitBrief/commitbrief/internal/git" + "github.com/CommitBrief/commitbrief/internal/graph" + "github.com/CommitBrief/commitbrief/internal/ui" +) + +// Commit-graph rendering for `commitbrief map` (ADR-0037). +// +// internal/graph owns lane assignment; this file owns nothing but appearance — +// which rune goes in a cell, what is coloured, and how a row is clipped to the +// terminal. The split is what lets the topology be tested against hand-written +// DAGs with no terminal in sight. + +// GraphOptions controls appearance. The zero value is a safe ASCII, no-colour, +// unclipped render — what a pipe or a test buffer should get. +type GraphOptions struct { + // Color enables ANSI styling. Callers pass ui.ColorEnabled(w, mode). + Color bool + // Unicode selects box-drawing glyphs over the ASCII fallback. Callers + // normally tie this to Color: a terminal that refused ANSI is also the + // one most likely to mangle U+2502. + Unicode bool + // Width is the terminal width; 0 means unknown, so do not clip. + Width int + // Filtered marks that a commit filter was active, which is what makes the + // matched/context distinction meaningful. Without it every commit renders + // as matched and no legend is printed. + Filtered bool + // Now anchors relative dates. Zero means time.Now() — injectable so tests + // are not clock-dependent. + Now time.Time +} + +// graphGlyphs is one rune set. Two exist: box-drawing for a capable terminal, +// ASCII for everything else. +type graphGlyphs struct { + commit string // this row's commit, matched + context string // this row's commit, filtered out + vertical string + fork string + merge string + branchTee string + branchEnd string +} + +var ( + unicodeGlyphs = graphGlyphs{ + commit: "●", context: "○", vertical: "│", fork: "╲", merge: "╱", + branchTee: "├─", branchEnd: "└─", + } + asciiGlyphs = graphGlyphs{ + commit: "*", context: "o", vertical: "|", fork: "\\", merge: "/", + branchTee: "|-", branchEnd: "`-", + } +) + +// Graph colours. Deliberately reusing the palette already established by the +// cards renderer and the progress tree (DESIGN_SYSTEM.md) rather than +// introducing a third one. +var ( + graphLaneStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("#5b6273")) + graphMatchedStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("#22d3a0")) + graphContextStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("#9CA3AF")) + graphHashStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("#e2b714")) + graphRefStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("#7aa2f7")) + graphMetaStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("#9CA3AF")) + graphDimStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("#5b6273")) +) + +// GraphRow pairs a laid-out row with the commit metadata needed to describe it. +type GraphRow struct { + Row graph.Row + Commit git.CommitMeta + Refs []string // branch/tag labels pointing at this commit +} + +// Graph writes the commit DAG. Rows must already be laid out by graph.Layout +// and carry their commit metadata. +func Graph(w io.Writer, rows []GraphRow, opts GraphOptions) error { + if len(rows) == 0 { + return nil + } + g := asciiGlyphs + if opts.Unicode { + g = unicodeGlyphs + } + now := opts.Now + if now.IsZero() { + now = time.Now() + } + + // The lane gutter is fixed-width across all rows so the text columns line + // up; graph.Layout already padded every row to the same cell count. + laneCells := rows[0].Row.Width() + + var sb strings.Builder + for _, r := range rows { + gutter := renderLaneGutter(r.Row, g, opts, laneCells) + text := renderCommitText(r, opts, now) + + line := gutter + " " + text + // Clip rather than wrap: a wrapped row would have no lane gutter on + // its continuation line, which visually detaches it from the graph. + sb.WriteString(ui.Clip(line, opts.Width)) + sb.WriteByte('\n') + } + + if opts.Filtered { + sb.WriteByte('\n') + sb.WriteString(renderGraphLegend(g, opts)) + sb.WriteByte('\n') + } + _, err := io.WriteString(w, sb.String()) + if err != nil { + return fmt.Errorf("render: write graph: %w", err) + } + return nil +} + +// renderLaneGutter draws one row's lane columns. +func renderLaneGutter(row graph.Row, g graphGlyphs, opts GraphOptions, cells int) string { + var sb strings.Builder + for i := 0; i < cells; i++ { + var glyph graph.Glyph + if i < len(row.Cells) { + glyph = row.Cells[i] + } + switch glyph { + case graph.GlyphCommit: + marker := g.commit + style := graphMatchedStyle + if !row.Matched { + marker, style = g.context, graphContextStyle + } + sb.WriteString(paint(marker, style, opts.Color)) + case graph.GlyphVertical: + sb.WriteString(paint(g.vertical, graphLaneStyle, opts.Color)) + case graph.GlyphFork: + sb.WriteString(paint(g.fork, graphLaneStyle, opts.Color)) + case graph.GlyphMerge: + sb.WriteString(paint(g.merge, graphLaneStyle, opts.Color)) + default: + sb.WriteString(" ") + } + } + return sb.String() +} + +// renderCommitText draws everything right of the lane gutter: hash, ref +// labels, subject, author, relative date. +func renderCommitText(r GraphRow, opts GraphOptions, now time.Time) string { + c := r.Commit + parts := make([]string, 0, 5) + parts = append(parts, paint(shortOrHash(c), graphHashStyle, opts.Color)) + + if len(r.Refs) > 0 { + parts = append(parts, paint("("+strings.Join(r.Refs, ", ")+")", graphRefStyle, opts.Color)) + } + + subject := c.Subject + if subject == "" { + subject = "(no subject)" + } + // A filtered-out commit is context, not the answer — dim it so the eye + // lands on what the filter actually selected. + if opts.Filtered && !r.Row.Matched { + subject = paint(subject, graphDimStyle, opts.Color) + } + parts = append(parts, subject) + + meta := c.Author + if !c.Date.IsZero() { + if meta != "" { + meta += " " + } + meta += RelativeAge(c.Date, now) + } + if meta != "" { + parts = append(parts, paint(meta, graphMetaStyle, opts.Color)) + } + return strings.Join(parts, " ") +} + +func renderGraphLegend(g graphGlyphs, opts GraphOptions) string { + return paint(g.commit, graphMatchedStyle, opts.Color) + " matches the filter " + + paint(g.context, graphContextStyle, opts.Color) + " context" +} + +func shortOrHash(c git.CommitMeta) string { + if c.Short != "" { + return c.Short + } + if len(c.Hash) >= 7 { + return c.Hash[:7] + } + return c.Hash +} + +// BranchTopology writes the `--branches` view: one row per branch with its +// ahead/behind position relative to the base. +func BranchTopology(w io.Writer, branches []git.Branch, opts GraphOptions) error { + if len(branches) == 0 { + return nil + } + g := asciiGlyphs + if opts.Unicode { + g = unicodeGlyphs + } + now := opts.Now + if now.IsZero() { + now = time.Now() + } + + // Align the counts column against the widest rendered name, connector + // included, so the numbers form a readable column. + nameWidth := 0 + for i, b := range branches { + w := lipgloss.Width(branchLabel(b, g, i == len(branches)-1)) + if w > nameWidth { + nameWidth = w + } + } + + var sb strings.Builder + for i, b := range branches { + label := branchLabel(b, g, i == len(branches)-1) + pad := strings.Repeat(" ", nameWidth-lipgloss.Width(label)) + + var counts string + if b.IsBase { + counts = paint("base", graphMatchedStyle, opts.Color) + } else { + counts = paint(fmt.Sprintf("%s%-4d", aheadMark(opts.Unicode), b.Ahead), graphMatchedStyle, opts.Color) + + paint(fmt.Sprintf("%s%-4d", behindMark(opts.Unicode), b.Behind), graphContextStyle, opts.Color) + } + + meta := b.Author + if !b.Date.IsZero() { + if meta != "" { + meta += " " + } + meta += RelativeAge(b.Date, now) + } + + line := label + pad + " " + counts + " " + paint(meta, graphMetaStyle, opts.Color) + sb.WriteString(ui.Clip(strings.TrimRight(line, " "), opts.Width)) + sb.WriteByte('\n') + } + if _, err := io.WriteString(w, sb.String()); err != nil { + return fmt.Errorf("render: write branch topology: %w", err) + } + return nil +} + +// branchLabel prefixes non-base branches with a tree connector so the base +// visually parents them. +func branchLabel(b git.Branch, g graphGlyphs, last bool) string { + if b.IsBase { + return b.Name + } + connector := g.branchTee + if last { + connector = g.branchEnd + } + return connector + " " + b.Name +} + +func aheadMark(unicode bool) string { + if unicode { + return "▲" + } + return "+" +} + +func behindMark(unicode bool) string { + if unicode { + return "▼" + } + return "-" +} + +// paint applies a style only when colour is enabled, so the same code path +// produces clean text for a pipe, a file, or --color=never. +func paint(s string, style lipgloss.Style, color bool) string { + if !color || s == "" { + return s + } + return style.Render(s) +} + +// RelativeAge renders a coarse "how long ago" label. Deliberately low +// resolution — the graph wants a scannable column, not a precise duration, and +// a fixed-width-ish token keeps rows aligned. +func RelativeAge(t, now time.Time) string { + d := now.Sub(t) + switch { + case d < 0: + // A commit dated in the future (skewed clock, rewritten history). + // Reporting "now" is honest enough and avoids a negative duration. + return "now" + case d < time.Minute: + return "now" + case d < time.Hour: + return fmt.Sprintf("%dm", int(d.Minutes())) + case d < 24*time.Hour: + return fmt.Sprintf("%dh", int(d.Hours())) + case d < 7*24*time.Hour: + return fmt.Sprintf("%dd", int(d.Hours()/24)) + case d < 365*24*time.Hour: + return fmt.Sprintf("%dw", int(d.Hours()/(24*7))) + default: + return fmt.Sprintf("%dy", int(d.Hours()/(24*365))) + } +} diff --git a/internal/render/graph_test.go b/internal/render/graph_test.go new file mode 100644 index 0000000..cb4292f --- /dev/null +++ b/internal/render/graph_test.go @@ -0,0 +1,214 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package render + +import ( + "bytes" + "strings" + "testing" + "time" + + "github.com/CommitBrief/commitbrief/internal/git" + "github.com/CommitBrief/commitbrief/internal/graph" +) + +func graphFixture(matched map[string]bool) []GraphRow { + commits := []git.CommitMeta{ + {Hash: "aaa", Short: "aaa1111", Subject: "feat: one", Author: "Alice"}, + {Hash: "bbb", Short: "bbb2222", Subject: "fix: two", Author: "Bob"}, + } + nodes := []graph.Commit{ + {Hash: "aaa", Parents: []string{"bbb"}}, + {Hash: "bbb"}, + } + laid := graph.Layout(nodes, matched) + rows := make([]GraphRow, len(laid)) + for i := range laid { + rows[i] = GraphRow{Row: laid[i], Commit: commits[i]} + } + return rows +} + +func TestGraphEmptyWritesNothing(t *testing.T) { + var w bytes.Buffer + if err := Graph(&w, nil, GraphOptions{}); err != nil { + t.Fatal(err) + } + if w.Len() != 0 { + t.Fatalf("empty graph should write nothing, got %q", w.String()) + } +} + +func TestGraphASCIIByDefault(t *testing.T) { + // The zero GraphOptions is the pipe/test-buffer case: no ANSI, no + // box-drawing. A consumer redirecting to a file must get plain text. + var w bytes.Buffer + if err := Graph(&w, graphFixture(nil), GraphOptions{}); err != nil { + t.Fatal(err) + } + out := w.String() + if strings.ContainsAny(out, "●○│╲╱") { + t.Errorf("unicode glyphs leaked into the ASCII fallback:\n%s", out) + } + if strings.Contains(out, "\x1b[") { + t.Errorf("ANSI escapes leaked with Color=false:\n%s", out) + } + if !strings.Contains(out, "* aaa1111") { + t.Errorf("expected an ASCII commit marker and short hash:\n%s", out) + } +} + +func TestGraphUnicodeGlyphs(t *testing.T) { + var w bytes.Buffer + if err := Graph(&w, graphFixture(nil), GraphOptions{Unicode: true}); err != nil { + t.Fatal(err) + } + if !strings.Contains(w.String(), "●") { + t.Errorf("expected the unicode commit glyph:\n%s", w.String()) + } +} + +func TestGraphMarksMatchedAndContextDifferently(t *testing.T) { + var w bytes.Buffer + opts := GraphOptions{Unicode: true, Filtered: true} + if err := Graph(&w, graphFixture(map[string]bool{"aaa": true}), opts); err != nil { + t.Fatal(err) + } + out := w.String() + lines := strings.Split(strings.TrimSpace(out), "\n") + if !strings.HasPrefix(lines[0], "●") { + t.Errorf("matched commit should use the filled glyph; got %q", lines[0]) + } + if !strings.HasPrefix(lines[1], "○") { + t.Errorf("context commit should use the hollow glyph; got %q", lines[1]) + } + // The distinction is meaningless without a key, so a filtered render + // always explains itself. + if !strings.Contains(out, "matches the filter") { + t.Errorf("a filtered graph must print its legend:\n%s", out) + } +} + +func TestGraphOmitsLegendWhenUnfiltered(t *testing.T) { + var w bytes.Buffer + if err := Graph(&w, graphFixture(nil), GraphOptions{}); err != nil { + t.Fatal(err) + } + if strings.Contains(w.String(), "matches the filter") { + t.Errorf("an unfiltered graph has nothing to explain:\n%s", w.String()) + } +} + +func TestGraphClipsToWidth(t *testing.T) { + // A wrapped row would have no lane gutter on its continuation line, which + // visually detaches it from the graph — so rows clip instead. + var w bytes.Buffer + if err := Graph(&w, graphFixture(nil), GraphOptions{Width: 20}); err != nil { + t.Fatal(err) + } + for _, line := range strings.Split(strings.TrimSpace(w.String()), "\n") { + if len([]rune(line)) > 20 { + t.Errorf("line exceeds the clip width: %q (%d runes)", line, len([]rune(line))) + } + } +} + +func TestGraphZeroWidthMeansNoClipping(t *testing.T) { + // 0 is TerminalWidth's "unknown" signal and must not be read as "clip + // everything away". + var w bytes.Buffer + if err := Graph(&w, graphFixture(nil), GraphOptions{Width: 0}); err != nil { + t.Fatal(err) + } + if !strings.Contains(w.String(), "feat: one") { + t.Errorf("width 0 must leave the row intact:\n%s", w.String()) + } +} + +func TestGraphHandlesMissingSubject(t *testing.T) { + rows := graphFixture(nil) + rows[0].Commit.Subject = "" + var w bytes.Buffer + if err := Graph(&w, rows, GraphOptions{}); err != nil { + t.Fatal(err) + } + if !strings.Contains(w.String(), "(no subject)") { + t.Errorf("an empty subject should render a placeholder, not a blank column:\n%s", w.String()) + } +} + +func TestGraphRendersRefLabels(t *testing.T) { + rows := graphFixture(nil) + rows[0].Refs = []string{"main", "v1.2.0"} + var w bytes.Buffer + if err := Graph(&w, rows, GraphOptions{}); err != nil { + t.Fatal(err) + } + if !strings.Contains(w.String(), "(main, v1.2.0)") { + t.Errorf("expected ref labels:\n%s", w.String()) + } +} + +func TestBranchTopologyRendersCounts(t *testing.T) { + now := time.Date(2026, 7, 26, 12, 0, 0, 0, time.UTC) + branches := []git.Branch{ + {Name: "main", IsBase: true, Author: "Alice", Date: now.Add(-2 * time.Hour)}, + {Name: "feature/x", Ahead: 3, Behind: 12, Author: "Bob", Date: now.Add(-72 * time.Hour)}, + } + var w bytes.Buffer + if err := BranchTopology(&w, branches, GraphOptions{Now: now}); err != nil { + t.Fatal(err) + } + out := w.String() + if !strings.Contains(out, "base") { + t.Errorf("the base branch should be labelled as such:\n%s", out) + } + if !strings.Contains(out, "+3") || !strings.Contains(out, "-12") { + t.Errorf("expected ahead/behind counts:\n%s", out) + } + if !strings.Contains(out, "2h") || !strings.Contains(out, "3d") { + t.Errorf("expected relative ages:\n%s", out) + } +} + +func TestBranchTopologyEmptyWritesNothing(t *testing.T) { + var w bytes.Buffer + if err := BranchTopology(&w, nil, GraphOptions{}); err != nil { + t.Fatal(err) + } + if w.Len() != 0 { + t.Fatalf("no branches should write nothing, got %q", w.String()) + } +} + +func TestRelativeAge(t *testing.T) { + now := time.Date(2026, 7, 26, 12, 0, 0, 0, time.UTC) + cases := []struct { + name string + ago time.Duration + want string + }{ + {"seconds", 30 * time.Second, "now"}, + {"minutes", 42 * time.Minute, "42m"}, + {"hours", 5 * time.Hour, "5h"}, + {"days", 3 * 24 * time.Hour, "3d"}, + {"weeks", 3 * 7 * 24 * time.Hour, "3w"}, + {"years", 2 * 365 * 24 * time.Hour, "2y"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := RelativeAge(now.Add(-tc.ago), now); got != tc.want { + t.Fatalf("RelativeAge(-%v) = %q, want %q", tc.ago, got, tc.want) + } + }) + } +} + +func TestRelativeAgeFutureDateDoesNotGoNegative(t *testing.T) { + // A skewed clock or rewritten history can date a commit in the future; + // "-3h" would read as nonsense. + now := time.Date(2026, 7, 26, 12, 0, 0, 0, time.UTC) + if got := RelativeAge(now.Add(3*time.Hour), now); got != "now" { + t.Fatalf("future date rendered as %q, want \"now\"", got) + } +} diff --git a/internal/ui/progress.go b/internal/ui/progress.go index b5ba542..204e4e8 100644 --- a/internal/ui/progress.go +++ b/internal/ui/progress.go @@ -5,13 +5,10 @@ package ui import ( "fmt" "io" - "os" "strings" "sync" "sync/atomic" "time" - - "golang.org/x/term" ) // Progress is the staged-spinner driving the review pipeline's @@ -94,13 +91,9 @@ func NewProgress(w io.Writer, mode ColorMode, quiet bool) *Progress { p.stop = make(chan struct{}) p.done = make(chan struct{}) // Capture terminal width so redraw can clip lines and never wrap. - // Animated mode implies a TTY writer, so GetSize normally succeeds; + // Animated mode implies a TTY writer, so the query normally succeeds; // a failure leaves width 0 (clipping disabled — best-effort). - if f, ok := w.(*os.File); ok { - if cols, _, err := term.GetSize(int(f.Fd())); err == nil { - p.width = cols - } - } + p.width = TerminalWidth(w) default: p.mode = progressPlain } @@ -515,24 +508,9 @@ const ( stageInfoLeader = " " // bare space (no glyph for info lines) ) -// clip truncates s to at most max display columns (rune count, a good -// enough proxy here), appending "…" when it cuts. max <= 0 means "no -// limit" (terminal width unknown). Keeping rendered lines within the -// terminal width is what prevents wrapping, which would otherwise desync -// the cursor-up redraw and flood the screen. -func clip(s string, max int) string { - if max <= 0 { - return s - } - r := []rune(s) - if len(r) <= max { - return s - } - if max == 1 { - return "…" - } - return string(r[:max-1]) + "…" -} +// clip is the package-internal spelling of Clip (see width.go), kept so the +// redraw hot path reads the same as it always did. +func clip(s string, max int) string { return Clip(s, max) } func animatedDotColor(frame int) string { return breathingColors[frame%len(breathingColors)] + "⏺\033[0m" diff --git a/internal/ui/width.go b/internal/ui/width.go new file mode 100644 index 0000000..c28cb91 --- /dev/null +++ b/internal/ui/width.go @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package ui + +import ( + "io" + "os" + + "golang.org/x/term" +) + +// TerminalWidth reports the writer's terminal width in columns, or 0 when it +// cannot be determined — a non-file writer (a test buffer, a pipe), a +// redirected stream, or a platform that refuses the query. +// +// **0 means "unknown", not "zero columns".** Callers must treat it as "do not +// clip" rather than "clip everything away"; both the progress tree and the +// commit-graph renderer rely on that reading. +// +// The width is a snapshot: nothing here watches SIGWINCH, so a caller that +// holds the value across a resize will be working from a stale number. That is +// deliberate — every consumer renders in one pass, and re-querying per line +// would cost a syscall per row for no benefit. +func TerminalWidth(w io.Writer) int { + f, ok := w.(*os.File) + if !ok { + return 0 + } + cols, _, err := term.GetSize(int(f.Fd())) + if err != nil { + return 0 + } + return cols +} + +// Clip truncates s to at most max display columns (rune count, a good enough +// proxy here), appending "…" when it cuts. max <= 0 means "no limit", matching +// TerminalWidth's "0 = unknown" convention. +// +// Keeping a rendered line inside the terminal width is what prevents wrapping. +// For the progress tree a wrapped line desyncs the cursor-up redraw and floods +// the screen; for the commit graph it breaks the lane columns, since the +// continuation row carries no graph gutter. +func Clip(s string, max int) string { + if max <= 0 { + return s + } + r := []rune(s) + if len(r) <= max { + return s + } + if max == 1 { + return "…" + } + return string(r[:max-1]) + "…" +} diff --git a/man/commitbrief-leaks.1 b/man/commitbrief-leaks.1 new file mode 100644 index 0000000..7c24ab7 --- /dev/null +++ b/man/commitbrief-leaks.1 @@ -0,0 +1,199 @@ +.nh +.TH "COMMITBRIEF" "1" "Jul 2026" "Auto generated by spf13/cobra" "" + +.SH NAME +commitbrief-leaks - Scan the working tree and git history for committed credentials + + +.SH SYNOPSIS +\fBcommitbrief leaks [\&...] [flags]\fP + + +.SH DESCRIPTION +Report credential-shaped content in the repository: every tracked file in the working tree, plus the lines added by historical commits. + +.PP +This is the audit counterpart to the pre-send secret scanner, which only ever sees the one diff about to be reviewed. A secret that was committed and later removed is still in the history — and still reachable by anyone who clones the repo — so the history half is what finds it. + +.PP +Both halves run by default; --no-worktree and --no-history switch either off. The commit filters (--author, --start-date, --end-date, --text) narrow which commits the history half reads, and the path filters narrow which files either half reads. + +.PP +Deterministic: the same regex set the review path uses, no provider call, no cost. Findings report a file, a line and the pattern names that matched — never the matched text, so the report itself cannot leak. + +.PP +Exits 1 when anything is found, so it gates CI out of the box; pass --fail-on none to report without failing. + + +.SH OPTIONS +\fB-h\fP, \fB--help\fP[=false] + help for leaks + +.PP +\fB--no-history\fP[=false] + skip the commit-history half + +.PP +\fB--no-worktree\fP[=false] + skip the working-tree half + +.PP +\fB--patterns\fP[=false] + list the effective pattern set (built-ins + configured) and exit + + +.SH OPTIONS INHERITED FROM PARENT COMMANDS +\fB--allow-secrets\fP[=false] + bypass the pre-send secret scanner (use with care) + +.PP +\fB--author\fP=[] + review only commits authored by these people (repeatable; matches name or email, case-insensitive). Switches the scope to a commit walk + +.PP +\fB--cli\fP="" + use a locally-installed CLI tool (claude|gemini|codex) as the review backend; shorthand for --provider -cli + +.PP +\fB--color\fP="auto" + color output: auto, always, never + +.PP +\fB--committer\fP=[] + review only commits committed by these people (repeatable; matches name or email, case-insensitive) + +.PP +\fB--compact\fP[=false] + one-line per finding (dense review output) + +.PP +\fB--copy\fP[=false] + copy findings (severity, path, title, description) to the system clipboard via OSC 52 + native tool + +.PP +\fB-d\fP, \fB--dir\fP=[] + review only files under these directories or matching dir globs (e.g. \fBinternal/**\fR; repeatable, one pattern per flag); combines with the active scope flag + +.PP +\fB--end-date\fP="" + review only commits on or before this date (YYYY-MM-DD, inclusive) + +.PP +\fB--exclude-dir\fP=[] + skip files under these directories or matching dir globs (repeatable, one pattern per flag); applied after --dir so an exclusion wins + +.PP +\fB--exclude-file\fP=[] + skip these files or globs (repeatable, one pattern per flag); same matching rules as --file, applied after it so an exclusion wins + +.PP +\fB--fail-on\fP="" + exit 1 if any finding meets/exceeds severity (critical|high|medium|low|info|any|none) + +.PP +\fB-f\fP, \fB--file\fP=[] + review only these files or globs (e.g. \fB*.go\fR, \fBinternal/**/*.ts\fR; repeatable, one pattern per flag — patterns can't be comma-joined); combines with the active scope flag + +.PP +\fB--json\fP[=false] + emit machine-readable JSON output + +.PP +\fB--lang\fP="" + AI output language (e.g. tr, fr); the CLI interface localizes for en/tr only, output for any recognized language. Resolution: --lang → repo config → user config → English + +.PP +\fB--markdown\fP[=false] + emit plain markdown (no ANSI) + +.PP +\fB--max-commits\fP=0 + cap how many matching commits enter the review (0 = 200); only meaningful with another commit filter + +.PP +\fB--merges\fP[=false] + include merge commits in a commit-filtered review (excluded by default); only meaningful with another commit filter + +.PP +\fB--min-severity\fP="" + hide findings below this severity in the rendered output (critical|high|medium|low|info); --json and --fail-on still see the full set + +.PP +\fB--model\fP="" + override configured model + +.PP +\fB--no-architecture\fP[=false] + skip architecture-aware review (do not read architecture.json into the prompt) for this run (ADR-0030) + +.PP +\fB--no-baseline\fP[=false] + ignore the signal-control baseline for this run (show everything, even baselined findings) + +.PP +\fB--no-cache\fP[=false] + bypass cache (read and write) + +.PP +\fB--no-cost-check\fP[=false] + skip the pre-send cost estimate prompt + +.PP +\fB--no-flaky\fP[=false] + skip the deterministic flaky-test detector (ADR-0022) + +.PP +\fB-o\fP, \fB--output\fP="" + write output to file instead of stdout + +.PP +\fB--provider\fP="" + override configured provider + +.PP +\fB-q\fP, \fB--quiet\fP[=false] + suppress info messages on stderr + +.PP +\fB--sandbox-rerun\fP[=0] + confirm flagged flaky tests by re-running each in isolation N times (ADR-0022); mixed pass+fail = confirmed flaky, all-fail = real failure, all-pass = transient. 0 = off; bare --sandbox-rerun uses N=5. Requires a bound rerun executor + +.PP +\fB--show-prompt\fP[=false] + print the exact system + user prompt that would be sent, then exit (no provider call, no cost) + +.PP +\fB--start-date\fP="" + review only commits on or after this date (YYYY-MM-DD, inclusive) + +.PP +\fB--suggest-commit\fP[=false] + after the review, suggest a Conventional Commit message for the staged diff (requires --staged; prints to stdout; not with --json/--markdown/--output) + +.PP +\fB--text\fP="" + review only commits whose message contains this text, plus commits unique to a branch whose name contains it (case-insensitive) + +.PP +\fB--update-baseline\fP[=false] + rewrite .commitbrief/baseline.json from the current findings (accepts them all) instead of filtering this run; user-private, gitignored (ADR-0027) + +.PP +\fB-v\fP, \fB--verbose\fP[=false] + show token/cost/latency footer + +.PP +\fB--with-context\fP[=false] + let the CLI provider read project files beyond the diff to ground the review (CLI providers only; the host CLI's agent reads your repo — see --help) + +.PP +\fB-y\fP, \fB--yes\fP[=false] + auto-confirm prompts (pre-send guard, init overwrite) + + +.SH SEE ALSO +\fBcommitbrief(1)\fP + + +.SH HISTORY +26-Jul-2026 Auto generated by spf13/cobra diff --git a/man/commitbrief-map.1 b/man/commitbrief-map.1 new file mode 100644 index 0000000..cdbec68 --- /dev/null +++ b/man/commitbrief-map.1 @@ -0,0 +1,192 @@ +.nh +.TH "COMMITBRIEF" "1" "Jul 2026" "Auto generated by spf13/cobra" "" + +.SH NAME +commitbrief-map - Draw the commit graph, highlighting what a filter selected + + +.SH SYNOPSIS +\fBcommitbrief map [\&...] [flags]\fP + + +.SH DESCRIPTION +Render the commit DAG for a range (or HEAD's history) as a lane graph: one row per commit with its branch/tag labels, subject, author and age. + +.PP +The commit filters apply on top, and that is the point: with --author / --start-date / --end-date / --text set, matching commits are highlighted and the rest are drawn as dimmed context, so you can see exactly what a filter selects before spending a review on it. + +.PP +--branches switches to a branch topology summary — where each branch sits relative to the base, and how far ahead/behind. + +.PP +Read-only and deterministic: no provider call, no cache, no cost. + + +.SH OPTIONS +\fB--all\fP[=false] + walk every branch, not just the given range (commit graph only) + +.PP +\fB--branches\fP[=false] + show a branch topology summary instead of the commit graph + +.PP +\fB-h\fP, \fB--help\fP[=false] + help for map + + +.SH OPTIONS INHERITED FROM PARENT COMMANDS +\fB--allow-secrets\fP[=false] + bypass the pre-send secret scanner (use with care) + +.PP +\fB--author\fP=[] + review only commits authored by these people (repeatable; matches name or email, case-insensitive). Switches the scope to a commit walk + +.PP +\fB--cli\fP="" + use a locally-installed CLI tool (claude|gemini|codex) as the review backend; shorthand for --provider -cli + +.PP +\fB--color\fP="auto" + color output: auto, always, never + +.PP +\fB--committer\fP=[] + review only commits committed by these people (repeatable; matches name or email, case-insensitive) + +.PP +\fB--compact\fP[=false] + one-line per finding (dense review output) + +.PP +\fB--copy\fP[=false] + copy findings (severity, path, title, description) to the system clipboard via OSC 52 + native tool + +.PP +\fB-d\fP, \fB--dir\fP=[] + review only files under these directories or matching dir globs (e.g. \fBinternal/**\fR; repeatable, one pattern per flag); combines with the active scope flag + +.PP +\fB--end-date\fP="" + review only commits on or before this date (YYYY-MM-DD, inclusive) + +.PP +\fB--exclude-dir\fP=[] + skip files under these directories or matching dir globs (repeatable, one pattern per flag); applied after --dir so an exclusion wins + +.PP +\fB--exclude-file\fP=[] + skip these files or globs (repeatable, one pattern per flag); same matching rules as --file, applied after it so an exclusion wins + +.PP +\fB--fail-on\fP="" + exit 1 if any finding meets/exceeds severity (critical|high|medium|low|info|any|none) + +.PP +\fB-f\fP, \fB--file\fP=[] + review only these files or globs (e.g. \fB*.go\fR, \fBinternal/**/*.ts\fR; repeatable, one pattern per flag — patterns can't be comma-joined); combines with the active scope flag + +.PP +\fB--json\fP[=false] + emit machine-readable JSON output + +.PP +\fB--lang\fP="" + AI output language (e.g. tr, fr); the CLI interface localizes for en/tr only, output for any recognized language. Resolution: --lang → repo config → user config → English + +.PP +\fB--markdown\fP[=false] + emit plain markdown (no ANSI) + +.PP +\fB--max-commits\fP=0 + cap how many matching commits enter the review (0 = 200); only meaningful with another commit filter + +.PP +\fB--merges\fP[=false] + include merge commits in a commit-filtered review (excluded by default); only meaningful with another commit filter + +.PP +\fB--min-severity\fP="" + hide findings below this severity in the rendered output (critical|high|medium|low|info); --json and --fail-on still see the full set + +.PP +\fB--model\fP="" + override configured model + +.PP +\fB--no-architecture\fP[=false] + skip architecture-aware review (do not read architecture.json into the prompt) for this run (ADR-0030) + +.PP +\fB--no-baseline\fP[=false] + ignore the signal-control baseline for this run (show everything, even baselined findings) + +.PP +\fB--no-cache\fP[=false] + bypass cache (read and write) + +.PP +\fB--no-cost-check\fP[=false] + skip the pre-send cost estimate prompt + +.PP +\fB--no-flaky\fP[=false] + skip the deterministic flaky-test detector (ADR-0022) + +.PP +\fB-o\fP, \fB--output\fP="" + write output to file instead of stdout + +.PP +\fB--provider\fP="" + override configured provider + +.PP +\fB-q\fP, \fB--quiet\fP[=false] + suppress info messages on stderr + +.PP +\fB--sandbox-rerun\fP[=0] + confirm flagged flaky tests by re-running each in isolation N times (ADR-0022); mixed pass+fail = confirmed flaky, all-fail = real failure, all-pass = transient. 0 = off; bare --sandbox-rerun uses N=5. Requires a bound rerun executor + +.PP +\fB--show-prompt\fP[=false] + print the exact system + user prompt that would be sent, then exit (no provider call, no cost) + +.PP +\fB--start-date\fP="" + review only commits on or after this date (YYYY-MM-DD, inclusive) + +.PP +\fB--suggest-commit\fP[=false] + after the review, suggest a Conventional Commit message for the staged diff (requires --staged; prints to stdout; not with --json/--markdown/--output) + +.PP +\fB--text\fP="" + review only commits whose message contains this text, plus commits unique to a branch whose name contains it (case-insensitive) + +.PP +\fB--update-baseline\fP[=false] + rewrite .commitbrief/baseline.json from the current findings (accepts them all) instead of filtering this run; user-private, gitignored (ADR-0027) + +.PP +\fB-v\fP, \fB--verbose\fP[=false] + show token/cost/latency footer + +.PP +\fB--with-context\fP[=false] + let the CLI provider read project files beyond the diff to ground the review (CLI providers only; the host CLI's agent reads your repo — see --help) + +.PP +\fB-y\fP, \fB--yes\fP[=false] + auto-confirm prompts (pre-send guard, init overwrite) + + +.SH SEE ALSO +\fBcommitbrief(1)\fP + + +.SH HISTORY +26-Jul-2026 Auto generated by spf13/cobra diff --git a/man/commitbrief.1 b/man/commitbrief.1 index 3fce791..e7a31c1 100644 --- a/man/commitbrief.1 +++ b/man/commitbrief.1 @@ -179,7 +179,7 @@ Local LLM-powered code review of git diffs .SH SEE ALSO -\fBcommitbrief-cache(1)\fP, \fBcommitbrief-commit(1)\fP, \fBcommitbrief-completion(1)\fP, \fBcommitbrief-compress(1)\fP, \fBcommitbrief-config(1)\fP, \fBcommitbrief-diff(1)\fP, \fBcommitbrief-doctor(1)\fP, \fBcommitbrief-dry-run(1)\fP, \fBcommitbrief-guard(1)\fP, \fBcommitbrief-init(1)\fP, \fBcommitbrief-install-hook(1)\fP, \fBcommitbrief-list(1)\fP, \fBcommitbrief-mcp(1)\fP, \fBcommitbrief-providers(1)\fP, \fBcommitbrief-remote(1)\fP, \fBcommitbrief-setup(1)\fP, \fBcommitbrief-summary(1)\fP, \fBcommitbrief-upgrade(1)\fP +\fBcommitbrief-cache(1)\fP, \fBcommitbrief-commit(1)\fP, \fBcommitbrief-completion(1)\fP, \fBcommitbrief-compress(1)\fP, \fBcommitbrief-config(1)\fP, \fBcommitbrief-diff(1)\fP, \fBcommitbrief-doctor(1)\fP, \fBcommitbrief-dry-run(1)\fP, \fBcommitbrief-guard(1)\fP, \fBcommitbrief-init(1)\fP, \fBcommitbrief-install-hook(1)\fP, \fBcommitbrief-leaks(1)\fP, \fBcommitbrief-list(1)\fP, \fBcommitbrief-map(1)\fP, \fBcommitbrief-mcp(1)\fP, \fBcommitbrief-providers(1)\fP, \fBcommitbrief-remote(1)\fP, \fBcommitbrief-setup(1)\fP, \fBcommitbrief-summary(1)\fP, \fBcommitbrief-upgrade(1)\fP .SH HISTORY