From 98a1b2d3a54887853a8e3e07c8837dbd05503805 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Wed, 8 Jul 2026 21:26:50 +0100 Subject: [PATCH] test: isolate the git environment in cmd/ git-subprocess tests The cmd/ test helpers scoped the repo (-C ) but ran git with append(os.Environ()), which inherits host GIT_* (a stray GIT_DIR could redirect a command at the real repo) and reads the developer's real ~/.gitconfig. Add a package-cmd gitIsolatedEnv() mirroring the evals harness helper: strip every inherited GIT_* var, neutralise system/global config discovery (GIT_CONFIG_NOSYSTEM=1, GIT_CONFIG_GLOBAL=/dev/null), and pin a distinctive ferry-test identity via env. The repo's LOCAL .git/config is untouched, so the git-hardening tests still exercise their hostile local config. Also drops the bare 't ' fixture identity that made the recent mis-attribution leak hard to trace. Defence-in-depth for the concurrency-corruption hazard in issue #17. Refs #17. --- cmd/git_harden_test.go | 7 +++--- cmd/gittest_test.go | 42 ++++++++++++++++++++++++++++++++++++ cmd/init_github_gate_test.go | 3 +-- cmd/sync_restore_test.go | 7 ++---- 4 files changed, 48 insertions(+), 11 deletions(-) create mode 100644 cmd/gittest_test.go diff --git a/cmd/git_harden_test.go b/cmd/git_harden_test.go index 51a460c..5fa7e3a 100644 --- a/cmd/git_harden_test.go +++ b/cmd/git_harden_test.go @@ -18,8 +18,7 @@ import ( func rawGit(t *testing.T, repo string, args ...string) { t.Helper() cmd := exec.Command("git", append([]string{"-C", repo}, args...)...) - cmd.Env = append(os.Environ(), "GIT_PAGER=cat", - "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@t", "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@t") + cmd.Env = gitIsolatedEnv("GIT_PAGER=cat") if out, err := cmd.CombinedOutput(); err != nil { t.Fatalf("git %v: %v\n%s", args, err, out) } @@ -82,7 +81,7 @@ func TestGitSync_InsteadOfExtNeutralized(t *testing.T) { rawMarker := filepath.Join(markerDir, "raw.txt") writeExtConfig(rawMarker) rawFetch := exec.Command("git", "-C", repo, "-c", "fetch.recurseSubmodules=no", "fetch", "--no-recurse-submodules", "--no-tags", "origin") - rawFetch.Env = append(os.Environ(), "GIT_PAGER=cat", "GIT_TERMINAL_PROMPT=0") + rawFetch.Env = gitIsolatedEnv("GIT_PAGER=cat") _, _ = rawFetch.CombinedOutput() if _, err := os.Stat(rawMarker); err != nil { t.Skipf("ext:: did not fire under raw git on this platform (%v) — fixture inert here", err) @@ -112,7 +111,7 @@ func TestGitSync_MaliciousFsmonitorNeutralized(t *testing.T) { // Baseline: raw (unhardened) git status FIRES the fsmonitor — the fixture is real. rawStatus := exec.Command("git", "-C", repo, "status", "--porcelain") - rawStatus.Env = append(os.Environ(), "GIT_PAGER=cat") + rawStatus.Env = gitIsolatedEnv("GIT_PAGER=cat") _, _ = rawStatus.CombinedOutput() if _, err := os.Stat(rawMarker); err != nil { t.Skipf("fsmonitor did not fire under raw git on this platform (%v) — fixture inert here", err) diff --git a/cmd/gittest_test.go b/cmd/gittest_test.go new file mode 100644 index 0000000..41ac344 --- /dev/null +++ b/cmd/gittest_test.go @@ -0,0 +1,42 @@ +package cmd + +import ( + "os" + "strings" +) + +// gitIsolatedEnv returns the environment for a git subprocess in a cmd-package +// test that cannot read or mutate the developer's real git configuration, or any +// repo outside the test's temp space — even if a call omits `-C`/`cmd.Dir`, or the +// ambient repo's git state is corrupted (e.g. core.bare flipped by a concurrent +// operation). It mirrors the evals harness helper of the same name. +// +// `append(os.Environ(), …)` is unsafe here: an inherited GIT_DIR/GIT_WORK_TREE +// could redirect the command at the host repo, and git would read the host +// ~/.gitconfig — making tests non-deterministic and, under adverse conditions, +// letting a test's identity or fixture commits leak into the working repo. So this +// strips every inherited GIT_* variable, then pins a deterministic identity, +// non-interactive behaviour, and neutralised config discovery (no system config, +// an empty global config). The repo's own LOCAL .git/config is untouched, so tests +// that seed hostile local config (git-hardening tests) still exercise it. +// +// The identity is deliberately distinctive (`ferry-test`) so that if it ever does +// leak, it is instantly recognisable as a test fixture — unlike the bare `t` that +// previously made such a leak hard to trace. +func gitIsolatedEnv(extra ...string) []string { + env := make([]string, 0, len(os.Environ())+8) + for _, kv := range os.Environ() { + if strings.HasPrefix(kv, "GIT_") { + continue // drop GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE/GIT_CONFIG*/… host bleed + } + env = append(env, kv) + } + env = append(env, + "GIT_TERMINAL_PROMPT=0", + "GIT_CONFIG_NOSYSTEM=1", + "GIT_CONFIG_GLOBAL=/dev/null", + "GIT_AUTHOR_NAME=ferry-test", "GIT_AUTHOR_EMAIL=ferry-test@localhost", + "GIT_COMMITTER_NAME=ferry-test", "GIT_COMMITTER_EMAIL=ferry-test@localhost", + ) + return append(env, extra...) +} diff --git a/cmd/init_github_gate_test.go b/cmd/init_github_gate_test.go index f6f822c..2e387c0 100644 --- a/cmd/init_github_gate_test.go +++ b/cmd/init_github_gate_test.go @@ -26,13 +26,12 @@ func TestWholeCommitGate_BlocksNonZshrcFile(t *testing.T) { repo := t.TempDir() run := func(args ...string) { cmd := exec.Command("git", append([]string{"-C", repo}, args...)...) + cmd.Env = gitIsolatedEnv() if out, err := cmd.CombinedOutput(); err != nil { t.Fatalf("git %v: %v\n%s", args, err, out) } } run("init", "-q") - run("config", "user.email", "t@localhost") - run("config", "user.name", "t") // A committed file that is NOT ~/.zshrc — e.g. a generated manifest — carrying a // fake secret. The gate must catch it. diff --git a/cmd/sync_restore_test.go b/cmd/sync_restore_test.go index 9dc5c23..b8c1625 100644 --- a/cmd/sync_restore_test.go +++ b/cmd/sync_restore_test.go @@ -17,10 +17,7 @@ import ( func testGit(t *testing.T, repo string, args ...string) string { t.Helper() cmd := exec.Command("git", append([]string{"-C", repo}, args...)...) - cmd.Env = append(os.Environ(), - "GIT_TERMINAL_PROMPT=0", "GIT_PAGER=cat", - "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@t", "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@t", - ) + cmd.Env = gitIsolatedEnv("GIT_PAGER=cat") out, err := cmd.CombinedOutput() if err != nil { t.Fatalf("git %v: %v\n%s", args, err, out) @@ -69,7 +66,7 @@ func newStashRepo(t *testing.T) (repo string, s *snapshot, realStashSHA string) func stashCount(t *testing.T, repo string) int { t.Helper() cmd := exec.Command("git", "-C", repo, "stash", "list") - cmd.Env = append(os.Environ(), "GIT_PAGER=cat") + cmd.Env = gitIsolatedEnv("GIT_PAGER=cat") out, _ := cmd.CombinedOutput() n := 0 for _, l := range strings.Split(strings.TrimSpace(string(out)), "\n") {