Merge pull request #13455 from esengine/feat/shellsafe-git-readonly

feat(shellsafe): classify read-only git listings as reads

internal/safety/evidence/git_mutation_test.go

@@ -0,0 +1,21 @@
+package evidence
+
+import "testing"
+
+func TestGitListingsLeaveNoUnprovenMutation(t *testing.T) {
+	for command, want := range map[string]string{
+		"git branch --list":    MutationNone,
+		"git remote -v":        MutationNone,
+		"git config --get a.b": MutationNone,
+		"git stash list":       MutationNone,
+		"git merge-base a b":   MutationNone,
+		"git branch -d old":    MutationUnknown,
+		"git config a.b c":     MutationUnknown,
+		"git stash pop":        MutationUnknown,
+		"git remote add o u":   MutationUnknown,
+	} {
+		if got := bashMutationClass(command); got != want {
+			t.Errorf("bashMutationClass(%q) = %q, want %q", command, got, want)
+		}
+	}
+}

internal/safety/permission/bash_readonly_test.go

@@ -50,7 +50,7 @@ func TestIsReadOnlyBashSubject(t *testing.T) {
 		{"git log >NUL", true},
 		{"git log 2>&1", true},
 		{"git log &>/dev/null", true},
-		{"git remote", false},
+		{"git remote", true},
 		{"git remote add origin [email protected]:x/y", false},
 		{"git config --global user.name Xinwei", false},
 		{"git stash", false},
@@ -78,7 +78,7 @@ func TestIsReadOnlyBashSubject(t *testing.T) {
 		{"rm -rf /", false},
 		{"env rm -rf /", false},
 		{"git commit -m 'msg'", false},
-		{"git branch", false},
+		{"git branch", true},
 		{"git branch feature/new", false},
 		{"git push", false},
 		{"git push --force", false},

internal/safety/permission/git_readonly_test.go

@@ -0,0 +1,26 @@
+package permission
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+func TestGitListingsAreReadOnlyAndTheirWriteFormsAreNot(t *testing.T) {
+	for command, want := range map[string]bool{
+		"git merge-base main HEAD":      true,
+		"git branch --list":             true,
+		"git remote -v":                 true,
+		"git config --get user.name":    true,
+		"git stash list":                true,
+		"git branch new":                false,
+		"git remote add o u":            false,
+		"git config user.name x":        false,
+		"git stash pop":                 false,
+		"git branch -a && git branch x": false,
+	} {
+		args, _ := json.Marshal(map[string]any{"command": command})
+		if got := BashCommandIsReadOnly(args); got != want {
+			t.Errorf("BashCommandIsReadOnly(%q) = %v, want %v", command, got, want)
+		}
+	}
+}

internal/safety/shellsafe/git_readonly.go

@@ -0,0 +1,154 @@
+package shellsafe
+
+import "strings"
+
+// gitBranchFlags are the options of `git branch` that only choose what a
+// listing shows; the valued ones take a following word. Any other option, and
+// every positional word that is not a listing pattern, is outside the list:
+// that is how `git branch name` or `-d` would read as a listing.
+var gitBranchFlags = map[string]bool{
+	"-l": false, "--list": false, "-a": false, "--all": false, "-r": false, "--remotes": false,
+	"-v": false, "-vv": false, "--verbose": false, "--show-current": false, "-i": false, "--ignore-case": false,
+	"--no-color": false, "--no-column": false, "--no-abbrev": false,
+	"--contains": true, "--no-contains": true, "--merged": true, "--no-merged": true, "--points-at": true,
+}
+
+// gitBranchValued are the `--name=value` forms accepted for the same listing.
+var gitBranchValued = []string{"--sort=", "--format=", "--color=", "--column=", "--abbrev=", "--contains=", "--no-contains=", "--merged=", "--no-merged=", "--points-at="}
+
+var gitConfigReads = map[string]bool{"--get": true, "--get-all": true}
+
+// gitSecretKeys are substrings (compared case-insensitively) of configuration
+// keys that hold credentials or lead to them; reading one is never classified.
+var gitSecretKeys = []string{"credential", "extraheader", "token", "password", "insteadof", "signingkey", "secret"}
+
+// gitConfigFlags may accompany a read of the configuration; the valued ones
+// take a following word. Naming a file or blob is not among them.
+var gitConfigFlags = map[string]bool{
+	"--local": false, "--global": false, "--system": false, "--worktree": false, "--show-origin": false,
+	"--show-scope": false, "-z": false, "--null": false, "--includes": false, "--no-includes": false,
+	"--bool": false, "--int": false, "--path": false, "--bool-or-int": false, "--expiry-date": false,
+	"--default": true, "--type": true,
+}
+
+// gitReadOnlyCall reports whether fields are a `git` call that reads although
+// its subcommand also has write forms (branch, remote, config, stash), and the
+// subcommand that serves as its classification. `merge-base` has no write form
+// and sits in ReadOnlyPrefixes instead.
+func gitReadOnlyCall(fields []string) (sub string, ok bool) {
+	if len(fields) < 2 || strings.ToLower(fields[0]) != "git" || hasResolvedSubstitution(fields) {
+		return "", false
+	}
+	sub = strings.ToLower(fields[1])
+	args := fields[2:]
+	switch sub {
+	case "branch":
+		ok = gitBranchLists(args)
+	case "remote":
+		ok = gitRemoteReads(args)
+	case "config":
+		ok = gitConfigReadsOnly(args)
+	case "stash":
+		ok = len(args) > 0 && (args[0] == "list" || args[0] == "show") && !hasLongOpt(args, "--output", 5)
+	}
+	return sub, ok
+}
+
+func gitBranchLists(args []string) bool {
+	listing, positional := false, 0
+	for i := 0; i < len(args); i++ {
+		arg := args[i]
+		if arg == "--" {
+			return false
+		}
+		if !strings.HasPrefix(arg, "-") {
+			positional++
+			continue
+		}
+		if valued, listed := gitBranchFlags[arg]; listed {
+			if arg == "-l" || arg == "--list" {
+				listing = true
+			}
+			if valued && i+1 < len(args) && !strings.HasPrefix(args[i+1], "-") {
+				i++
+			}
+			continue
+		}
+		matched := false
+		for _, prefix := range gitBranchValued {
+			if strings.HasPrefix(arg, prefix) {
+				matched = true
+				break
+			}
+		}
+		if !matched {
+			return false
+		}
+	}
+	return positional == 0 || listing
+}
+
+func gitRemoteReads(args []string) bool {
+	if len(args) == 0 {
+		return true
+	}
+	switch args[0] {
+	case "-v", "--verbose":
+		return len(args) == 1
+	case "show":
+		return len(args) > 2 && args[1] == "-n" && gitOperandsOnly(args[2:])
+	case "get-url":
+		return gitOperandsOnly(args[1:], "--all", "--push")
+	}
+	return false
+}
+
+// gitOperandsOnly accepts the listed flags and plain words, nothing else.
+func gitOperandsOnly(args []string, flags ...string) bool {
+	for _, arg := range args {
+		if strings.HasPrefix(arg, "-") && !hasAnyArg([]string{arg}, flags...) {
+			return false
+		}
+	}
+	return true
+}
+
+// gitConfigReadsOnly accepts a read of exactly one key. A listing, a regexp or a
+// url match would put the whole merged configuration, including the user-level
+// files outside the workspace, in front of the model.
+func gitConfigReadsOnly(args []string) bool {
+	if len(args) > 0 && args[0] == "get" {
+		return gitConfigFlagsOnly(args[1:], false)
+	}
+	return gitConfigFlagsOnly(args, true)
+}
+
+func gitConfigFlagsOnly(args []string, needsRead bool) bool {
+	read, keys := !needsRead, 0
+	for i := 0; i < len(args); i++ {
+		arg := args[i]
+		if gitConfigReads[arg] {
+			read = true
+			continue
+		}
+		if !strings.HasPrefix(arg, "-") {
+			keys++
+			lower := strings.ToLower(arg)
+			for _, secret := range gitSecretKeys {
+				if strings.Contains(lower, secret) {
+					return false
+				}
+			}
+			continue
+		}
+		name, _, attached := strings.Cut(arg, "=")
+		valued, listed := gitConfigFlags[name]
+		if !listed || (!valued && attached) {
+			return false
+		}
+		if valued && !attached {
+			i++
+		}
+	}
+	return read && keys == 1
+}

internal/safety/shellsafe/git_readonly_test.go

@@ -0,0 +1,46 @@
+package shellsafe
+
+import "testing"
+
+func TestGitReadOnlyCallsThatAlsoHaveWriteForms(t *testing.T) {
+	reads := []string{
+		"git merge-base main HEAD", "git merge-base --is-ancestor a b", "git merge-base --fork-point main",
+		"git branch", "git branch --list", "git branch -a", "git branch -r", "git branch -vv", "git branch --show-current",
+		"git branch --list 'feat/*'", "git branch -a --contains abc123", "git branch --merged main", "git branch --sort=-committerdate",
+		"git branch --format='%(refname:short)'", "git branch --no-merged", "git branch -r --list 'origin/*'",
+		"git remote", "git remote -v", "git remote show -n origin", "git remote get-url origin", "git remote get-url --all origin",
+		"git config --get user.name", "git config --get-all remote.origin.fetch", "git config --show-origin --get user.email", "git config --local --get core.bare", "git config get user.name", "git config --global --get user.name", "git config --system --get core.autocrlf",
+		"git config --bool --get core.bare", "git config --default x --get a.b",
+		"git stash list", "git stash show", "git stash show -p stash@{1}", "git stash list --oneline",
+		"git -C /tmp/repo branch -a",
+	}
+	for _, command := range reads {
+		if _, _, _, ok := ClassifyReadOnlyCommand(command); !ok {
+			t.Errorf("%q must be a read", command)
+		}
+	}
+	escapes := []string{
+		"git branch newname", "git branch newname abc123", "git branch -d old", "git branch -D old", "git branch -m old new", "git branch -M new",
+		"git branch -c a b", "git branch -f x HEAD", "git branch --set-upstream-to=origin/x", "git branch -u origin/x", "git branch --unset-upstream",
+		"git branch --edit-description", "git branch --delete old", "git branch --move a b", "git branch --track x origin/x", "git branch -av", "git branch --unknown",
+		"git branch -- name",
+		"git remote show origin", "git remote show", "git remote show -n", "git remote show -n origin extra --evil",
+		"git config --list", "git config -l", "git config --global --list", "git config --system -l", "git config list", "git config --get-regexp '^remote'", "git config --get-urlmatch http.extraheader https://x",
+		"git config --get user.name user.email", "git config --get", "git config get", "git config --name-only --get a.b",
+		"git config --get credential.helper", "git config --get remote.origin.url.insteadOf", "git config --get http.https://x/.extraheader", "git config --get user.signingkey",
+		"git config --get GitHub.Token", "git config get credential.https://x.password", "git config --global --get url.https://tok@x/.insteadOf", "git config --get my.secret",
+		"git remote add o url", "git remote remove o", "git remote rm o", "git remote rename a b", "git remote set-url o u", "git remote set-head o -a",
+		"git remote prune o", "git remote update", "git remote set-branches o x", "git remote -v add", "git remote show --evil o",
+		"git config user.name Bob", "git config --global user.name Bob", "git config --unset a.b", "git config --add a.b c", "git config -e", "git config --edit",
+		"git config --replace-all a.b c", "git config --rename-section a b", "git config --remove-section a", "git config set a b", "git config unset a",
+		"git config --file /etc/passwd --list", "git config -f x --get a", "git config --blob HEAD:x --list", "git config", "git config --get --unknown a",
+		"git stash", "git stash push", "git stash pop", "git stash apply", "git stash drop", "git stash clear", "git stash save x", "git stash branch b",
+		"git stash create", "git stash store abc", "git stash list --output=/tmp/x", "git stash show --output=/tmp/x",
+		"git -c core.pager=sh branch", "git --exec-path=/tmp branch -a",
+	}
+	for _, command := range escapes {
+		if base, sub, _, ok := ClassifyReadOnlyCommand(command); ok {
+			t.Errorf("%q must not be classified as a read (got %s %s)", command, base, sub)
+		}
+	}
+}

internal/safety/shellsafe/shellsafe.go

@@ -62,7 +62,7 @@ var ReadOnlyPrefixes = map[string]map[string]bool{
 		"blame": true, "grep": true, "ls-files": true, "ls-tree": true,
 		"rev-parse": true, "rev-list": true, "describe": true, "reflog": true,
 		"shortlog": true, "whatchanged": true, "cherry": true,
-		"cat-file": true, "for-each-ref": true, "name-rev": true,
+		"cat-file": true, "for-each-ref": true, "name-rev": true, "merge-base": true,
 	},
 	"go": {
 		"vet": true, "doc": true, "list": true,
@@ -145,6 +145,9 @@ func ClassifyReadOnlyFields(fields []string) (base, sub string, out []string, ok
 	if sub, ok := ghReadOnlyCall(fields); ok {
 		return base, sub, fields, true
 	}
+	if sub, ok := gitReadOnlyCall(fields); ok {
+		return base, sub, fields, true
+	}
 	return "", "", nil, false
 }
 
@@ -278,6 +281,9 @@ func readOnlyFields(fields []string) (base, sub string, ok bool) {
 	if sub, ok := ghReadOnlyCall(fields); ok {
 		return base, sub, true
 	}
+	if sub, ok := gitReadOnlyCall(fields); ok {
+		return base, sub, true
+	}
 	return "", "", false
 }
 

release-notes/studio/unreleased/13293g.md

@@ -0,0 +1,2 @@
+- 更多只读 git 命令（merge-base、分支/远程/配置/暂存的查看形式）不再被当成“无法证明只读的改动”，也不再触发验证提示。关联 #13293。
+- More read-only git commands (merge-base and the listing forms of branch, remote, config and stash) no longer count as an unproven change or raise the verification prompt. Refs #13293.