feat(git/exec): a shared environment composer for git commands

platform/git/exec/BUILD.bazel

@@ -1,8 +1,18 @@
-load("@rules_go//go:def.bzl", "go_library")
+load("@rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
     srcs = ["gitexec.go"],
     importpath = "github.com/uber/submitqueue/platform/git/exec",
     visibility = ["//visibility:public"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["gitexec_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "@com_github_stretchr_testify//assert:go_default_library",
+        "@com_github_stretchr_testify//require:go_default_library",
+    ],
+)

platform/git/exec/gitexec.go

@@ -12,19 +12,23 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package gitexec locates a git binary and runs it with the ambient
-// environment stripped out.
+// Package gitexec locates a git binary and composes the environment git runs
+// in. It is the single source of truth for that environment across every
+// SubmitQueue caller — demo tooling, the change provider's repository, and the
+// Runway merger.
 //
-// Demo and development tooling drives git on a developer's own machine, where
-// hooks, a signing key, or a commit template configured globally would each
-// break a run in a way that has nothing to do with SubmitQueue. Every command
-// built here therefore carries the same scrubbed environment the git merger
-// uses (see runway/extension/merger/git), so tooling behaves the same on every
-// machine.
+// The environment has two halves. The scrub set denies git all ambient
+// configuration that could change what a command produces — a global hooks
+// path, a signing requirement, a commit template — which is what makes a
+// scripted run behave the same on every machine. The transport set carries
+// what a command needs to reach a remote — the SSH agent socket, git's ssh and
+// credential helpers on PATH, TLS roots, proxy settings — none of which can
+// change an answer. Every caller shares both halves; they differ only in the
+// literal entries they add (a pinned exec path, an isolated HOME), which is why
+// Env takes those as options rather than baking one caller's policy in.
 //
-// This resolves only the executable, because tooling runs porcelain
-// (init, clone, commit, push) rather than constructing a merger's GitRuntime,
-// which additionally pins the exec path and template directory.
+// HOME is deliberately not in the transport set: a caller that isolates HOME
+// and a caller that inherits it disagree, so each supplies it itself.
 package gitexec
 
 import (
@@ -67,23 +71,75 @@ func Resolve(path string) (string, error) {
 	return absolute, nil
 }
 
+// scrubEnv denies git every ambient configuration input that could change what
+// a command produces. Always applied, first, so a later entry can override it.
+var scrubEnv = []string{
+	"GIT_CONFIG_NOSYSTEM=1",
+	"GIT_CONFIG_GLOBAL=" + os.DevNull,
+	"GIT_ATTR_NOSYSTEM=1",
+	"GIT_TERMINAL_PROMPT=0",
+	"GIT_PAGER=cat",
+	"GIT_EDITOR=:",
+}
+
+// transportEnvNames are inherited from the parent process when set. None can
+// change what a command produces; each decides whether a remote is reachable.
+// HOME is intentionally absent — see the package doc.
+var transportEnvNames = []string{
+	"PATH",
+	"SSH_AUTH_SOCK", "SSH_AGENT_PID",
+	"GIT_SSH", "GIT_SSH_COMMAND", "GIT_SSH_VARIANT",
+	"GIT_SSL_CAINFO", "GIT_SSL_CAPATH",
+	"SSL_CERT_DIR", "SSL_CERT_FILE",
+	"HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY",
+	"http_proxy", "https_proxy", "no_proxy",
+}
+
+// EnvOptions selects what, on top of the always-applied scrub set, a git
+// command's environment carries.
+type EnvOptions struct {
+	// Transport inherits the transport variables from the parent when set.
+	Transport bool
+	// Passthrough names further variables to inherit from the parent when set.
+	Passthrough []string
+	// Literal entries are appended last as "NAME=value", so they override any
+	// inherited value of the same name.
+	Literal []string
+}
+
+// Env composes a git command environment: the scrub set, then the requested
+// variables inherited from the parent (only those actually set, so an unset
+// SSH_AUTH_SOCK stays absent rather than becoming empty), then the literals.
+func Env(opts EnvOptions) []string {
+	env := make([]string, 0, len(scrubEnv)+len(transportEnvNames)+len(opts.Passthrough)+len(opts.Literal))
+	env = append(env, scrubEnv...)
+
+	names := make([]string, 0, len(transportEnvNames)+len(opts.Passthrough))
+	if opts.Transport {
+		names = append(names, transportEnvNames...)
+	}
+	names = append(names, opts.Passthrough...)
+
+	seen := make(map[string]bool, len(names))
+	for _, name := range names {
+		if name == "" || seen[name] {
+			continue
+		}
+		seen[name] = true
+		if v, ok := os.LookupEnv(name); ok {
+			env = append(env, name+"="+v)
+		}
+	}
+	return append(env, opts.Literal...)
+}
+
 // Command builds a git invocation in dir with the ambient environment removed.
-// An empty dir runs in the current working directory.
+// An empty dir runs in the current working directory. PATH is passed so git can
+// find its helpers; nothing else the host sets reaches the command.
 func Command(ctx context.Context, git, dir string, args ...string) *exec.Cmd {
 	cmd := exec.CommandContext(ctx, git, args...)
 	cmd.Dir = dir
-	// A developer's global config is the usual reason a scripted git run fails
-	// on one machine and not another: a hooks path, a signing requirement, or a
-	// commit template. None of it is relevant to seeding a sandbox.
-	cmd.Env = []string{
-		"GIT_CONFIG_NOSYSTEM=1",
-		"GIT_CONFIG_GLOBAL=" + os.DevNull,
-		"GIT_ATTR_NOSYSTEM=1",
-		"GIT_TERMINAL_PROMPT=0",
-		"GIT_PAGER=cat",
-		"GIT_EDITOR=:",
-		"PATH=" + os.Getenv("PATH"),
-	}
+	cmd.Env = Env(EnvOptions{Literal: []string{"PATH=" + os.Getenv("PATH")}})
 	return cmd
 }
 

platform/git/exec/gitexec_test.go

@@ -0,0 +1,97 @@
+// Copyright (c) 2025 Uber Technologies, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package gitexec
+
+import (
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// value returns the value of NAME=value entry for name, and whether it is
+// present at all.
+func value(env []string, name string) (string, bool) {
+	prefix := name + "="
+	found := ""
+	ok := false
+	for _, e := range env {
+		if strings.HasPrefix(e, prefix) {
+			found = strings.TrimPrefix(e, prefix)
+			ok = true
+		}
+	}
+	return found, ok
+}
+
+func TestEnv_AlwaysScrubs(t *testing.T) {
+	env := Env(EnvOptions{})
+	for _, want := range scrubEnv {
+		assert.Contains(t, env, want)
+	}
+}
+
+func TestEnv_TransportInheritedOnlyWhenSet(t *testing.T) {
+	t.Setenv("SSH_AUTH_SOCK", "/tmp/agent.sock")
+
+	withTransport := Env(EnvOptions{Transport: true})
+	got, ok := value(withTransport, "SSH_AUTH_SOCK")
+	assert.True(t, ok)
+	assert.Equal(t, "/tmp/agent.sock", got)
+
+	withoutTransport := Env(EnvOptions{})
+	_, ok = value(withoutTransport, "SSH_AUTH_SOCK")
+	assert.False(t, ok)
+}
+
+func TestEnv_UnsetTransportVarStaysAbsent(t *testing.T) {
+	// An unset SSH_AUTH_SOCK means "there is no agent", so it must be absent
+	// rather than exported empty. t.Setenv records the original for restoration;
+	// Unsetenv then removes it for the duration of the test.
+	t.Setenv("SSH_AUTH_SOCK", "placeholder")
+	require.NoError(t, os.Unsetenv("SSH_AUTH_SOCK"))
+
+	env := Env(EnvOptions{Transport: true})
+	_, ok := value(env, "SSH_AUTH_SOCK")
+	assert.False(t, ok)
+}
+
+func TestEnv_LiteralOverridesInherited(t *testing.T) {
+	t.Setenv("PATH", "/host/bin")
+
+	env := Env(EnvOptions{Transport: true, Literal: []string{"PATH=/pinned/bin"}})
+	got, ok := value(env, "PATH")
+	assert.True(t, ok)
+	assert.Equal(t, "/pinned/bin", got)
+}
+
+func TestEnv_PassthroughDeduplicatesWithTransport(t *testing.T) {
+	t.Setenv("PATH", "/host/bin")
+
+	env := Env(EnvOptions{Transport: true, Passthrough: []string{"PATH"}})
+	count := 0
+	for _, e := range env {
+		if strings.HasPrefix(e, "PATH=") {
+			count++
+		}
+	}
+	assert.Equal(t, 1, count)
+}
+
+func TestEnv_HomeNotInSharedTransportList(t *testing.T) {
+	assert.NotContains(t, transportEnvNames, "HOME")
+}