diff --git a/git/AGENTS.md b/git/AGENTS.md index 53c238c..e8f4e6d 100644 --- a/git/AGENTS.md +++ b/git/AGENTS.md @@ -32,6 +32,17 @@ specific application or forge workflow. settings in the imported worktree. This is a narrow Git execution boundary, not an OS sandbox for hostile configuration, remotes, lifecycle scripts, same-user replacement races, or resource exhaustion. +- Replacement merge drivers for untrusted merge-request imports must run the + resolved Git executable without looking it up through the worktree `PATH`. + They must clear inherited repository bindings and counted configuration, + classify binary inputs without repository attributes or external + diff/textconv helpers, and pin `core.bigFileThreshold=1023m` to match + `merge-file`'s maximum text size. They must write clean text merges and diff3 + markers for text conflicts, and treat classified binary content as an + ordinary per-file conflict. Classifier failures, text-merge I/O failures, a + missing Git executable, or a merge-process crash must fail the whole + operation. This contract requires Git 2.42.0+ on non-Windows and Git for + Windows 2.53.0.windows.3+. Keep both platform behaviors explicit. - Reject isolation-sensitive command-scope configuration during import because worktree configuration cannot outrank it. Explicit command-scope overrides on later Git commands are caller policy, not a sandbox boundary Kit can diff --git a/git/cmd/gitcmd.go b/git/cmd/gitcmd.go index 432d050..7064b63 100644 --- a/git/cmd/gitcmd.go +++ b/git/cmd/gitcmd.go @@ -27,6 +27,7 @@ import ( "time" gitenv "go.kenn.io/kit/git/env" + "go.kenn.io/kit/git/internal/shellquote" ) // Config is one temporary git config entry injected through GIT_CONFIG_* env. @@ -187,15 +188,11 @@ func credentialHelper(path string) string { return `!f() { ` + `while IFS= read -r line && [ -n "$line" ]; do :; done; ` + `if [ "$1" = get ]; then ` + - `while IFS= read -r line; do printf '%s\n' "$line"; done < ` + shellSingleQuote(path) + `; ` + + `while IFS= read -r line; do printf '%s\n' "$line"; done < ` + shellquote.Single(path) + `; ` + `fi; ` + `}; f` } -func shellSingleQuote(value string) string { - return "'" + strings.ReplaceAll(value, "'", "'\\''") + "'" -} - var ( emptyGlobalConfigOnce sync.Once emptyGlobalConfigPath string diff --git a/git/cmd/gitcmd_test.go b/git/cmd/gitcmd_test.go index 98bac7e..a5fda2f 100644 --- a/git/cmd/gitcmd_test.go +++ b/git/cmd/gitcmd_test.go @@ -16,6 +16,7 @@ import ( Require "github.com/stretchr/testify/require" gitenv "go.kenn.io/kit/git/env" + "go.kenn.io/kit/git/internal/shellquote" ) func TestRunnerCommandUsesDefensiveEnvironment(t *testing.T) { @@ -566,7 +567,7 @@ func captureGitEnv(t *testing.T, runner Runner) string { binDir := t.TempDir() envPath := filepath.Join(t.TempDir(), "env") gitPath := filepath.Join(binDir, "git") - script := "#!/bin/sh\nenv > " + shellSingleQuote(envPath) + "\n" + script := "#!/bin/sh\nenv > " + shellquote.Single(envPath) + "\n" if os.PathSeparator == '\\' { gitPath += ".bat" script = "@echo off\r\nset > " + shellDoubleQuote(envPath) + "\r\n" diff --git a/git/internal/shellquote/shellquote.go b/git/internal/shellquote/shellquote.go new file mode 100644 index 0000000..c453bb6 --- /dev/null +++ b/git/internal/shellquote/shellquote.go @@ -0,0 +1,9 @@ +// Package shellquote quotes arguments embedded in Git shell command strings. +package shellquote + +import "strings" + +// Single returns value enclosed in POSIX shell single quotes. +func Single(value string) string { + return "'" + strings.ReplaceAll(value, "'", "'\\''") + "'" +} diff --git a/git/internal/shellquote/shellquote_test.go b/git/internal/shellquote/shellquote_test.go new file mode 100644 index 0000000..2e5a987 --- /dev/null +++ b/git/internal/shellquote/shellquote_test.go @@ -0,0 +1,23 @@ +package shellquote + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSingle(t *testing.T) { + t.Parallel() + for _, test := range []struct { + name, input, want string + }{ + {name: "empty", want: "''"}, + {name: "spaces", input: "/opt/Git Tools/git", want: "'/opt/Git Tools/git'"}, + {name: "single quote", input: "/opt/Git's/git", want: "'/opt/Git'\\''s/git'"}, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, test.want, Single(test.input)) + }) + } +} diff --git a/git/managed/lifecycle.go b/git/managed/lifecycle.go index 0cea5f1..a959908 100644 --- a/git/managed/lifecycle.go +++ b/git/managed/lifecycle.go @@ -56,6 +56,13 @@ type HookError struct { } // GitRunner runs one Git command under an application's process policy. +// +// It governs how Kit's own Git commands are executed, not which Git +// installation Kit targets. Merge-request import pins the git found on the +// process PATH into the replacement merge driver, because Git runs that driver +// itself and cannot route it back through this callback. A runner that +// executes some other Git therefore does not redirect the merge driver, and +// import fails outright when no git is on PATH. type GitRunner func( ctx context.Context, runner gitcmd.Runner, dir string, args ...string, ) ([]byte, error) diff --git a/git/managed/lifecycle_mr.go b/git/managed/lifecycle_mr.go index dc25a41..71ac74d 100644 --- a/git/managed/lifecycle_mr.go +++ b/git/managed/lifecycle_mr.go @@ -110,6 +110,9 @@ type mergeRequestRemoteTarget struct { // submodule recursion. The existing repository, its configuration, provider // metadata, remotes, and explicitly configured setup hook remain trusted. // +// Untrusted-tree isolation requires Git 2.42.0 or newer on non-Windows +// platforms and Git for Windows 2.53.0.windows.3 or newer. +// // The function also configures upstream tracking when possible, non-fatally // skipping it when the fork cannot be fetched. Failures after the worktree // exists roll it back. diff --git a/git/managed/lifecycle_mr_test.go b/git/managed/lifecycle_mr_test.go index c10f2a0..eab9e5c 100644 --- a/git/managed/lifecycle_mr_test.go +++ b/git/managed/lifecycle_mr_test.go @@ -69,6 +69,15 @@ func worktreeOnlyConfig(t *testing.T, dir, key string) string { return strings.TrimSpace(string(out)) } +func expectedSafeMergeDriverCommand(t *testing.T, worktree string) string { + t.Helper() + path, err := resolveMergeDriverGitPath() + Require.NoError(t, err) + hooksPath := worktreeConfig(t, worktree, "core.hooksPath") + Require.NotEmpty(t, hooksPath) + return safeMergeDriverCommand(path, hooksPath) +} + // TestCreateWorktreeFromMergeRequestSameRepo covers the same-repo scenario: // the head branch is fetched from origin, the new local branch starts at // it, and upstream tracking points at origin's head branch. @@ -519,7 +528,7 @@ func TestCreateWorktreeFromMergeRequestIsolatesUntrustedTreeGitPrograms(t *testi worktreeConfig(t, dest, "diff.owned.command")) assert.Equal(safeTextconvCommand, worktreeConfig(t, dest, "diff.owned.textconv")) - assert.Equal(safeMergeDriverCommand, + assert.Equal(expectedSafeMergeDriverCommand(t, dest), worktreeConfig(t, dest, "merge.owned.driver")) if err := os.Remove(fsmonitorMarker); err != nil { @@ -669,7 +678,7 @@ func TestCreateWorktreeFromMergeRequestNeutralizesCaseDistinctAttributeDrivers( worktreeOnlyConfig(t, dest, "filter."+driver+".required")) assert.Equal(safeExternalDiffCommand, worktreeOnlyConfig(t, dest, "diff."+driver+".command")) - assert.Equal(safeMergeDriverCommand, + assert.Equal(expectedSafeMergeDriverCommand(t, dest), worktreeOnlyConfig(t, dest, "merge."+driver+".driver")) } } @@ -684,23 +693,38 @@ func TestCreateWorktreeFromMergeRequestDoesNotPATHSearchDriverHelpers( assert := assert.New(t) origin, clone := initOriginAndClone(t) marker := filepath.Join(t.TempDir(), "attacker-sh-ran") - lifecycleGit(t, origin, "checkout", "-q", "-b", "path-driver") require.NoError(os.WriteFile( filepath.Join(origin, ".gitattributes"), - []byte("payload diff=owned\n"), 0o644, + []byte("payload diff=owned merge=owned\n"), 0o644, )) require.NoError(os.WriteFile( - filepath.Join(origin, "payload"), []byte("external\n"), 0o644, + filepath.Join(origin, "payload"), []byte("base\n"), 0o644, )) + for _, helper := range []string{"git", "sh"} { + require.NoError(os.WriteFile( + filepath.Join(origin, helper), + []byte("#!/bin/sh\n: > \""+marker+"\"\nexit 0\n"), 0o755, + )) + } + lifecycleGit(t, origin, "add", ".gitattributes", "payload", "git", "sh") + lifecycleGit(t, origin, "commit", "-qm", "PATH driver base") + lifecycleGit(t, origin, "checkout", "-q", "-b", "path-driver") require.NoError(os.WriteFile( - filepath.Join(origin, "sh"), - []byte("#!/bin/sh\n: > \""+marker+"\"\nexit 0\n"), 0o755, + filepath.Join(origin, "payload"), []byte("current\n"), 0o644, )) - lifecycleGit(t, origin, "add", ".gitattributes", "payload", "sh") - lifecycleGit(t, origin, "commit", "-qm", "PATH driver fixture") + lifecycleGit(t, origin, "commit", "-qam", "PATH driver current") headSHA := lifecycleGit(t, origin, "rev-parse", "HEAD") lifecycleGit(t, origin, "checkout", "-q", "main") + lifecycleGit(t, origin, "checkout", "-q", "-b", "path-driver-other") + require.NoError(os.WriteFile( + filepath.Join(origin, "payload"), []byte("other\n"), 0o644, + )) + lifecycleGit(t, origin, "commit", "-qam", "PATH driver other") + lifecycleGit(t, origin, "checkout", "-q", "main") + lifecycleGit(t, clone, "fetch", "-q", "origin", + "refs/heads/path-driver-other:refs/remotes/origin/path-driver-other") lifecycleGit(t, clone, "config", "diff.owned.command", "false") + lifecycleGit(t, clone, "config", "merge.owned.driver", "false") dest := filepath.Join(t.TempDir(), "wt") _, err := CreateWorktreeFromMergeRequest( @@ -712,15 +736,36 @@ func TestCreateWorktreeFromMergeRequestDoesNotPATHSearchDriverHelpers( ExpectedHeadSHA: headSHA, }) require.NoError(err) + require.NoError(os.WriteFile( filepath.Join(dest, "payload"), []byte("changed\n"), 0o644, )) + diffCmd := lifecycleGitCommand(t, dest, "diff", "--", "payload") + diffCmd.Env = append(diffCmd.Env, "PATH="+dest+":"+os.Getenv("PATH")) + diff, err := diffCmd.CombinedOutput() + require.NoError(err, string(diff)) + assert.Contains(string(diff), "-current") + assert.Contains(string(diff), "+changed") + assert.NoFileExists(marker) + require.NoError(os.WriteFile( + filepath.Join(dest, "payload"), []byte("current\n"), 0o644, + )) - cmd := lifecycleGitCommand(t, dest, "diff", "--", "payload") - cmd.Env = append(cmd.Env, "PATH="+dest+":"+os.Getenv("PATH")) - out, err := cmd.CombinedOutput() + mergeCmd := lifecycleGitCommand( + t, dest, "merge", "refs/remotes/origin/path-driver-other", + ) + mergeCmd.Env = append(mergeCmd.Env, "PATH="+dest+":"+os.Getenv("PATH")) + out, err := mergeCmd.CombinedOutput() - require.NoError(err, string(out)) + require.Error(err, string(out)) + assert.Equal("UU payload", + lifecycleGit(t, dest, "status", "--short", "--", "payload")) + payload, readErr := os.ReadFile(filepath.Join(dest, "payload")) + require.NoError(readErr) + assert.Equal( + "<<<<<<< current\ncurrent\n||||||| base\nbase\n=======\nother\n>>>>>>> other\n", + string(payload), + ) assert.NoFileExists(marker) } @@ -867,7 +912,7 @@ func TestCreateWorktreeFromMergeRequestInspectsSelectedConfigFiles(t *testing.T) worktreeOnlyConfig(t, dest, "filter.selected.required")) assert.Equal(safeExternalDiffCommand, worktreeOnlyConfig(t, dest, "diff.selected.command")) - assert.Equal(safeMergeDriverCommand, + assert.Equal(expectedSafeMergeDriverCommand(t, dest), worktreeOnlyConfig(t, dest, "merge.selected.driver")) } @@ -1054,11 +1099,11 @@ func TestSectionLevelConfigKeysDoNotNameSubsections(t *testing.T) { _, ok := configuredSubmoduleName("submodule.path") assert.False(t, ok) - assert.Empty(t, neutralizeAttributeDrivers([]string{ + assert.False(t, configuredAttributeDrivers([]string{ "filter.clean", "diff.command", "merge.driver", - })) + }).configured()) } func TestSafeTextconvPreservesUnterminatedLines(t *testing.T) { diff --git a/git/managed/untrusted_tree.go b/git/managed/untrusted_tree.go index 0376255..902a66b 100644 --- a/git/managed/untrusted_tree.go +++ b/git/managed/untrusted_tree.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "os" + "os/exec" "path/filepath" "regexp" "runtime" @@ -13,6 +14,7 @@ import ( "strings" gitcmd "go.kenn.io/kit/git/cmd" + "go.kenn.io/kit/git/internal/shellquote" ) // untrustedTreeIsolation neutralizes Git programs that a fetched tree can @@ -20,20 +22,72 @@ import ( // configuration remain trusted; only the fetched commit is treated as // untrusted. type untrustedTreeIsolation struct { - runner gitcmd.Runner - config []gitcmd.Config + runner gitcmd.Runner + config []gitcmd.Config + mergeDriverCommand string } // Git invokes these through its compiled-in shell because they contain shell -// syntax. They use only shell builtins, so an untrusted worktree cannot steer -// them through PATH. The diff replacement emits a simple old/new rendering; -// the merge replacement declines the custom driver so Git reports a conflict. +// syntax. The diff and textconv replacements use only shell builtins. The +// merge replacement invokes the resolved Git executable by absolute path, so +// an untrusted worktree cannot steer a helper through PATH. The diff replacement +// emits a simple old/new rendering; the merge replacement performs a fixed-label +// diff3 merge. const ( safeExternalDiffCommand = `f() { emit() { prefix=$1; file=$2; line=; while IFS= read -r line; do printf '%s%s\n' "$prefix" "$line"; line=; done < "$file"; case "$line" in "") ;; *) printf '%s%s\n' "$prefix" "$line"; printf '%s\n' '\ No newline at end of file' ;; esac; }; emit - "$2"; emit + "$5"; }; f` safeTextconvCommand = `f() { line=; while IFS= read -r line; do printf '%s\n' "$line"; line=; done < "$1"; case "$line" in "") ;; *) printf '%s' "$line" ;; esac; }; f` - safeMergeDriverCommand = `f() { return 1; }; f` ) +func resolveMergeDriverGitPath() (string, error) { + path, err := exec.LookPath("git") + if err != nil { + return "", fmt.Errorf("resolve Git executable for safe merge driver: %w", err) + } + path, err = filepath.Abs(path) + if err != nil { + return "", fmt.Errorf("make Git executable path absolute: %w", err) + } + path = filepath.Clean(path) + if runtime.GOOS == "windows" { + path = filepath.ToSlash(path) + } + return path, nil +} + +func safeMergeDriverCommand(gitPath, classifierDir string) string { + // Git expands merge-driver placeholders before the shell parses the + // command. Double literal percent signs before applying shell quoting. + git := shellquote.Single(strings.ReplaceAll(gitPath, "%", "%%")) + classifier := shellquote.Single(strings.ReplaceAll(classifierDir, "%", "%%")) + ceiling := filepath.Dir(classifierDir) + if runtime.GOOS == "windows" { + classifier = shellquote.Single(strings.ReplaceAll( + filepath.ToSlash(classifierDir), "%", "%%", + )) + ceiling = filepath.ToSlash(ceiling) + } + ceiling = shellquote.Single(strings.ReplaceAll(ceiling, "%", "%%")) + return `f() { [ -x ` + git + ` ] || return 129; ` + + `case "$2" in /*|[A-Za-z]:/*) current=$2 ;; *) current=$PWD/$2 ;; esac; ` + + `case "$3" in /*|[A-Za-z]:/*) base=$3 ;; *) base=$PWD/$3 ;; esac; ` + + `case "$4" in /*|[A-Za-z]:/*) other=$4 ;; *) other=$PWD/$4 ;; esac; ` + + `unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_OBJECT_DIRECTORY ` + + `GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_COMMON_DIR GIT_NAMESPACE GIT_PREFIX || return 129; ` + + `classify() { output=$(GIT_CONFIG_COUNT=0 GIT_ATTR_NOSYSTEM=1 GIT_CEILING_DIRECTORIES=` + + ceiling + ` ` + git + + ` -c core.attributesFile= -c core.bigFileThreshold=1023m -C ` + classifier + + ` diff --no-index --numstat --no-ext-diff --no-textconv -- "$1" "$2"); ` + + `status=$?; [ "$status" -le 1 ] || return 129; ` + + `case "$output" in -*) return 1 ;; "") [ "$status" -eq 0 ] || return 129 ;; ` + + `[0-9]*) ;; *) return 129 ;; esac; return 0; }; ` + + `classify "$current" "$base"; status=$?; ` + + `[ "$status" -eq 1 ] && return 1; [ "$status" -eq 0 ] || return "$status"; ` + + `classify "$base" "$other"; status=$?; ` + + `[ "$status" -eq 1 ] && return 1; [ "$status" -eq 0 ] || return "$status"; ` + + git + ` merge-file --diff3 --marker-size="$1" -L current -L base -L other "$2" "$3" "$4"; ` + + `return $?; }; f %L "%A" "%O" "%B"` +} + var untrustedTreeGitVersionPattern = regexp.MustCompile( `(?i)git version (\d+)\.(\d+)(?:\.(\d+))?(?:\.windows\.(\d+))?(?:\s|$)`, ) @@ -55,7 +109,7 @@ func validateUntrustedTreeCheckoutGitVersion( if supportsUntrustedTreeCheckoutGitVersion(string(out), runtime.GOOS) { return nil } - requirement := "Git 2.39.1 or newer" + requirement := "Git 2.42.0 or newer" if runtime.GOOS == "windows" || isGitForWindowsVersion(string(out)) { requirement = "Git for Windows 2.53.0.windows.3 or newer" } @@ -75,7 +129,7 @@ func supportsUntrustedTreeCheckoutGitVersion(output, goos string) bool { major, _ := strconv.Atoi(match[1]) minor, _ := strconv.Atoi(match[2]) patch, _ := strconv.Atoi(match[3]) - if major < 2 || major == 2 && (minor < 39 || minor == 39 && patch < 1) { + if major < 2 || major == 2 && minor < 42 { return false } if goos != "windows" && match[4] == "" { @@ -149,6 +203,10 @@ func prepareUntrustedTreeIsolation( ); err != nil { return untrustedTreeIsolation{}, err } + gitPath, err := resolveMergeDriverGitPath() + if err != nil { + return untrustedTreeIsolation{}, err + } hooksPath, err := managedEmptyHooksPath(ctx, root) if err != nil { return untrustedTreeIsolation{}, err @@ -164,7 +222,11 @@ func prepareUntrustedTreeIsolation( for _, entry := range config { runner = runner.WithConfig(entry.Key, entry.Value) } - return untrustedTreeIsolation{runner: runner, config: config}, nil + return untrustedTreeIsolation{ + runner: runner, + config: config, + mergeDriverCommand: safeMergeDriverCommand(gitPath, hooksPath), + }, nil } func rejectCommandScopeIsolationOverrides( @@ -223,7 +285,7 @@ func isolationSensitiveConfigKey(key string) bool { strings.HasSuffix(lower, ".fetchrecursesubmodules") { return true } - return len(neutralizeAttributeDrivers([]string{key})) != 0 + return configuredAttributeDrivers([]string{key}).configured() } func managedEmptyHooksPath(ctx context.Context, root string) (string, error) { @@ -282,7 +344,9 @@ func completeUntrustedTreeIsolation( strings.Join(hooks, ", "), ) } - drivers := neutralizeAttributeDrivers(keys) + drivers := neutralizeAttributeDrivers( + configuredAttributeDrivers(keys), isolation.mergeDriverCommand, + ) submodules, err := submoduleFetchRecurseConfig( ctx, worktreePath, isolation.runner, ) @@ -523,10 +587,18 @@ func withoutGitRepositoryBindings(env []string) []string { return clean } -func neutralizeAttributeDrivers(keys []string) []gitcmd.Config { - filters := make(map[string]struct{}) - diffs := make(map[string]struct{}) - merges := make(map[string]struct{}) +type attributeDrivers struct { + filters map[string]struct{} + diffs map[string]struct{} + merges map[string]struct{} +} + +func configuredAttributeDrivers(keys []string) attributeDrivers { + drivers := attributeDrivers{ + filters: make(map[string]struct{}), + diffs: make(map[string]struct{}), + merges: make(map[string]struct{}), + } for _, key := range keys { lower := strings.ToLower(key) switch { @@ -534,25 +606,34 @@ func neutralizeAttributeDrivers(keys []string) []gitcmd.Config { if driver, ok := configuredDriverName( key, []string{".clean", ".smudge", ".process", ".required"}, ); ok { - filters[driver] = struct{}{} + drivers.filters[driver] = struct{}{} } case strings.HasPrefix(lower, "diff."): if driver, ok := configuredDriverName( key, []string{".command", ".textconv"}, ); ok { - diffs[driver] = struct{}{} + drivers.diffs[driver] = struct{}{} } case strings.HasPrefix(lower, "merge."): if driver, ok := configuredDriverName( key, []string{".driver"}, ); ok { - merges[driver] = struct{}{} + drivers.merges[driver] = struct{}{} } } } + return drivers +} + +func (drivers attributeDrivers) configured() bool { + return len(drivers.filters)+len(drivers.diffs)+len(drivers.merges) != 0 +} +func neutralizeAttributeDrivers( + drivers attributeDrivers, mergeDriverCommand string, +) []gitcmd.Config { var config []gitcmd.Config - for _, driver := range sortedDriverNames(filters) { + for _, driver := range sortedDriverNames(drivers.filters) { prefix := "filter." + driver config = append(config, gitcmd.Config{Key: prefix + ".clean", Value: ""}, @@ -561,16 +642,16 @@ func neutralizeAttributeDrivers(keys []string) []gitcmd.Config { gitcmd.Config{Key: prefix + ".required", Value: "false"}, ) } - for _, driver := range sortedDriverNames(diffs) { + for _, driver := range sortedDriverNames(drivers.diffs) { prefix := "diff." + driver config = append(config, gitcmd.Config{Key: prefix + ".command", Value: safeExternalDiffCommand}, gitcmd.Config{Key: prefix + ".textconv", Value: safeTextconvCommand}, ) } - for _, driver := range sortedDriverNames(merges) { + for _, driver := range sortedDriverNames(drivers.merges) { config = append(config, gitcmd.Config{ - Key: "merge." + driver + ".driver", Value: safeMergeDriverCommand, + Key: "merge." + driver + ".driver", Value: mergeDriverCommand, }) } return config diff --git a/git/managed/untrusted_tree_merge_test.go b/git/managed/untrusted_tree_merge_test.go new file mode 100644 index 0000000..ce66e84 --- /dev/null +++ b/git/managed/untrusted_tree_merge_test.go @@ -0,0 +1,497 @@ +package managedworktree + +import ( + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.kenn.io/kit/git/internal/shellquote" +) + +type mergeDriverFixture struct { + worktree string + otherRef string +} + +func newMergeDriverFixture( + t *testing.T, base, current, other []byte, + currentSubject, otherBranch string, +) mergeDriverFixture { + t.Helper() + require := require.New(t) + + if currentSubject == "" { + currentSubject = "current change" + } + if otherBranch == "" { + otherBranch = "merge-driver-other" + } + + origin, clone := initOriginAndClone(t) + require.NoError(os.WriteFile( + filepath.Join(origin, ".gitattributes"), + []byte("payload merge=owned\n"), 0o644, + )) + require.NoError(os.WriteFile( + filepath.Join(origin, "payload"), base, 0o644, + )) + require.NoError(os.WriteFile( + filepath.Join(origin, "binary.dat"), []byte("\x00base\n"), 0o644, + )) + lifecycleGit(t, origin, "add", ".gitattributes", "payload", "binary.dat") + lifecycleGit(t, origin, "commit", "-qm", "merge driver base") + + const currentBranch = "merge-driver-current" + lifecycleGit(t, origin, "checkout", "-q", "-b", currentBranch) + require.NoError(os.WriteFile( + filepath.Join(origin, "payload"), current, 0o644, + )) + lifecycleGit(t, origin, "commit", "-qam", currentSubject) + currentSHA := lifecycleGit(t, origin, "rev-parse", "HEAD") + + lifecycleGit(t, origin, "checkout", "-q", "main") + lifecycleGit(t, origin, "checkout", "-q", "-b", otherBranch) + require.NoError(os.WriteFile( + filepath.Join(origin, "payload"), other, 0o644, + )) + lifecycleGit(t, origin, "commit", "-qam", "other change") + otherSHA := lifecycleGit(t, origin, "rev-parse", "HEAD") + lifecycleGit(t, origin, "checkout", "-q", "main") + + otherRef := "refs/remotes/origin/" + otherBranch + lifecycleGit(t, clone, "fetch", "-q", "origin", + "refs/heads/"+otherBranch+":"+otherRef) + require.Equal(otherSHA, lifecycleGit(t, clone, "rev-parse", otherRef)) + lifecycleGit(t, clone, "config", "merge.owned.driver", "false") + + worktree := filepath.Join(t.TempDir(), "worktree") + _, err := CreateWorktreeFromMergeRequest( + t.Context(), MergeRequestWorktreeOptions{ + Runner: lifecycleTestRunner(t), + ProjectRoot: clone, + Branch: "imported-current", + Path: worktree, + Number: 112, + HeadBranch: currentBranch, + HeadRepoCloneURL: origin, + ProjectRepoIdentity: identityOfCloneURL(origin), + ExpectedHeadSHA: currentSHA, + }, + ) + require.NoError(err) + + return mergeDriverFixture{worktree: worktree, otherRef: otherRef} +} + +func TestUntrustedTreeMergeDriverMergesNonOverlappingText(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + fixture := newMergeDriverFixture(t, + []byte("alpha\nmiddle\nomega\n"), + []byte("alpha current\nmiddle\nomega\n"), + []byte("alpha\nmiddle\nomega other\n"), + "", "", + ) + + cmd := lifecycleGitCommand(t, fixture.worktree, "merge", fixture.otherRef) + out, err := cmd.CombinedOutput() + + require.NoError(err, string(out)) + contents, err := os.ReadFile(filepath.Join(fixture.worktree, "payload")) + require.NoError(err) + assert.Contains(string(contents), "alpha current") + assert.Contains(string(contents), "omega other") + assert.Empty(lifecycleGit(t, fixture.worktree, "ls-files", "--unmerged")) +} + +func TestUntrustedTreeMergeDriverWritesDiff3ConflictMarkers(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + fixture := newMergeDriverFixture(t, + []byte("before\nbase\nafter\n"), + []byte("before\ncurrent\nafter\n"), + []byte("before\nother\nafter\n"), + "", "", + ) + + cmd := lifecycleGitCommand(t, fixture.worktree, "merge", fixture.otherRef) + out, err := cmd.CombinedOutput() + + require.Error(err, string(out)) + assert.Equal("UU payload", + lifecycleGit(t, fixture.worktree, "status", "--short", "--", "payload")) + contents, err := os.ReadFile(filepath.Join(fixture.worktree, "payload")) + require.NoError(err) + assert.Equal( + "before\n<<<<<<< current\ncurrent\n||||||| base\nbase\n=======\nother\n>>>>>>> other\nafter\n", + string(contents), + ) +} + +func TestUntrustedTreeMergeDriverDoesNotEvaluateGitLabels(t *testing.T) { + t.Run("merge branch label", func(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + markers := []string{"branch-dollar-marker", "branch-backtick-marker"} + fixture := newMergeDriverFixture(t, + []byte("base\n"), + []byte("current\n"), + []byte("other\n"), + "", + "other-$(touch${IFS}branch-dollar-marker)-"+ + "`touch${IFS}branch-backtick-marker`", + ) + + cmd := lifecycleGitCommand(t, fixture.worktree, "merge", fixture.otherRef) + out, err := cmd.CombinedOutput() + + require.Error(err, string(out)) + assert.Equal("UU payload", + lifecycleGit(t, fixture.worktree, "status", "--short", "--", "payload")) + for _, marker := range markers { + assert.NoFileExists(filepath.Join(fixture.worktree, marker)) + } + contents, err := os.ReadFile(filepath.Join(fixture.worktree, "payload")) + require.NoError(err) + assert.Equal( + "<<<<<<< current\ncurrent\n||||||| base\nbase\n=======\nother\n>>>>>>> other\n", + string(contents), + ) + }) + + t.Run("rebase commit subject label", func(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + markers := []string{"subject-dollar-marker", "subject-backtick-marker"} + fixture := newMergeDriverFixture(t, + []byte("base\n"), + []byte("current\n"), + []byte("other\n"), + "current $(touch${IFS}subject-dollar-marker) "+ + "`touch${IFS}subject-backtick-marker`", + "", + ) + + cmd := lifecycleGitCommand(t, fixture.worktree, "rebase", fixture.otherRef) + out, err := cmd.CombinedOutput() + + require.Error(err, string(out)) + assert.Equal("UU payload", + lifecycleGit(t, fixture.worktree, "status", "--short", "--", "payload")) + for _, marker := range markers { + assert.NoFileExists(filepath.Join(fixture.worktree, marker)) + } + contents, err := os.ReadFile(filepath.Join(fixture.worktree, "payload")) + require.NoError(err) + for _, want := range []string{ + "<<<<<<< current\n", + "||||||| base\n", + "=======\n", + ">>>>>>> other\n", + "base\n", + "current\n", + "other\n", + } { + assert.Contains(string(contents), want) + } + assert.NotContains(string(contents), "subject-dollar-marker") + assert.NotContains(string(contents), "subject-backtick-marker") + }) +} + +func TestUntrustedTreeMergeDriverEscapesPlaceholdersInGitPath(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + markers := []string{"percent-dollar-marker", "percent-backtick-marker"} + fixture := newMergeDriverFixture(t, + []byte("base\n"), + []byte("current\n"), + []byte("other\n"), + "", + "other-$(touch${IFS}percent-dollar-marker)-"+ + "`touch${IFS}percent-backtick-marker`", + ) + missingGit := filepath.Join(t.TempDir(), "missing-%Y-git") + lifecycleGit(t, fixture.worktree, "config", "--worktree", + "merge.owned.driver", safeMergeDriverCommand(missingGit, + worktreeConfig(t, fixture.worktree, "core.hooksPath"))) + + cmd := lifecycleGitCommand(t, fixture.worktree, "merge", fixture.otherRef) + out, err := cmd.CombinedOutput() + + require.Error(err, string(out)) + assert.Empty(lifecycleGit(t, fixture.worktree, "status", "--short")) + assert.Empty(lifecycleGit(t, fixture.worktree, "ls-files", "--unmerged")) + for _, marker := range markers { + assert.NoFileExists(filepath.Join(fixture.worktree, marker)) + } + payload, err := os.ReadFile(filepath.Join(fixture.worktree, "payload")) + require.NoError(err) + assert.Equal("current\n", string(payload)) +} + +func TestUntrustedTreeMergeDriverFailsWhenResolvedGitDisappears(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + fixture := newMergeDriverFixture(t, + []byte("base\n"), []byte("current\n"), []byte("other\n"), "", "", + ) + + gitCopy := filepath.Join(t.TempDir(), "git") + require.NoError(os.WriteFile(gitCopy, nil, 0o755)) + lifecycleGit(t, fixture.worktree, "config", "--worktree", + "merge.owned.driver", safeMergeDriverCommand(gitCopy, + worktreeConfig(t, fixture.worktree, "core.hooksPath"))) + require.NoError(os.Remove(gitCopy)) + + cmd := lifecycleGitCommand(t, fixture.worktree, "merge", fixture.otherRef) + out, err := cmd.CombinedOutput() + + require.Error(err, string(out)) + assert.Empty(lifecycleGit(t, fixture.worktree, "status", "--short")) + assert.Empty(lifecycleGit(t, fixture.worktree, "ls-files", "--unmerged")) + contents, err := os.ReadFile(filepath.Join(fixture.worktree, "payload")) + require.NoError(err) + assert.Equal("current\n", string(contents)) +} + +func TestUntrustedTreeMergeDriverKeepsBinaryConflictLocal(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + fixture := newMergeDriverFixture(t, + []byte("base\n"), []byte("current\n"), []byte("other\n"), "", "", + ) + + otherWorktree := filepath.Join(t.TempDir(), "other") + lifecycleGit(t, fixture.worktree, "worktree", "add", "--detach", + otherWorktree, fixture.otherRef) + for _, worktree := range []string{fixture.worktree, otherWorktree} { + require.NoError(os.WriteFile( + filepath.Join(worktree, ".gitattributes"), + []byte("payload merge=owned -diff\nbinary.dat merge=owned diff=owned\n"), 0o644, + )) + } + lifecycleGit(t, fixture.worktree, "config", "--worktree", + "diff.owned.command", ": > classifier-diff-marker") + lifecycleGit(t, fixture.worktree, "config", "--worktree", + "diff.owned.textconv", ": > classifier-textconv-marker") + currentBinary := []byte("\x00current\n") + otherBinary := []byte("\x00other\n") + require.NoError(os.WriteFile( + filepath.Join(fixture.worktree, "binary.dat"), currentBinary, 0o644, + )) + lifecycleGit(t, fixture.worktree, "add", ".gitattributes", "binary.dat") + lifecycleGit(t, fixture.worktree, "commit", "-qm", "current binary") + require.NoError(os.WriteFile( + filepath.Join(otherWorktree, "binary.dat"), otherBinary, 0o644, + )) + lifecycleGit(t, otherWorktree, "add", ".gitattributes", "binary.dat") + lifecycleGit(t, otherWorktree, "commit", "-qm", "other binary") + otherCommit := lifecycleGit(t, otherWorktree, "rev-parse", "HEAD") + lifecycleGit(t, fixture.worktree, "worktree", "remove", "--force", otherWorktree) + + cmd := lifecycleGitCommand(t, fixture.worktree, "merge", otherCommit) + out, err := cmd.CombinedOutput() + + require.Error(err, string(out)) + status := strings.Split( + lifecycleGit(t, fixture.worktree, "status", "--short"), "\n", + ) + assert.ElementsMatch([]string{"UU binary.dat", "UU payload"}, status) + binary, err := os.ReadFile(filepath.Join(fixture.worktree, "binary.dat")) + require.NoError(err) + assert.Equal(currentBinary, binary) + assert.NotContains(string(binary), "<<<<<<<") + payload, err := os.ReadFile(filepath.Join(fixture.worktree, "payload")) + require.NoError(err) + assert.Contains(string(payload), "<<<<<<< current") + assert.Contains(string(payload), "||||||| base") + assert.Contains(string(payload), ">>>>>>> other") + assert.NotContains(string(out), "Cannot merge binary files: ") + assert.NoFileExists(filepath.Join(fixture.worktree, "classifier-diff-marker")) + assert.NoFileExists(filepath.Join(fixture.worktree, "classifier-textconv-marker")) +} + +func TestUntrustedTreeMergeDriverIgnoresAmbientBigFileThreshold(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + fixture := newMergeDriverFixture(t, + []byte("base\n"), []byte("current\n"), []byte("other\n"), "", "", + ) + + cmd := lifecycleGitCommand(t, fixture.worktree, "merge", fixture.otherRef) + cmd.Env = append(cmd.Env, + "GIT_CONFIG_COUNT=1", + "GIT_CONFIG_KEY_0=core.bigFileThreshold", + "GIT_CONFIG_VALUE_0=1", + ) + out, err := cmd.CombinedOutput() + + require.Error(err, string(out)) + assert.Equal("UU payload", + lifecycleGit(t, fixture.worktree, "status", "--short", "--", "payload")) + contents, err := os.ReadFile(filepath.Join(fixture.worktree, "payload")) + require.NoError(err) + assert.Equal( + "<<<<<<< current\ncurrent\n||||||| base\nbase\n=======\nother\n>>>>>>> other\n", + string(contents), + ) +} + +func TestUntrustedTreeMergeDriverIgnoresGlobalBigFileThreshold(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + fixture := newMergeDriverFixture(t, + []byte("base\n"), []byte("current\n"), []byte("other\n"), "", "", + ) + globalConfig := filepath.Join(t.TempDir(), "global.gitconfig") + require.NoError(os.WriteFile(globalConfig, []byte( + "[core]\n\tbigFileThreshold = 1\n", + ), 0o600)) + + cmd := lifecycleGitCommand(t, fixture.worktree, "merge", fixture.otherRef) + cmd.Env = append(isolatedLifecycleBaseEnv(t), + "GIT_CONFIG_GLOBAL="+globalConfig, + "GIT_CONFIG_NOSYSTEM=1", + ) + out, err := cmd.CombinedOutput() + + require.Error(err, string(out)) + assert.Equal("UU payload", + lifecycleGit(t, fixture.worktree, "status", "--short", "--", "payload")) + contents, err := os.ReadFile(filepath.Join(fixture.worktree, "payload")) + require.NoError(err) + assert.Equal( + "<<<<<<< current\ncurrent\n||||||| base\nbase\n=======\nother\n>>>>>>> other\n", + string(contents), + ) +} + +func TestUntrustedTreeMergeDriverClearsInheritedRepositoryBindings(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + fixture := newMergeDriverFixture(t, + []byte("base\n"), []byte("current\n"), []byte("other\n"), "", "", + ) + + require.NoError(os.WriteFile( + filepath.Join(fixture.worktree, ".gitattributes"), + []byte("payload merge=owned\n.merge_file_* diff=owned\n"), 0o644, + )) + lifecycleGit(t, fixture.worktree, "add", ".gitattributes") + lifecycleGit(t, fixture.worktree, "commit", "-qm", "bind merge temp attributes") + lifecycleGit(t, fixture.worktree, "config", "--worktree", + "diff.owned.binary", "true") + lifecycleGit(t, fixture.worktree, "config", "--worktree", + "diff.owned.command", ": > classifier-binding-diff-marker") + lifecycleGit(t, fixture.worktree, "config", "--worktree", + "diff.owned.textconv", ": > classifier-binding-textconv-marker") + gitDir := lifecycleGit(t, fixture.worktree, "rev-parse", "--absolute-git-dir") + outside := t.TempDir() + classifier := worktreeConfig(t, fixture.worktree, "core.hooksPath") + require.NotEmpty(classifier) + + cmd := lifecycleGitCommand(t, outside, + "--git-dir="+gitDir, + "--work-tree="+fixture.worktree, + "merge", fixture.otherRef, + ) + out, err := cmd.CombinedOutput() + + require.Error(err, string(out)) + assert.Equal("UU payload", + lifecycleGit(t, fixture.worktree, "status", "--short", "--", "payload")) + contents, err := os.ReadFile(filepath.Join(fixture.worktree, "payload")) + require.NoError(err) + assert.Equal( + "<<<<<<< current\ncurrent\n||||||| base\nbase\n=======\nother\n>>>>>>> other\n", + string(contents), + ) + // The classifier runs Git with -C in the hooks directory, so a helper + // invoked there would leave its marker in that directory rather than in + // the worktree or the caller's directory. + for _, dir := range []string{fixture.worktree, outside, classifier} { + assert.NoFileExists(filepath.Join(dir, "classifier-binding-diff-marker")) + assert.NoFileExists(filepath.Join(dir, "classifier-binding-textconv-marker")) + } +} + +func TestUntrustedTreeMergeDriverTreatsMergeFile255AsOperationError(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the merge-file error simulator is a POSIX shell script") + } + require := require.New(t) + assert := assert.New(t) + fixture := newMergeDriverFixture(t, + []byte("base\n"), []byte("current\n"), []byte("other\n"), "", "", + ) + + gitPath, err := exec.LookPath("git") + require.NoError(err) + gitPath, err = filepath.Abs(gitPath) + require.NoError(err) + simulator := filepath.Join(t.TempDir(), "git") + script := "#!/bin/sh\n" + + "if [ \"$1\" = merge-file ]; then echo simulated-output-failure >&2; exit 255; fi\n" + + "exec " + shellquote.Single(gitPath) + " \"$@\"\n" + require.NoError(os.WriteFile(simulator, []byte(script), 0o755)) + require.NoError(os.Chmod(simulator, 0o755)) + lifecycleGit(t, fixture.worktree, "config", "--worktree", + "merge.owned.driver", safeMergeDriverCommand(simulator, + worktreeConfig(t, fixture.worktree, "core.hooksPath"))) + + cmd := lifecycleGitCommand(t, fixture.worktree, "merge", fixture.otherRef) + out, err := cmd.CombinedOutput() + + require.Error(err, string(out)) + assert.Contains(string(out), "simulated-output-failure") + assert.Empty(lifecycleGit(t, fixture.worktree, "status", "--short")) + assert.Empty(lifecycleGit(t, fixture.worktree, "ls-files", "--unmerged")) + contents, err := os.ReadFile(filepath.Join(fixture.worktree, "payload")) + require.NoError(err) + assert.Equal("current\n", string(contents)) +} + +func TestUntrustedTreeMergeDriverTreatsMergeFileCrashAsOperationError(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the merge-file crash simulator is a POSIX shell script") + } + require := require.New(t) + assert := assert.New(t) + fixture := newMergeDriverFixture(t, + []byte("base\n"), []byte("current\n"), []byte("other\n"), "", "", + ) + + gitPath, err := exec.LookPath("git") + require.NoError(err) + gitPath, err = filepath.Abs(gitPath) + require.NoError(err) + simulator := filepath.Join(t.TempDir(), "git") + script := "#!/bin/sh\n" + + "if [ \"$1\" = merge-file ]; then kill -KILL $$; fi\n" + + "exec " + shellquote.Single(gitPath) + " \"$@\"\n" + require.NoError(os.WriteFile(simulator, []byte(script), 0o755)) + require.NoError(os.Chmod(simulator, 0o755)) + lifecycleGit(t, fixture.worktree, "config", "--worktree", + "merge.owned.driver", safeMergeDriverCommand(simulator, + worktreeConfig(t, fixture.worktree, "core.hooksPath"))) + + cmd := lifecycleGitCommand(t, fixture.worktree, "merge", fixture.otherRef) + out, err := cmd.CombinedOutput() + + // A signal-killed merge process must abort the whole operation rather + // than record a conflict whose working file holds only the current side. + require.Error(err, string(out)) + assert.Empty(lifecycleGit(t, fixture.worktree, "status", "--short")) + assert.Empty(lifecycleGit(t, fixture.worktree, "ls-files", "--unmerged")) + contents, err := os.ReadFile(filepath.Join(fixture.worktree, "payload")) + require.NoError(err) + assert.Equal("current\n", string(contents)) +} diff --git a/git/managed/untrusted_tree_test.go b/git/managed/untrusted_tree_test.go index 296ca13..fe83aac 100644 --- a/git/managed/untrusted_tree_test.go +++ b/git/managed/untrusted_tree_test.go @@ -18,8 +18,9 @@ func TestSupportsUntrustedTreeCheckoutGitVersion(t *testing.T) { goos string want bool }{ - {output: "git version 2.39.0", goos: "linux"}, - {output: "git version 2.39.1", goos: "linux", want: true}, + {output: "git version 2.41.0", goos: "linux"}, + {output: "git version 2.41.9", goos: "linux"}, + {output: "git version 2.42.0", goos: "linux", want: true}, {output: "git version 2.45.2 (Apple Git-145)", goos: "darwin", want: true}, {output: "git version 2.52.2.windows.4", goos: "linux"}, {output: "git version 2.52.2.windows.4", goos: "windows"},