diff --git a/internal/cli/app.go b/internal/cli/app.go index a138f014..a6088c2f 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -755,11 +755,12 @@ func runInteractiveTUIWithSetup(stderr io.Writer, deps appDeps, permissionMode a } sandboxBackend := deps.selectSandboxBackend(sandbox.BackendOptions{}) sandboxEngine := sandbox.NewEngine(sandbox.EngineOptions{ - WorkspaceRoot: workspaceRoot, - Policy: applyConfiguredSandboxPolicy(sandbox.DefaultPolicy(), resolved.Sandbox), - Store: sandboxStore, - Backend: sandboxBackend, - Scope: scope, + WorkspaceRoot: workspaceRoot, + Policy: applyConfiguredSandboxPolicy(sandbox.DefaultPolicy(), resolved.Sandbox), + Store: sandboxStore, + Backend: sandboxBackend, + Scope: scope, + SensitiveEnvKeys: providerSensitiveEnvKeys(resolved), }) lastKnownMCPConfig := mcpConfig fileTracker := tools.NewFileTracker() diff --git a/internal/cli/exec.go b/internal/cli/exec.go index 3d553dee..85b36ad5 100644 --- a/internal/cli/exec.go +++ b/internal/cli/exec.go @@ -815,14 +815,28 @@ func buildExecSandboxEngine(workspaceRoot string, resolved config.ResolvedConfig policy := applyConfiguredSandboxPolicy(sandbox.DefaultPolicy(), resolved.Sandbox) backend := deps.selectSandboxBackend(sandbox.BackendOptions{}) return sandbox.NewEngine(sandbox.EngineOptions{ - WorkspaceRoot: workspaceRoot, - Policy: policy, - Store: store, - Backend: backend, - Scope: scope, + WorkspaceRoot: workspaceRoot, + Policy: policy, + Store: store, + Backend: backend, + Scope: scope, + SensitiveEnvKeys: providerSensitiveEnvKeys(resolved), }), nil } +func providerSensitiveEnvKeys(resolved config.ResolvedConfig) []string { + keys := make([]string, 0, len(resolved.Providers)+1) + for _, profile := range resolved.Providers { + if key := strings.TrimSpace(profile.APIKeyEnv); key != "" { + keys = append(keys, key) + } + } + if key := strings.TrimSpace(resolved.Provider.APIKeyEnv); key != "" { + keys = append(keys, key) + } + return keys +} + // applyConfiguredSandboxPolicy overlays every config-sourced sandbox knob onto // the default policy. func applyConfiguredSandboxPolicy(policy sandbox.Policy, cfg config.SandboxConfig) sandbox.Policy { diff --git a/internal/cli/sandbox_sensitive_env_test.go b/internal/cli/sandbox_sensitive_env_test.go new file mode 100644 index 00000000..28e3a2e5 --- /dev/null +++ b/internal/cli/sandbox_sensitive_env_test.go @@ -0,0 +1,23 @@ +package cli + +import ( + "reflect" + "testing" + + "github.com/Gitlawb/zero/internal/config" +) + +func TestProviderSensitiveEnvKeys(t *testing.T) { + resolved := config.ResolvedConfig{ + Providers: []config.ProviderProfile{ + {Name: "custom", APIKeyEnv: " COMPANY_LLM_SECRET "}, + {Name: "catalog", APIKeyEnv: "OPENAI_API_KEY"}, + {Name: "inline"}, + }, + Provider: config.ProviderProfile{Name: "active", APIKeyEnv: "ACTIVE_PROVIDER_SECRET"}, + } + want := []string{"COMPANY_LLM_SECRET", "OPENAI_API_KEY", "ACTIVE_PROVIDER_SECRET"} + if got := providerSensitiveEnvKeys(resolved); !reflect.DeepEqual(got, want) { + t.Fatalf("providerSensitiveEnvKeys() = %v, want %v", got, want) + } +} diff --git a/internal/sandbox/engine.go b/internal/sandbox/engine.go index 162bd96e..ecb2f1cc 100644 --- a/internal/sandbox/engine.go +++ b/internal/sandbox/engine.go @@ -14,18 +14,22 @@ type EngineOptions struct { Store *GrantStore Backend Backend Scope *Scope + // SensitiveEnvKeys adds config-derived credential variable names to the + // catalog and namespace secrets scrubbed from sandboxed commands. + SensitiveEnvKeys []string } type Engine struct { - workspaceRoot string - policy Policy - store *GrantStore - backend Backend - scope *Scope - sessionGrants *memoryGrantSet - sessionProfiles *permissionProfileGrantSet - turnProfiles *permissionProfileGrantSet - commandPrefixes *commandPrefixGrantSet + workspaceRoot string + policy Policy + store *GrantStore + backend Backend + scope *Scope + sensitiveEnvKeys []string + sessionGrants *memoryGrantSet + sessionProfiles *permissionProfileGrantSet + turnProfiles *permissionProfileGrantSet + commandPrefixes *commandPrefixGrantSet } func NewEngine(options EngineOptions) *Engine { @@ -49,15 +53,16 @@ func NewEngine(options EngineOptions) *Engine { scope = newScopeBestEffort(workspaceRoot) } return &Engine{ - workspaceRoot: workspaceRoot, - policy: policy, - store: options.Store, - backend: options.Backend, - scope: scope, - sessionGrants: newMemoryGrantSet(), - sessionProfiles: newPermissionProfileGrantSet(), - turnProfiles: newPermissionProfileGrantSet(), - commandPrefixes: newCommandPrefixGrantSet(), + workspaceRoot: workspaceRoot, + policy: policy, + store: options.Store, + backend: options.Backend, + scope: scope, + sensitiveEnvKeys: normalizeSensitiveEnvKeys(options.SensitiveEnvKeys), + sessionGrants: newMemoryGrantSet(), + sessionProfiles: newPermissionProfileGrantSet(), + turnProfiles: newPermissionProfileGrantSet(), + commandPrefixes: newCommandPrefixGrantSet(), } } diff --git a/internal/sandbox/runner.go b/internal/sandbox/runner.go index 4bcf8380..506acb39 100644 --- a/internal/sandbox/runner.go +++ b/internal/sandbox/runner.go @@ -29,6 +29,9 @@ type CommandSpec struct { Args []string Dir string Env []string + // sensitiveEnvKeys is populated by Engine from config-derived credential + // variable names and carried through SandboxManager to the platform plan. + sensitiveEnvKeys []string } type CommandPlan struct { @@ -122,6 +125,7 @@ func (engine *Engine) BuildCommandPlan(spec CommandSpec) (CommandPlan, error) { return CommandPlan{}, errors.New("sandbox command name is required") } spec.Dir = commandDir + spec.sensitiveEnvKeys = engine.sensitiveEnvKeys backend := engine.backend if backend.Name == "" { @@ -207,7 +211,7 @@ func linuxSandboxHelperCommandPlan(execRequest SandboxExecutionRequest, policy P if err != nil { return CommandPlan{}, err } - env := sandboxEnvironmentForCommand(spec.Env, policy, BackendLinuxBwrap, "") + env := sandboxEnvironmentForCommandWithSensitiveEnv(spec.Env, policy, BackendLinuxBwrap, "", spec.sensitiveEnvKeys) planDir := spec.Dir if helper.Dir != "" { planDir = helper.Dir @@ -253,10 +257,27 @@ func directCommandPlan(spec CommandSpec, backend Backend, policy Policy, workspa Name: spec.Name, Args: cloneStrings(spec.Args), Dir: spec.Dir, - Env: cloneStrings(spec.Env), + Env: directCommandEnv(spec), } } +// directCommandEnv scrubs sensitive credentials from the environment for a +// direct (unwrapped) command plan: the platform sandbox backend is +// unavailable, disabled, or not required, so this is the actual environment +// the child process inherits. Without scrubbing here, config-derived +// apiKeyEnv and dynamic OAuth client-secret variables would leak into +// commands that fall back to this path (e.g. EnforcementDegraded). +func directCommandEnv(spec CommandSpec) []string { + env := cloneStrings(spec.Env) + if spec.Env == nil { + // Match the wrapped-plan behavior: an unset spec.Env means "inherit the + // caller's environment," so scrub a snapshot of it rather than passing + // nil through to exec.Cmd, which would skip scrubbing entirely. + env = os.Environ() + } + return scrubSensitiveEnv(env, spec.sensitiveEnvKeys...) +} + func (engine *Engine) resolveCommandDir(dir string, policy Policy) (string, string, error) { workspaceRoot := strings.TrimSpace(engine.workspaceRoot) if workspaceRoot == "" { @@ -318,7 +339,7 @@ func seatbeltCommandPlanWithProfile(spec CommandSpec, workspaceRoot string, prof if envBackend == "" { envBackend = BackendMacOSSeatbelt } - env := sandboxEnvironmentForCommand(spec.Env, policy, envBackend, "") + env := sandboxEnvironmentForCommandWithSensitiveEnv(spec.Env, policy, envBackend, "", spec.sensitiveEnvKeys) plan := CommandPlan{ Backend: backend, TargetBackend: backend.TargetBackend(), @@ -377,6 +398,10 @@ func sandboxEnvironment(policy Policy, backend BackendName, workspaceRoot string } func sandboxEnvironmentForCommand(specEnv []string, policy Policy, backend BackendName, workspaceRoot string) []string { + return sandboxEnvironmentForCommandWithSensitiveEnv(specEnv, policy, backend, workspaceRoot, nil) +} + +func sandboxEnvironmentForCommandWithSensitiveEnv(specEnv []string, policy Policy, backend BackendName, workspaceRoot string, sensitiveEnvKeys []string) []string { env := cloneStrings(specEnv) if specEnv == nil { // Preserve the caller environment for sandboxed commands. The sandbox @@ -384,7 +409,7 @@ func sandboxEnvironmentForCommand(specEnv []string, policy Policy, backend Backe // command env values still replace inherited values below. env = os.Environ() } - env = scrubSensitiveEnv(env) + env = scrubSensitiveEnv(env, sensitiveEnvKeys...) pathValue := envListValue(env, "PATH", defaultPath()) if runtime.GOOS == "darwin" { // Preserve standard user tool locations so a bare `python3`/`node` @@ -967,7 +992,7 @@ func regexpQuoteMeta(value string) string { return replacer.Replace(value) } -func scrubSensitiveEnv(env []string) []string { +func scrubSensitiveEnv(env []string, additionalKeys ...string) []string { // Secrets not covered by the provider catalog: cloud/VCS credentials and // providers Zero talks to through generic OpenAI-compatible endpoints. sensitiveKeys := []string{ @@ -997,6 +1022,7 @@ func scrubSensitiveEnv(env []string) []string { sensitiveKeys = append(sensitiveKeys, key) } } + sensitiveKeys = append(sensitiveKeys, normalizeSensitiveEnvKeys(additionalKeys)...) out := make([]string, 0, len(env)) for _, kv := range env { @@ -1005,7 +1031,7 @@ func scrubSensitiveEnv(env []string) []string { out = append(out, kv) continue } - drop := false + drop := isDynamicSensitiveEnvKey(key) for _, sensitive := range sensitiveKeys { if strings.EqualFold(key, sensitive) { drop = true @@ -1018,3 +1044,37 @@ func scrubSensitiveEnv(env []string) []string { } return out } + +func normalizeSensitiveEnvKeys(keys []string) []string { + out := make([]string, 0, len(keys)) + seen := make(map[string]struct{}, len(keys)) + for _, key := range keys { + key = strings.TrimSpace(key) + // A misconfigured value like "COMPANY_LLM_SECRET=..." would never + // match a real env key during scrubbing; keep only the name part. + if name, _, found := strings.Cut(key, "="); found { + key = strings.TrimSpace(name) + } + folded := strings.ToUpper(key) + if key == "" { + continue + } + if _, ok := seen[folded]; ok { + continue + } + seen[folded] = struct{}{} + out = append(out, key) + } + return out +} + +func isDynamicSensitiveEnvKey(key string) bool { + const ( + prefix = "ZERO_OAUTH_" + suffix = "_CLIENT_SECRET" + ) + key = strings.ToUpper(strings.TrimSpace(key)) + return strings.HasPrefix(key, prefix) && + strings.HasSuffix(key, suffix) && + len(key) > len(prefix)+len(suffix) +} diff --git a/internal/sandbox/runner_test.go b/internal/sandbox/runner_test.go index 5e7a5b25..67b66866 100644 --- a/internal/sandbox/runner_test.go +++ b/internal/sandbox/runner_test.go @@ -122,6 +122,47 @@ func TestBuildCommandPlanDegradesUnavailableFallback(t *testing.T) { } } +// TestBuildCommandPlanDegradedFallbackScrubsInheritedEnv covers the +// EnforcementDegraded fallback path: when the native backend is +// unavailable, BuildCommandPlan falls back to a direct (unwrapped) plan +// whose spec.Env is nil, so exec.Cmd would otherwise inherit the caller's +// environment — including configured and dynamically named credentials — +// unscrubbed. +func TestBuildCommandPlanDegradedFallbackScrubsInheritedEnv(t *testing.T) { + t.Setenv("COMPANY_LLM_SECRET", "custom-secret") + t.Setenv("ZERO_OAUTH_ACME_CLIENT_SECRET", "oauth-secret") + t.Setenv("SAFE_VAR", "hello") + + root := t.TempDir() + engine := NewEngine(EngineOptions{ + WorkspaceRoot: root, + Policy: DefaultPolicy(), + Backend: Backend{Name: BackendUnavailable, Message: "native sandbox unavailable"}, + SensitiveEnvKeys: []string{"COMPANY_LLM_SECRET"}, + }) + + plan, err := engine.BuildCommandPlan(CommandSpec{ + Name: "/bin/sh", + Args: []string{"-c", "pwd"}, + Dir: root, + }) + if err != nil { + t.Fatalf("BuildCommandPlan: %v", err) + } + if plan.Wrapped || plan.EnforcementLevel != EnforcementDegraded { + t.Fatalf("plan = %#v, want degraded direct plan", plan) + } + for _, entry := range plan.Env { + key, _, _ := strings.Cut(entry, "=") + if strings.EqualFold(key, "COMPANY_LLM_SECRET") || strings.EqualFold(key, "ZERO_OAUTH_ACME_CLIENT_SECRET") { + t.Fatalf("degraded plan.Env retained sensitive key %q: %v", key, plan.Env) + } + } + if got := envListValue(plan.Env, "SAFE_VAR", ""); got != "hello" { + t.Fatalf("SAFE_VAR = %q, want hello", got) + } +} + func TestBuildCommandPlanRejectsOutsideDirectory(t *testing.T) { root := t.TempDir() engine := NewEngine(EngineOptions{ @@ -653,14 +694,19 @@ func TestScrubSensitiveEnv(t *testing.T) { "XAI_API_KEY=xai-12345", "HUGGINGFACE_API_KEY=hf_12345", "GOOGLE_APPLICATION_CREDENTIALS=/home/user/sa-key.json", + "COMPANY_LLM_SECRET=custom-secret", + "ZERO_OAUTH_MY_SVC_CLIENT_SECRET=oauth-secret", + "zero_oauth_second_client_secret=case-insensitive-secret", + "ZERO_OAUTH_CLIENT_SECRET=not-a-provider-secret", "AWS_PROFILE=staging", "SAFE_VAR=hello", } - scrubbed := scrubSensitiveEnv(inputEnv) + scrubbed := scrubSensitiveEnv(inputEnv, " COMPANY_LLM_SECRET ", "company_llm_secret", "GITHUB_TOKEN=ghp_pasted-assignment", "=", "") expected := map[string]string{ - "PATH": "/usr/bin", - "SAFE_VAR": "hello", - "AWS_PROFILE": "staging", + "PATH": "/usr/bin", + "SAFE_VAR": "hello", + "AWS_PROFILE": "staging", + "ZERO_OAUTH_CLIENT_SECRET": "not-a-provider-secret", } actual := make(map[string]string, len(scrubbed)) for _, entry := range scrubbed { @@ -674,3 +720,34 @@ func TestScrubSensitiveEnv(t *testing.T) { t.Errorf("scrubSensitiveEnv() = %v, want %v", actual, expected) } } + +func TestEngineScrubsConfiguredSensitiveEnvKeys(t *testing.T) { + workspace := t.TempDir() + engine := NewEngine(EngineOptions{ + WorkspaceRoot: workspace, + Policy: DefaultPolicy(), + Backend: Backend{Name: BackendLinuxBwrap, Available: true, Executable: "/usr/bin/zero-linux-sandbox"}, + SensitiveEnvKeys: []string{"COMPANY_LLM_SECRET"}, + }) + plan, err := engine.BuildCommandPlan(CommandSpec{ + Name: "true", + Env: []string{ + "PATH=/usr/bin", + "COMPANY_LLM_SECRET=custom-secret", + "ZERO_OAUTH_CUSTOM_CLIENT_SECRET=oauth-secret", + "SAFE_VAR=hello", + }, + }) + if err != nil { + t.Fatalf("BuildCommandPlan: %v", err) + } + for _, entry := range plan.Env { + key, _, _ := strings.Cut(entry, "=") + if strings.EqualFold(key, "COMPANY_LLM_SECRET") || strings.EqualFold(key, "ZERO_OAUTH_CUSTOM_CLIENT_SECRET") { + t.Fatalf("plan.Env retained sensitive key %q: %v", key, plan.Env) + } + } + if got := envListValue(plan.Env, "SAFE_VAR", ""); got != "hello" { + t.Fatalf("SAFE_VAR = %q, want hello", got) + } +} diff --git a/internal/sandbox/windows_runner.go b/internal/sandbox/windows_runner.go index 2b271f96..ac8292ca 100644 --- a/internal/sandbox/windows_runner.go +++ b/internal/sandbox/windows_runner.go @@ -326,7 +326,7 @@ func windowsRestrictedTokenCommandPlan(execRequest SandboxExecutionRequest, poli if err != nil { return CommandPlan{}, err } - childEnv := sandboxEnvironmentForCommand(spec.Env, policy, BackendWindowsRestrictedToken, execRequest.WorkspaceRoot) + childEnv := sandboxEnvironmentForCommandWithSensitiveEnv(spec.Env, policy, BackendWindowsRestrictedToken, execRequest.WorkspaceRoot, spec.sensitiveEnvKeys) // The unelevated enforcement tier maps to the runner's unelevated level: same // restricted token, but the runner applies the workspace ACLs itself instead // of requiring the elevated setup marker.