diff --git a/AGENTS.md b/AGENTS.md index ded02a4..87c2673 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -73,6 +73,7 @@ Notes on the tests and on configuration: | `internal/s3fs` | Daemon-level state only, which is the SSH host key. | | `internal/mountfs`, `internal/treefs`, `internal/kefkash` | The hook sandbox filesystem and shell wiring. | | `internal/metrics` | Every Prometheus vector, plus thin helpers. | +| `internal/gittest` | `Isolate`, which keeps the real git client in tests away from this repository. | | `internal/snapshot` | erofs images of git trees: `Ensure`, `Open`, and the `Store` interface. | | `internal/slog.go` | JSON handler init. | | `cmd/membench/` | Push memory benchmark harness. Not shipped; see `docs/usage/memory-benchmark.md`. | @@ -112,6 +113,10 @@ Two more directories carry detail: `http_test.go` and `git_protocol_test.go`. - Reuse the shared test helpers `runGit`, `tryGit`, and `seedRepo`. They live in `git_protocol_test.go`. +- If a package runs the real git client in its tests, call `gittest.Isolate` + from its `TestMain`. The husky `commit-msg` hook runs `go test`, and git + exports `GIT_DIR` to hooks. Without `Isolate`, a test can write to the + config that all worktrees share and set `core.bare = true`. - Put a plan for non-trivial work in `docs/plans/`. `git-http-protocol.md` shows the style. - Put architecture notes in `docs/architecture/`, and not in this file. Keep diff --git a/cmd/objgitd/git_protocol_test.go b/cmd/objgitd/git_protocol_test.go index 88803c6..f40c4de 100644 --- a/cmd/objgitd/git_protocol_test.go +++ b/cmd/objgitd/git_protocol_test.go @@ -6,7 +6,6 @@ import ( "io" "log/slog" "net" - "os" "os/exec" "path/filepath" "strings" @@ -295,11 +294,6 @@ func runGit(t *testing.T, dir string, args ...string) string { func tryGit(dir string, args ...string) (string, error) { cmd := exec.Command("git", args...) cmd.Dir = dir - cmd.Env = append(os.Environ(), - "GIT_CONFIG_GLOBAL=/dev/null", - "GIT_CONFIG_SYSTEM=/dev/null", - "GIT_TERMINAL_PROMPT=0", - ) out, err := cmd.CombinedOutput() return string(out), err } diff --git a/cmd/objgitd/gitenv_test.go b/cmd/objgitd/gitenv_test.go new file mode 100644 index 0000000..5ae0a92 --- /dev/null +++ b/cmd/objgitd/gitenv_test.go @@ -0,0 +1,14 @@ +package main + +import ( + "testing" + + "github.com/tigrisdata/objgit/internal/gittest" +) + +// TestMain strips the GIT_* variables that a git hook exports, so the real +// git client in these tests can not write to the repository that runs them. +func TestMain(m *testing.M) { + gittest.Isolate() + m.Run() +} diff --git a/cmd/objgitd/ssh_test.go b/cmd/objgitd/ssh_test.go index bd05673..7e4eab6 100644 --- a/cmd/objgitd/ssh_test.go +++ b/cmd/objgitd/ssh_test.go @@ -167,12 +167,7 @@ func gitSSHEnv(t *testing.T) []string { t.Helper() key := sshClientKey(t) sshCmd := fmt.Sprintf("ssh -i %q -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", key) - return append(os.Environ(), - "GIT_SSH_COMMAND="+sshCmd, - "GIT_TERMINAL_PROMPT=0", - "GIT_CONFIG_GLOBAL=/dev/null", - "GIT_CONFIG_SYSTEM=/dev/null", - ) + return append(os.Environ(), "GIT_SSH_COMMAND="+sshCmd) } // gitWithEnv runs git with a custom environment, returning combined output and diff --git a/internal/gittest/gittest.go b/internal/gittest/gittest.go new file mode 100644 index 0000000..238969f --- /dev/null +++ b/internal/gittest/gittest.go @@ -0,0 +1,29 @@ +// Package gittest keeps the real git client used by tests away from the +// repository that runs the tests. +package gittest + +import ( + "os" + "strings" +) + +// Isolate removes every GIT_* variable from the process environment. Then it +// points git at empty global and system config files, and turns off +// credential prompts. Call it from TestMain in each package that runs git. +// +// Git exports GIT_DIR, GIT_INDEX_FILE, and other variables to hooks. The +// husky commit-msg hook runs go test. Without Isolate, a test's `git init` or +// `git config` writes to the repository that runs the hook, and not to the +// test's temp dir. In a worktree, that is the config that all worktrees share. +// It gets core.bare = true, and then every worktree stops working. +func Isolate() { + for _, kv := range os.Environ() { + name, _, _ := strings.Cut(kv, "=") + if strings.HasPrefix(name, "GIT_") { + os.Unsetenv(name) + } + } + os.Setenv("GIT_CONFIG_GLOBAL", os.DevNull) + os.Setenv("GIT_CONFIG_SYSTEM", os.DevNull) + os.Setenv("GIT_TERMINAL_PROMPT", "0") +} diff --git a/internal/gittest/gittest_test.go b/internal/gittest/gittest_test.go new file mode 100644 index 0000000..66fb54c --- /dev/null +++ b/internal/gittest/gittest_test.go @@ -0,0 +1,126 @@ +package gittest + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// TestMain isolates this package too. The victim repositories that +// TestIsolate sets up must not see the variables of a real hook. +func TestMain(m *testing.M) { + Isolate() + m.Run() +} + +// TestIsolate simulates a test binary run from a git hook, where git has +// exported GIT_DIR and friends. After Isolate, a test's own git commands +// must not touch the repository that ran the hook. +func TestIsolate(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not on PATH") + } + + for _, tt := range []struct { + name string + env func(victim string) map[string]string + }{ + { + name: "commit-msg hook in main checkout", + env: func(victim string) map[string]string { + return map[string]string{ + "GIT_DIR": filepath.Join(victim, ".git"), + "GIT_INDEX_FILE": filepath.Join(victim, ".git", "index"), + } + }, + }, + { + name: "commit-msg hook in worktree", + env: func(victim string) map[string]string { + wt := filepath.Join(filepath.Dir(victim), "wt") + git(t, victim, "worktree", "add", "-q", wt) + gitDir := filepath.Join(victim, ".git", "worktrees", "wt") + return map[string]string{ + "GIT_DIR": gitDir, + "GIT_INDEX_FILE": filepath.Join(gitDir, "index"), + } + }, + }, + { + name: "git -c parameters", + env: func(string) map[string]string { + return map[string]string{ + "GIT_CONFIG_PARAMETERS": "'user.name'='Leaked'", + } + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + victim := filepath.Join(t.TempDir(), "victim") + git(t, "", "init", "-q", victim) + git(t, victim, "-c", "user.name=a", "-c", "user.email=a@example.com", + "commit", "-q", "--allow-empty", "-m", "init") + configPath := filepath.Join(victim, ".git", "config") + before := readFile(t, configPath) + + // t.Setenv registers a restore for each name that Isolate touches. + for _, name := range []string{"GIT_CONFIG_GLOBAL", "GIT_CONFIG_SYSTEM", "GIT_TERMINAL_PROMPT"} { + t.Setenv(name, os.Getenv(name)) + } + for k, v := range tt.env(victim) { + t.Setenv(k, v) + } + + Isolate() + + for _, kv := range os.Environ() { + name, _, _ := strings.Cut(kv, "=") + switch name { + case "GIT_CONFIG_GLOBAL", "GIT_CONFIG_SYSTEM", "GIT_TERMINAL_PROMPT": + default: + if strings.HasPrefix(name, "GIT_") { + t.Errorf("%s still set after Isolate", name) + } + } + } + + // This is what the protocol tests do in their temp dirs. + scratch := t.TempDir() + git(t, scratch, "init", "-q", "--bare", "bare.git") + git(t, scratch, "init", "-q", "work") + work := filepath.Join(scratch, "work") + git(t, work, "config", "user.name", "Test") + git(t, work, "config", "user.email", "test@example.com") + git(t, work, "commit", "-q", "--allow-empty", "-m", "test") + + if after := readFile(t, configPath); after != before { + t.Errorf("victim config changed:\n--- before\n%s\n--- after\n%s", before, after) + } + if got := strings.TrimSpace(git(t, work, "log", "-1", "--format=%an")); got != "Test" { + t.Errorf("commit author = %q, want Test", got) + } + }) + } +} + +func git(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return string(out) +} + +func readFile(t *testing.T, path string) string { + t.Helper() + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(b) +} diff --git a/internal/storage/tigris/gitenv_test.go b/internal/storage/tigris/gitenv_test.go new file mode 100644 index 0000000..887a457 --- /dev/null +++ b/internal/storage/tigris/gitenv_test.go @@ -0,0 +1,14 @@ +package tigris + +import ( + "testing" + + "github.com/tigrisdata/objgit/internal/gittest" +) + +// TestMain strips the GIT_* variables that a git hook exports, so the real +// git client in these tests can not write to the repository that runs them. +func TestMain(m *testing.M) { + gittest.Isolate() + m.Run() +} diff --git a/internal/storage/tigris/pack_test.go b/internal/storage/tigris/pack_test.go index b99aa07..abe3660 100644 --- a/internal/storage/tigris/pack_test.go +++ b/internal/storage/tigris/pack_test.go @@ -637,17 +637,12 @@ type packFixture struct { byHash map[plumbing.Hash]fixtureObj } -// gitOutput runs git in dir with an isolated config, feeding stdin if -// non-nil, and fails the test on any error. +// gitOutput runs git in dir, feeding stdin if non-nil, and fails the test on +// any error. TestMain isolates git from the caller's config. func gitOutput(t *testing.T, dir string, stdin []byte, args ...string) []byte { t.Helper() cmd := exec.Command("git", args...) cmd.Dir = dir - cmd.Env = append(os.Environ(), - "GIT_CONFIG_GLOBAL=/dev/null", - "GIT_CONFIG_SYSTEM=/dev/null", - "GIT_TERMINAL_PROMPT=0", - ) if stdin != nil { cmd.Stdin = bytes.NewReader(stdin) }