Check the staged diff for credentials before push commits

`push` runs `git add --all .`, so anything lying in the project gets
committed, and with `mirror = true` it reaches a public server in the
same breath. It is the one action in mgsh that cannot be undone: a
deleted server repository comes back from an archive, a published
credential does not.

The staged diff is now scanned before the commit is made -- private keys,
GitHub/GitLab/Slack/AWS/PyPI tokens, and credential-shaped assignments --
and a hit is shown with file and line before asking whether to continue.
Declining leaves the changes staged but uncommitted, so removing the file
and adding a .gitignore entry is all it takes.

The hard part is not detection but silence. A scanner that cries wolf
gets answered with a reflexive "y" and stops being a safety net, so
values that are plainly environment references, dotted identifiers,
constant names, template slots or masked stand-ins are filtered out. A
test scans mgsh's own README and mgshrc.example -- both full of
credential-shaped text -- and fails if either would trip the check. It
caught the documentation for this very feature, which is why the README
describes the sample output instead of reproducing it.

For a line that legitimately looks like a credential there is
`mgsh:allow`, which suppresses that one line; `secretscan = off` turns
the check off entirely. Only an explicit "off" does that -- a typo in the
setting leaves the safety net in place, which is what the new falsy()
is for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

commands.go

@@ -272,6 +272,13 @@ func runCommandDepth(line string, depth int) bool {
 		}
 		comment := strings.Join(fields[1:], " ")
 		git(DIR, "add", "--all", ".")
