fix(#333): unify env var blocklist for sandbox and credentials

internal/cli/run.go

@@ -1141,6 +1141,10 @@ func setupFetchService(ctx context.Context, forgeClient forge.Client, h *harness
 }
 
 func bootstrapEnv(sandboxName, remoteRepositoryDir string, h *harness.Harness, runtimeEnvExports []string, fetchEnv ...fetchServiceEnv) error {
+	if err := sandbox.ValidateEnvKeys(h.RunnerEnv); err != nil {
+		return fmt.Errorf("validating runner_env: %w", err)
+	}
+
 	remoteEnvFile := sandbox.SandboxWorkspace + "/.env"
 	outputDir := sandbox.SandboxWorkspace + "/output"
 

internal/cli/run_test.go

@@ -1222,6 +1222,54 @@ func TestLockCommand_HasForgeFlag(t *testing.T) {
 	assert.Equal(t, "", flag.DefValue)
 }
 
+func TestBootstrapEnv_RejectsReservedRunnerEnvKey(t *testing.T) {
+	h := &harness.Harness{
+		Agent:     "agents/test.md",
+		RunnerEnv: map[string]string{"LD_PRELOAD": "/malicious.so"},
+	}
+
+	err := bootstrapEnv("nonexistent-sandbox", "/workspace/repo", h, nil)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "LD_PRELOAD")
+	assert.Contains(t, err.Error(), "reserved")
+}
+
+func TestBootstrapEnv_RejectsProxyOverride(t *testing.T) {
+	h := &harness.Harness{
+		Agent:     "agents/test.md",
+		RunnerEnv: map[string]string{"HTTP_PROXY": "http://evil.proxy:8080"},
+	}
+
+	err := bootstrapEnv("nonexistent-sandbox", "/workspace/repo", h, nil)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "HTTP_PROXY")
+	assert.Contains(t, err.Error(), "reserved")
+}
+
+func TestBootstrapEnv_RejectsFullsendPrefix(t *testing.T) {
+	h := &harness.Harness{
+		Agent:     "agents/test.md",
+		RunnerEnv: map[string]string{"FULLSEND_OUTPUT_DIR": "/override"},
+	}
+
+	err := bootstrapEnv("nonexistent-sandbox", "/workspace/repo", h, nil)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "FULLSEND_OUTPUT_DIR")
+	assert.Contains(t, err.Error(), "reserved")
+}
+
+func TestBootstrapEnv_AcceptsSafeRunnerEnvKeys(t *testing.T) {
+	h := &harness.Harness{
+		Agent:     "agents/test.md",
+		RunnerEnv: map[string]string{"REPO_NAME": "my-repo", "GH_TOKEN": "ghp_test"},
+	}
+
+	err := bootstrapEnv("nonexistent-sandbox", "/workspace/repo", h, nil)
+	// Expected to fail at sandbox.UploadFile — but not at validation.
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "copying .env file to sandbox")
+}
+
 func TestBootstrapEnv_IncludesFetchServiceVars(t *testing.T) {
 	h := &harness.Harness{Agent: "agents/test.md"}
 	fEnv := fetchServiceEnv{addr: "127.0.0.1:54321", token: "deadbeef"}

internal/sandbox/reserved.go

@@ -0,0 +1,118 @@
+package sandbox
+
+import (
+	"fmt"
+	"strings"
+)
+
+// ReservedEnvKeys is the unified set of environment variable names that must
+// never be used as provider credential keys or sandbox environment variable
+// names. A variable that is unsafe as a provider credential key is equally
+// unsafe when injected via runner_env, since both paths result in env vars
+// in the sandbox child process.
+//
+// Categories:
+//   - Infrastructure: core process environment variables
+//   - Dynamic linker injection: library preloading vectors
+//   - Shell injection: startup script variables
+//   - Proxy / network: HTTP(S) proxy override variables
+//   - TLS trust chain: certificate trust store overrides
+//   - Git config: git configuration injection vectors
+//   - Runtime injection: language-specific startup/path injection
+//
+// The FULLSEND_ prefix is also reserved (checked by [IsReservedEnvKey]).
+var ReservedEnvKeys = map[string]bool{
+	// Infrastructure
+	"PATH":     true,
+	"HOME":     true,
+	"SHELL":    true,
+	"USER":     true,
+	"LOGNAME":  true,
+	"HOSTNAME": true,
+	"TERM":     true,
+
+	// Dynamic linker injection
+	"LD_PRELOAD":            true,
+	"LD_LIBRARY_PATH":       true,
+	"DYLD_INSERT_LIBRARIES": true,
+	"DYLD_LIBRARY_PATH":     true,
+
+	// Shell injection
+	"BASH_ENV":       true,
+	"ENV":            true,
+	"PROMPT_COMMAND": true,
+
+	// Proxy / network
+	"HTTP_PROXY":  true,
+	"HTTPS_PROXY": true,
+	"http_proxy":  true,
+	"https_proxy": true,
+	"ALL_PROXY":   true,
+	"all_proxy":   true,
+	"NO_PROXY":    true,
+	"no_proxy":    true,
+	"FTP_PROXY":   true,
+	"ftp_proxy":   true,
+
+	// TLS trust chain
+	"SSL_CERT_FILE":       true,
+	"SSL_CERT_DIR":        true,
+	"NODE_EXTRA_CA_CERTS": true,
+	"REQUESTS_CA_BUNDLE":  true,
+	"CURL_CA_BUNDLE":      true,
+	"GIT_SSL_CAINFO":      true,
+	"GIT_SSL_CAPATH":      true,
+	"GIT_SSL_NO_VERIFY":   true,
+	"PIP_CERT":            true,
+	"AWS_CA_BUNDLE":       true,
+
+	// Git config
+	"GIT_CONFIG_GLOBAL": true,
+	"GIT_CONFIG_SYSTEM": true,
+	"GIT_EXEC_PATH":     true,
+	"GIT_TEMPLATE_DIR":  true,
+
+	// Runtime injection
+	"PYTHONSTARTUP": true,
+	"PYTHONPATH":    true,
+	"NODE_OPTIONS":  true,
+	"PERL5LIB":      true,
+	"PERL5OPT":      true,
+	"RUBYLIB":       true,
+	"RUBYOPT":       true,
+}
+
+// IsReservedEnvKey reports whether key is a reserved environment variable
+// name. It checks both the exact key against [ReservedEnvKeys] and the
+// FULLSEND_ prefix.
+func IsReservedEnvKey(key string) bool {
+	if ReservedEnvKeys[key] {
+		return true
+	}
+	return strings.HasPrefix(key, "FULLSEND_")
+}
+
+// ValidateCredentialKeys checks that none of the credential key names in
+// the map are reserved environment variables. Returns an error naming the
+// first reserved key found.
+func ValidateCredentialKeys(credentials map[string]string) error {
+	for key := range credentials {
+		if IsReservedEnvKey(key) {
+			return fmt.Errorf("credential key %q is a reserved environment variable and cannot be used", key)
+		}
+	}
+	return nil
+}
+
+// ValidateEnvKeys checks that none of the environment variable key names
+// in the map are reserved. Returns an error naming the first reserved key
+// found. Use this to validate runner_env keys before injecting them into
+// the sandbox.
+func ValidateEnvKeys(env map[string]string) error {
+	for key := range env {
+		if IsReservedEnvKey(key) {
+			return fmt.Errorf("environment key %q is a reserved variable and cannot be set in sandbox env", key)
+		}
+	}
+	return nil
+}

internal/sandbox/reserved_test.go

@@ -0,0 +1,212 @@
+package sandbox
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestReservedEnvKeys_CoversSecurity verifies that all security-sensitive
+// categories are represented in the unified blocklist. This prevents future
+// drift where one category is accidentally omitted.
+func TestReservedEnvKeys_CoversSecurity(t *testing.T) {
+	categories := map[string][]string{
+		"infrastructure":  {"PATH", "HOME", "SHELL"},
+		"dynamic_linker":  {"LD_PRELOAD", "LD_LIBRARY_PATH"},
+		"shell_injection": {"BASH_ENV", "ENV"},
+		"proxy": {
+			"HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy",
+			"ALL_PROXY", "all_proxy", "NO_PROXY", "no_proxy",
+		},
+		"tls_trust_chain": {
+			"SSL_CERT_FILE", "SSL_CERT_DIR", "NODE_EXTRA_CA_CERTS",
+			"REQUESTS_CA_BUNDLE", "CURL_CA_BUNDLE",
+			"GIT_SSL_CAINFO", "GIT_SSL_CAPATH", "GIT_SSL_NO_VERIFY",
+		},
+		"git_config": {
+			"GIT_CONFIG_GLOBAL", "GIT_CONFIG_SYSTEM",
+			"GIT_EXEC_PATH", "GIT_TEMPLATE_DIR",
+		},
+		"runtime_injection": {
+			"PYTHONSTARTUP", "PYTHONPATH", "NODE_OPTIONS",
+			"PERL5LIB", "PERL5OPT", "RUBYLIB", "RUBYOPT",
+		},
+	}
+	for category, keys := range categories {
+		for _, key := range keys {
+			assert.True(t, ReservedEnvKeys[key],
+				"ReservedEnvKeys must include %q (category: %s)", key, category)
+		}
+	}
+}
+
+func TestIsReservedEnvKey_ExactMatch(t *testing.T) {
+	reserved := []string{
+		"LD_PRELOAD", "LD_LIBRARY_PATH",
+		"HTTP_PROXY", "HTTPS_PROXY",
+		"PATH", "HOME", "SHELL",
+		"BASH_ENV", "ENV",
+		"SSL_CERT_FILE", "GIT_CONFIG_GLOBAL",
+		"NODE_OPTIONS", "PYTHONSTARTUP",
+	}
+	for _, key := range reserved {
+		assert.True(t, IsReservedEnvKey(key), "%q should be reserved", key)
+	}
+}
+
+func TestIsReservedEnvKey_FullsendPrefix(t *testing.T) {
+	assert.True(t, IsReservedEnvKey("FULLSEND_OUTPUT_DIR"))
+	assert.True(t, IsReservedEnvKey("FULLSEND_TOKEN"))
+	assert.True(t, IsReservedEnvKey("FULLSEND_TRACE_ID"))
+}
+
+func TestIsReservedEnvKey_SafeKeys(t *testing.T) {
+	safe := []string{
+		"API_KEY",
+		"MY_SECRET",
+		"ANTHROPIC_API_KEY",
+		"OPENAI_API_KEY",
+		"GH_TOKEN",
+		"REPO_NAME",
+	}
+	for _, key := range safe {
+		assert.False(t, IsReservedEnvKey(key), "%q should not be reserved", key)
+	}
+}
+
+func TestValidateCredentialKeys_RejectsReserved(t *testing.T) {
+	tests := []struct {
+		name        string
+		credentials map[string]string
+		wantKey     string
+	}{
+		{
+			name:        "LD_PRELOAD",
+			credentials: map[string]string{"API_KEY": "secret", "LD_PRELOAD": "/malicious.so"},
+			wantKey:     "LD_PRELOAD",
+		},
+		{
+			name:        "HTTP_PROXY",
+			credentials: map[string]string{"HTTP_PROXY": "http://evil.proxy"},
+			wantKey:     "HTTP_PROXY",
+		},
+		{
+			name:        "FULLSEND_prefix",
+			credentials: map[string]string{"FULLSEND_TOKEN": "secret"},
+			wantKey:     "FULLSEND_TOKEN",
+		},
+		{
+			name:        "PATH",
+			credentials: map[string]string{"PATH": "/override"},
+			wantKey:     "PATH",
+		},
+		{
+			name:        "GIT_CONFIG_GLOBAL",
+			credentials: map[string]string{"GIT_CONFIG_GLOBAL": "/evil/gitconfig"},
+			wantKey:     "GIT_CONFIG_GLOBAL",
+		},
+		{
+			name:        "SSL_CERT_FILE",
+			credentials: map[string]string{"SSL_CERT_FILE": "/evil/cert.pem"},
+			wantKey:     "SSL_CERT_FILE",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidateCredentialKeys(tt.credentials)
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), tt.wantKey)
+			assert.Contains(t, err.Error(), "reserved")
+		})
+	}
+}
+
+func TestValidateCredentialKeys_AcceptsSafe(t *testing.T) {
+	creds := map[string]string{
+		"API_KEY":       "secret",
+		"ANTHROPIC_KEY": "sk-ant-1234",
+		"OPENAI_KEY":    "sk-1234",
+	}
+	require.NoError(t, ValidateCredentialKeys(creds))
+}
+
+func TestValidateCredentialKeys_EmptyMap(t *testing.T) {
+	require.NoError(t, ValidateCredentialKeys(map[string]string{}))
+}
+
+func TestValidateCredentialKeys_NilMap(t *testing.T) {
+	require.NoError(t, ValidateCredentialKeys(nil))
+}
+
+func TestValidateEnvKeys_RejectsReserved(t *testing.T) {
+	tests := []struct {
+		name    string
+		env     map[string]string
+		wantKey string
+	}{
+		{
+			name:    "LD_PRELOAD",
+			env:     map[string]string{"MY_VAR": "safe", "LD_PRELOAD": "/malicious.so"},
+			wantKey: "LD_PRELOAD",
+		},
+		{
+			name:    "HTTP_PROXY",
+			env:     map[string]string{"HTTP_PROXY": "http://evil.proxy"},
+			wantKey: "HTTP_PROXY",
+		},
+		{
+			name:    "NODE_OPTIONS",
+			env:     map[string]string{"NODE_OPTIONS": "--require /evil.js"},
+			wantKey: "NODE_OPTIONS",
+		},
+		{
+			name:    "FULLSEND_prefix",
+			env:     map[string]string{"FULLSEND_OUTPUT_DIR": "/override"},
+			wantKey: "FULLSEND_OUTPUT_DIR",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidateEnvKeys(tt.env)
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), tt.wantKey)
+			assert.Contains(t, err.Error(), "reserved")
+		})
+	}
+}
+
+func TestValidateEnvKeys_AcceptsSafe(t *testing.T) {
+	env := map[string]string{
+		"MY_VAR":    "safe",
+		"REPO_NAME": "fullsend",
+		"GH_TOKEN":  "${GH_TOKEN}",
+	}
+	require.NoError(t, ValidateEnvKeys(env))
+}
+
+func TestValidateEnvKeys_EmptyMap(t *testing.T) {
+	require.NoError(t, ValidateEnvKeys(map[string]string{}))
+}
+
+func TestValidateEnvKeys_NilMap(t *testing.T) {
+	require.NoError(t, ValidateEnvKeys(nil))
+}
+
+// TestBlocklistCoversLowerCaseProxyVariants ensures that both upper and lower
+// case proxy variables are blocked. Many HTTP clients honour both forms.
+func TestBlocklistCoversLowerCaseProxyVariants(t *testing.T) {
+	pairs := [][2]string{
+		{"HTTP_PROXY", "http_proxy"},
+		{"HTTPS_PROXY", "https_proxy"},
+		{"ALL_PROXY", "all_proxy"},
+		{"NO_PROXY", "no_proxy"},
+		{"FTP_PROXY", "ftp_proxy"},
+	}
+	for _, pair := range pairs {
+		assert.True(t, ReservedEnvKeys[pair[0]],
+			"upper-case %q should be in blocklist", pair[0])
+		assert.True(t, ReservedEnvKeys[pair[1]],
+			"lower-case %q should be in blocklist", pair[1])
+	}
+}