From a3f1a7707219e822ce1fd224bce17a35d53b0c13 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 22 Jun 2026 20:10:48 -0500 Subject: [PATCH 1/2] Bound safe.directory git probes --- git/AGENTS.md | 2 ++ git/cmd/gitcmd.go | 7 ++++++- git/cmd/gitcmd_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/git/AGENTS.md b/git/AGENTS.md index 1a153dd..ee08185 100644 --- a/git/AGENTS.md +++ b/git/AGENTS.md @@ -13,6 +13,8 @@ not about one product's workflow. - Do not call `exec.Command("git", ...)` directly in package code unless the direct call is the behavior being tested. - Pass `context.Context` through git operations that can block. +- Keep best-effort helper probes bounded; they must not stall the caller's real + git command when optional ambient state is slow or broken. - Return git failures with captured stderr. Do not hide git's message behind a generic error. - Use `gitlock` around mutations that can race with another process touching diff --git a/git/cmd/gitcmd.go b/git/cmd/gitcmd.go index 0a60795..c339426 100644 --- a/git/cmd/gitcmd.go +++ b/git/cmd/gitcmd.go @@ -22,6 +22,7 @@ import ( "strconv" "strings" "sync" + "time" gitenv "go.kenn.io/kit/git/env" ) @@ -183,6 +184,8 @@ var ( emptyGlobalConfigPath string ) +var safeDirectoryProbeTimeout = 2 * time.Second + // nullGlobalConfigPath returns a path suitable for GIT_CONFIG_GLOBAL that makes // git read an empty (no-op) global config. // @@ -237,10 +240,12 @@ func readSafeDirectories(ctx context.Context, env []string, dir string) []string for _, scope := range scopes { // --includes is required for explicit-scope reads to honor include.path // and includeIf directives the way git's default config sequence does. - cmd := gitCommand(ctx, true, "config", scope, "--includes", "-z", "--get-all", "safe.directory") + probeCtx, cancel := context.WithTimeout(ctx, safeDirectoryProbeTimeout) + cmd := gitCommand(probeCtx, true, "config", scope, "--includes", "-z", "--get-all", "safe.directory") cmd.Dir = dir cmd.Env = env out, err := cmd.Output() + cancel() if err != nil || len(out) == 0 { continue } diff --git a/git/cmd/gitcmd_test.go b/git/cmd/gitcmd_test.go index 6496fa0..2c62f83 100644 --- a/git/cmd/gitcmd_test.go +++ b/git/cmd/gitcmd_test.go @@ -4,10 +4,13 @@ import ( "context" "encoding/base64" "os" + "os/exec" "path/filepath" + "runtime" "slices" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -128,6 +131,24 @@ func TestReadSafeDirectoriesHonorsNoSystem(t *testing.T) { assert.Equal(t, []string{"/home/repo"}, got) } +func TestReadSafeDirectoriesBoundsProbeRuntime(t *testing.T) { + origTimeout := safeDirectoryProbeTimeout + safeDirectoryProbeTimeout = 50 * time.Millisecond + t.Cleanup(func() { safeDirectoryProbeTimeout = origTimeout }) + + binDir := buildSleepingGit(t) + env := append(os.Environ(), + "PATH="+binDir+string(os.PathListSeparator)+os.Getenv("PATH"), + "GIT_CONFIG_NOSYSTEM=0", + ) + + start := time.Now() + got := readSafeDirectories(context.Background(), env, "") + + assert.Empty(t, got) + assert.Less(t, time.Since(start), time.Second, "safe.directory probes are best-effort and must not stall git commands") +} + func TestReadSafeDirectoriesConditionalInclude(t *testing.T) { // Regression test: the probes must run in the command's directory with // --includes so includeIf "gitdir:..." entries resolve for the repository @@ -298,6 +319,29 @@ func captureGitEnv(t *testing.T, runner Runner) string { return string(envBytes) } +func buildSleepingGit(t *testing.T) string { + t.Helper() + binDir := t.TempDir() + exeName := "git" + if runtime.GOOS == "windows" { + exeName += ".exe" + } + exePath := filepath.Join(binDir, exeName) + srcPath := filepath.Join(t.TempDir(), "main.go") + require.NoError(t, os.WriteFile(srcPath, []byte(`package main + +import "time" + +func main() { + time.Sleep(10 * time.Second) +} +`), 0o600)) + cmd := exec.Command("go", "build", "-o", exePath, srcPath) + out, err := cmd.CombinedOutput() + require.NoError(t, err, string(out)) + return binDir +} + func gitConfigValue(env, key string) string { values := map[string]string{} keys := map[string]string{} From 3373c3487188f50ef9ca73ef856b095d9bee6539 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 22 Jun 2026 21:17:08 -0500 Subject: [PATCH 2/2] Fix safe-directory probe regression test PATH --- git/cmd/gitcmd_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/git/cmd/gitcmd_test.go b/git/cmd/gitcmd_test.go index 2c62f83..d7ffe9b 100644 --- a/git/cmd/gitcmd_test.go +++ b/git/cmd/gitcmd_test.go @@ -137,10 +137,9 @@ func TestReadSafeDirectoriesBoundsProbeRuntime(t *testing.T) { t.Cleanup(func() { safeDirectoryProbeTimeout = origTimeout }) binDir := buildSleepingGit(t) - env := append(os.Environ(), - "PATH="+binDir+string(os.PathListSeparator)+os.Getenv("PATH"), - "GIT_CONFIG_NOSYSTEM=0", - ) + pathEnv := binDir + string(os.PathListSeparator) + os.Getenv("PATH") + t.Setenv("PATH", pathEnv) + env := append(os.Environ(), "PATH="+pathEnv, "GIT_CONFIG_NOSYSTEM=0") start := time.Now() got := readSafeDirectories(context.Background(), env, "")