+		// last look before anything is committed: `add --all` sweeps up whatever
+		// is lying around, and with mirroring on it goes straight to a public
+		// server. Nothing has been committed yet, so declining costs nothing.
+		if !secretsApproved(DIR) {
+			errorln("push cancelled — your changes are staged but not committed")
+			break
+		}
 		msg := strings.TrimSpace(fmt.Sprintf("[%s@%s] %s", USER, HOST, comment))
 		git(DIR, "commit", "-m", msg) // may be "nothing to commit"; continue anyway
 		if !gitOK(DIR, "push") {
@@ -711,7 +718,7 @@ var helpItems = []struct{ cmd, desc string }{
 	{"pull", "pull changes from git server"},
 	{"fetch", "fetch changes from git server"},
 	{"status [-a]", "short git status (-a: overview of all projects)"},
-	{"overview", "status of all projects (dirty, ahead/behind)"},
+	{"overview", "inventory of all projects, local and on the server"},
 	{"diff [args]", "show git diff"},
 	{"edit [number]", "edit last [number] commits (default is 10)"},
 	{"clone [-a] <repository>", "clone repository from git server (-a for archive)"},

config.go

@@ -38,6 +38,7 @@ type Config struct {
 	RemoteType  string // "gitea"|"github"|"gitlab" (auto-detected when empty)
 	RemoteVis   string // visibility of created repos: "private" (default)|"public"
 	Mirror      string // truthy -> `push` also mirrors via `pushremote`
+	SecretScan  string // falsy -> `push` skips the credential scan
 	Remotes     []RemoteTarget
 	RemoteNames string // "remotes": explicit, ordered subset of targets to use
 }
@@ -370,6 +371,7 @@ func applyConfig(c *Config, m map[string]string) {
 	set("remotevisibility", &c.RemoteVis)
 	set("remotes", &c.RemoteNames)
 	set("mirror", &c.Mirror)
+	set("secretscan", &c.SecretScan)
 	applyRemoteTargets(c, m)
 }
 
@@ -443,4 +445,5 @@ func applyEnv(c *Config) {
 	env("MGSH_REMOTEVISIBILITY", &c.RemoteVis)
 	env("MGSH_REMOTES", &c.RemoteNames)
 	env("MGSH_MIRROR", &c.Mirror)
+	env("MGSH_SECRETSCAN", &c.SecretScan)
 }

mgshrc.example

@@ -42,6 +42,11 @@ gitpath     = /home/git
 #
 # mirror           = true         # `push` also mirrors via pushremote
 
+# --- safety ---
+# `push` checks the staged diff for private keys and API tokens before it
+# commits, and asks before continuing. Only an explicit "off" disables it.
+# secretscan       = off
+
 # --- per-project overrides ---
 # A <project>/.mgshrc overrides all of the above for that project only, except
 # base, gitname, gitemail and pushdefault, which stay global. Typical use:

secrets.go

@@ -0,0 +1,206 @@
+package main
+
+// secrets.go — a last look at what `push` is about to commit.
+//
+// `push` runs `git add --all .`, so anything lying in the project — an .env, a
+// stray key file, a token pasted into a config — is committed and pushed, and
+// with `mirror = true` it reaches a *public* server in the same breath. That is
+// the one action in mgsh that cannot be undone: a deleted server repository can
+// come back from an archive, a published credential is burnt.
+//
+// So the staged diff is scanned for a small set of high-signal patterns before
+// the commit is made. This is not a complete secret scanner and does not try to
+// be one; it aims for a high hit rate on the things that actually leak, with
+// few enough false alarms that the prompt still means something. Turn it off
+// with `secretscan = off`.
+
+import (
+	"fmt"
+	"regexp"
+	"strconv"
+	"strings"
+)
+
+// secretHit is one suspicious added line.
+type secretHit struct {
+	file   string
+	lineNo int
+	kind   string
+	text   string
+}
+
+// secretPattern matches one kind of credential. `certain` patterns are
+// unmistakable and are reported as they are; the others match a shape that
+// merely looks like a secret and are filtered through looksLikePlaceholder.
+type secretPattern struct {
+	kind    string
+	re      *regexp.Regexp
+	certain bool
+}
+
+var secretPatterns = []secretPattern{
+	{"private key", regexp.MustCompile(`-----BEGIN (?:[A-Z]+ )?PRIVATE KEY-----`), true},
+	{"GitHub token", regexp.MustCompile(`\bgh[pousr]_[A-Za-z0-9]{20,}`), true},
+	{"GitLab token", regexp.MustCompile(`\bglpat-[A-Za-z0-9_-]{16,}`), true},
+	{"AWS access key", regexp.MustCompile(`\b(?:AKIA|ASIA)[0-9A-Z]{16}\b`), true},
+	{"Slack token", regexp.MustCompile(`\bxox[baprs]-[A-Za-z0-9-]{10,}`), true},
+	{"PyPI token", regexp.MustCompile(`\bpypi-AgEIcHlwaS5vcmc[A-Za-z0-9_-]{10,}`), true},
+	{"credential assignment", regexp.MustCompile(
+		`(?i)\b(?:password|passwd|secret|api[_-]?key|apikey|access[_-]?key|auth[_-]?token|token)\b` +
+			`\s*[:=]\s*(?:"([^"\s]{12,})"|'([^'\s]{12,})'|([^\s"';,]{20,}))\s*;?\s*$`), false},
+}
+
+var (
+	diffFileRe = regexp.MustCompile(`^\+\+\+ b/(.*)$`)
+	diffHunkRe = regexp.MustCompile(`^@@ -\d+(?:,\d+)? \+(\d+)`)
+	// a value that is plainly a reference or a stand-in, not a credential
+	constRefRe  = regexp.MustCompile(`^[A-Z][A-Z0-9_]*$`)
+	dottedRefRe = regexp.MustCompile(`^[\w-]+(?:\.[\w-]+)+$`)
+	maskedRe    = regexp.MustCompile(`^[*x•.]+$`)
+)
+
+// placeholderWords are the values people write when they mean "fill this in".
+var placeholderWords = map[string]bool{
+	"changeme": true, "change_me": true, "password": true, "secret": true,
+	"token": true, "your_token": true, "your-token": true, "yourtoken": true,
+	"todo": true, "none": true, "null": true, "example": true, "redacted": true,
+}
+
+// looksLikePlaceholder reports whether a matched value is obviously not a real
+// credential: a template slot, an environment reference, a constant name, or a
+// masked stand-in. Documentation and example files are full of these, and every
+// one of them that reaches the prompt makes the prompt worth less.
+func looksLikePlaceholder(v string) bool {
+	v = strings.Trim(v, `"'`)
+	if v == "" {
+		return true
+	}
+	if strings.ContainsAny(v, "<>${}()") { // <token>, ${VAR}, $(cmd), {{ tpl }}
+		return true
+	}
+	if maskedRe.MatchString(v) || constRefRe.MatchString(v) || dottedRefRe.MatchString(v) {
+		return true
+	}
+	if placeholderWords[strings.ToLower(v)] {
+		return true
+	}
+	// a value made of one repeated character carries no information
+	if strings.Count(v, string(v[0])) == len(v) {
+		return true
+	}
+	return false
+}
+
+// scanDiff finds suspicious added lines in a unified diff. Only added lines are
+// examined: removing a secret is what we want people to do.
+func scanDiff(diff string) []secretHit {
+	var hits []secretHit
+	file := ""
+	lineNo := 0
+
+	for _, ln := range strings.Split(diff, "\n") {
+		switch {
+		case strings.HasPrefix(ln, "+++ "):
+			file = ""
+			if m := diffFileRe.FindStringSubmatch(ln); m != nil {
+				file = m[1]
+			}
+			continue
+		case strings.HasPrefix(ln, "@@"):
+			if m := diffHunkRe.FindStringSubmatch(ln); m != nil {
+				lineNo, _ = strconv.Atoi(m[1])
+			}
+			continue
+		case strings.HasPrefix(ln, "---") || strings.HasPrefix(ln, "diff ") ||
+			strings.HasPrefix(ln, "index ") || strings.HasPrefix(ln, "new file") ||
+			strings.HasPrefix(ln, "deleted file") || strings.HasPrefix(ln, "similarity "):
+			continue
+		case strings.HasPrefix(ln, "-"):
+			continue // removed line: not our problem
+		case !strings.HasPrefix(ln, "+"):
+			lineNo++ // context line
+			continue
+		}
+
+		text := ln[1:]
+		if kind := matchSecret(text); kind != "" {
+			hits = append(hits, secretHit{file: file, lineNo: lineNo, kind: kind, text: text})
+		}
+		lineNo++
+	}
+	return hits
+}
+
+// allowMarker suppresses the check for one line. Any scanner needs a per-line
+// escape: a project will eventually hold something credential-shaped on
+// purpose, and switching the whole check off for that is far too blunt.
+const allowMarker = "mgsh:allow"
+
+// matchSecret returns the kind of credential a line appears to contain, or "".
+func matchSecret(text string) string {
+	if strings.Contains(text, allowMarker) {
+		return ""
+	}
+	for _, p := range secretPatterns {
+		m := p.re.FindStringSubmatch(text)
+		if m == nil {
+			continue
+		}
+		if p.certain {
+			return p.kind
+		}
+		// the first non-empty capture group is the value that was assigned
+		value := ""
+		for _, g := range m[1:] {
+			if g != "" {
+				value = g
+				break
+			}
+		}
+		if !looksLikePlaceholder(value) {
+			return p.kind
+		}
+	}
+	return ""
+}
+
+// secretScanEnabled reports whether the scan runs. It is on unless explicitly
+// switched off, so a typo in the setting leaves the safety net in place.
+func secretScanEnabled() bool { return !falsy(cfg.SecretScan) }
+
+// secretsApproved scans what `push` has staged. With nothing suspicious found
+// it returns true silently; otherwise it shows the findings and asks. Returns
+// false when the push should stop.
+func secretsApproved(dir string) bool {
+	if !secretScanEnabled() {
+		return true
+	}
+	diff, err := gitCapture(dir, "diff", "--cached", "-U0", "--no-color")
+	if err != nil {
+		return true // nothing staged, or no HEAD yet: not our call to block
+	}
+	hits := scanDiff(diff)
+	if len(hits) == 0 {
+		return true
+	}
+
+	fmt.Println(col(cRed, fmt.Sprintf("%d possible credential(s) in what is about to be committed:", len(hits))))
+	for _, h := range hits {
+		where := h.file
+		if h.lineNo > 0 {
+			where += ":" + strconv.Itoa(h.lineNo)
+		}
+		fmt.Printf("  %s  %s\n    %s\n",
+			col(cYellow, where), col(cGray, h.kind), col(cRed, ellipsis(strings.TrimSpace(h.text), 100)))
+	}
+	fmt.Println(col(cGray, "  (set 'secretscan = off' to skip this check)"))
+	return yesno("push anyway?", false)
+}
+
+// ellipsis shortens s to at most n characters.
+func ellipsis(s string, n int) string {
+	if len(s) <= n {
+		return s
+	}
+	return s[:n-1] + "…"
+}

secrets_test.go

@@ -0,0 +1,212 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// TestMatchSecretCatchesRealCredentials: the shapes that actually leak.
+func TestMatchSecretCatchesRealCredentials(t *testing.T) {
+	lines := []string{
+		"-----BEGIN OPENSSH PRIVATE KEY-----",
+		"-----BEGIN RSA PRIVATE KEY-----",
+		"  ghp_aB3dEfGh1jKlMn0pQrStUvWxYz012345678",
+		"GITLAB=glpat-aB3dEfGh1jKlMn0pQrSt",
+		`aws_access_key_id = AKIAIOSFODNN7EXAMPLE`,
+		"slack: xoxb-1234567890-abcdefghij",
+		`API_KEY="s3cr3tV4lu3W1thStuff"`,
+		"password = hunter2hunter2hunter2",
+		"token: 'aB3dEfGh1jKlMn0pQrSt'",
+		"auth-token=9f8e7d6c5b4a39281706abcdef123456",
+	}
+	for _, ln := range lines {
+		if matchSecret(ln) == "" {
+			t.Errorf("missed a credential in %q", ln)
+		}
+	}
+}
+
+// TestMatchSecretIgnoresNoise: everyday code and documentation must not trip
+// the prompt, or people learn to answer "yes" without reading it.
+func TestMatchSecretIgnoresNoise(t *testing.T) {
+	lines := []string{
+		"remotekey        = <personal-access-token>",  // our own README
+		"# remotekey      = <personal-access-token>",  // and mgshrc.example
+		"token = process.env.GITHUB_TOKEN",            // reference, not a value
+		"const token = getToken()",                    // call
+		"password = ${DB_PASSWORD}",                   // template
+		`api_key = "changeme"`,                        // placeholder
+		"secret: TODO",                                //
+		"key gh***************xk",                     // masked, from `config`
+		"password = xxxxxxxxxxxxxxxxxxxxxxx",          // masked
+		"// the token is never persisted in the repo", // prose
+		"apiKey := os.Getenv(\"MGSH_REMOTEKEY\")",     // lookup
+		"token = SOME_CONSTANT_NAME",                  // constant
+		"secret = my.config.value",                    // dotted reference
+		"+++ b/token.go",                              // diff furniture
+		"password = short",                            // too short to be one
+		"Authorization: Basic <base64(owner:token)>",  // documentation
+		"remote.hub.key        = <personal-access-token>",
+	}
+	for _, ln := range lines {
+		if kind := matchSecret(ln); kind != "" {
+			t.Errorf("false positive (%s) on %q", kind, ln)
+		}
+	}
+}
+
+// TestScanDiffReportsFileAndLine: the report has to point at the right place,
+// and must ignore removed lines — deleting a secret is the desired action.
+func TestScanDiffReportsFileAndLine(t *testing.T) {
+	diff := `diff --git a/.env b/.env
+new file mode 100644
+--- /dev/null
++++ b/.env
+@@ -0,0 +1,3 @@
++HOME=/tmp
++API_KEY="s3cr3tV4lu3W1thStuff"
++DEBUG=1
+diff --git a/old.txt b/old.txt
+--- a/old.txt
++++ b/old.txt
+@@ -7,1 +7,0 @@
+-password = hunter2hunter2hunter2
+`
+	hits := scanDiff(diff)
+	if len(hits) != 1 {
+		t.Fatalf("expected exactly one hit, got %d: %+v", len(hits), hits)
+	}
+	h := hits[0]
+	if h.file != ".env" {
+		t.Errorf("file = %q, want .env", h.file)
+	}
+	if h.lineNo != 2 {
+		t.Errorf("lineNo = %d, want 2", h.lineNo)
+	}
+	if !strings.Contains(h.text, "API_KEY") {
+		t.Errorf("text = %q", h.text)
+	}
+}
+
+// TestScanDiffCountsLinesAcrossHunks keeps the line numbers honest when a file
+// is edited in several places.
+func TestScanDiffCountsLinesAcrossHunks(t *testing.T) {
+	diff := `+++ b/config.yml
+@@ -1,0 +1,1 @@
++harmless: yes
+@@ -40,0 +41,2 @@
++also fine
++aws_key = AKIAIOSFODNN7EXAMPLE
+`
+	hits := scanDiff(diff)
+	if len(hits) != 1 || hits[0].lineNo != 42 {
+		t.Fatalf("hits = %+v, want one at line 42", hits)
+	}
+}
+
+// TestSecretsApprovedBlocksThePush drives the real thing: a staged .env, the
+// scan, and the answer deciding whether push continues.
+func TestSecretsApprovedBlocksThePush(t *testing.T) {
+	dir := t.TempDir()
+	mustGit(t, dir, "init", "-q")
+	mustGit(t, dir, "config", "user.name", "t")
+	mustGit(t, dir, "config", "user.email", "t@e")
+	mustGit(t, dir, "commit", "-q", "--allow-empty", "-m", "base")
+
+	old := cfg
+	defer func() { cfg = old }()
+	cfg = Config{}
+
+	// clean tree: no prompt, no interference
+	asked := fakeAnswers(t, false)
+	if !secretsApproved(dir) {
+		t.Fatal("a clean tree must not block the push")
+	}
+	if len(*asked) != 0 {
+		t.Fatalf("asked about a clean tree: %v", *asked)
+	}
+
+	if err := os.WriteFile(filepath.Join(dir, ".env"),
+		[]byte("API_KEY=\"s3cr3tV4lu3W1thStuff\"\n"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	mustGit(t, dir, "add", "--all", ".")
+
+	declined := fakeAnswers(t, false)
+	if secretsApproved(dir) {
+		t.Error("a staged credential must stop the push when declined")
+	}
+	if len(*declined) == 0 {
+		t.Error("the user was never asked")
+	}
+
+	accepted := fakeAnswers(t, true)
+	if !secretsApproved(dir) {
+		t.Error("an explicit yes must let the push through")
+	}
+	if len(*accepted) == 0 {
+		t.Error("the user was never asked")
+	}
+
+	// and the escape hatch really switches it off
+	cfg.SecretScan = "off"
+	never := fakeAnswers(t, false)
+	if !secretsApproved(dir) {
+		t.Error("secretscan = off must not block")
+	}
+	if len(*never) != 0 {
+		t.Errorf("secretscan = off still asked: %v", *never)
+	}
+}
+
+// TestSecretScanDefaultsToOn: only a deliberate "off" disables it, so a typo
+// leaves the safety net in place.
+func TestSecretScanDefaultsToOn(t *testing.T) {
+	old := cfg
+	defer func() { cfg = old }()
+	for _, v := range []string{"", "true", "on", "yes", "wharrgarbl", "1"} {
+		cfg = Config{SecretScan: v}
+		if !secretScanEnabled() {
+			t.Errorf("secretscan = %q disabled the scan", v)
+		}
+	}
+	for _, v := range []string{"off", "0", "false", "no", " OFF "} {
+		cfg = Config{SecretScan: v}
+		if secretScanEnabled() {
+			t.Errorf("secretscan = %q did not disable the scan", v)
+		}
+	}
+}
+
+// TestOwnDocsDoNotTripTheScanner: mgsh's own README and example config are full
+// of credential-shaped text; committing mgsh itself must stay quiet.
+func TestOwnDocsDoNotTripTheScanner(t *testing.T) {
+	for _, f := range []string{"README.md", "mgshrc.example"} {
+		data, err := os.ReadFile(f)
+		if err != nil {
+			t.Fatal(err)
+		}
+		for i, ln := range strings.Split(string(data), "\n") {
+			if kind := matchSecret(ln); kind != "" {
+				t.Errorf("%s:%d would trip the scanner (%s): %q", f, i+1, kind, ln)
+			}
+		}
+	}
+}
+
+// TestAllowMarkerSuppressesOneLine: the per-line escape for something that only
+// looks like a credential and is meant to stay.
+func TestAllowMarkerSuppressesOneLine(t *testing.T) {
+	line := `API_KEY="s3cr3tV4lu3W1thStuff"`
+	if matchSecret(line) == "" {
+		t.Fatal("test line is not detected at all")
+	}
+	if kind := matchSecret(line + "   # mgsh:allow — sample value"); kind != "" {
+		t.Errorf("mgsh:allow did not suppress the hit (%s)", kind)
+	}
+	if kind := matchSecret("-----BEGIN OPENSSH PRIVATE KEY----- mgsh:allow"); kind != "" {
+		t.Errorf("mgsh:allow did not suppress a certain pattern (%s)", kind)
+	}
+}

show_config.go

@@ -44,6 +44,7 @@ func showConfig() {
 		{"pushdefault", cfg.PushDefault},
 		{"editor", cfg.Editor},
 		{"mirror", cfg.Mirror},
+		{"secretscan", cfg.SecretScan},
 		{"remotes", cfg.RemoteNames},
 	}
 
@@ -147,7 +148,7 @@ func configKeys() []string {
 		"base", "githost", "gitport", "gituser", "gitpath", "gitkey",
 		"gitname", "gitemail", "pushdefault", "editor",
 		"remoteurl", "remotekey", "remotetype", "remotevisibility",
-		"remotes", "mirror",
+		"remotes", "mirror", "secretscan",
 	}
 	sort.Strings(keys)
 	return keys

util.go

@@ -39,6 +39,17 @@ func truthy(s string) bool {
 	return false
 }
 
+// falsy reports whether a config string explicitly means "off". It is not the
+// negation of truthy: for a setting that defaults to on, an unset value or a
+// typo must leave it on, and only a deliberate "off" may switch it off.
+func falsy(s string) bool {
+	switch strings.ToLower(strings.TrimSpace(s)) {
+	case "0", "false", "no", "off":
+		return true
+	}
+	return false
+}
+
 func fileExists(p string) bool {
 	fi, err := os.Stat(p)
 	return err == nil && !fi.IsDir()