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

Add Env(EnvOptions) as the single place that builds a git command's environment: the always-applied scrub set, plus transport variables inherited from the parent when set (SSH agent, PATH, TLS, proxy), plus caller-supplied literals appended last so they override. Re-express Command in terms of Env so there is one composer, not two.

This gives the change provider's repository and the Runway merger one source of truth to build on instead of each keeping its own copy of the scrub set and transport list. HOME is intentionally excluded from the shared transport list, since callers that isolate HOME and callers that inherit it disagree; each supplies it through Literal or Passthrough.

Author: behinddwalls

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")
+}