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..d7ffe9b 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,23 @@ 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) + 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, "") + + 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 +318,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{}