From e6b80d7af4663e972175c232e2272774a63fd5ad Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Tue, 14 Jul 2026 21:31:14 +0200 Subject: [PATCH 1/3] fix(sandbox): scrub dynamic credential env vars --- internal/cli/app.go | 11 ++--- internal/cli/exec.go | 24 ++++++++--- internal/cli/sandbox_sensitive_env_test.go | 23 +++++++++++ internal/sandbox/engine.go | 41 ++++++++++-------- internal/sandbox/runner.go | 48 +++++++++++++++++++--- internal/sandbox/runner_test.go | 44 ++++++++++++++++++-- internal/sandbox/windows_runner.go | 2 +- 7 files changed, 155 insertions(+), 38 deletions(-) create mode 100644 internal/cli/sandbox_sensitive_env_test.go diff --git a/internal/cli/app.go b/internal/cli/app.go index a138f0142..a6088c2f4 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 3d553dee7..85b36ad5a 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 000000000..28e3a2e52 --- /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 162bd96e9..ecb2f1cc0 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 4bcf83808..6a4b2465d 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 @@ -318,7 +322,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 +381,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 +392,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 +975,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 +1005,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 +1014,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 +1027,32 @@ 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) + 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 5e7a5b256..36aa436f4 100644 --- a/internal/sandbox/runner_test.go +++ b/internal/sandbox/runner_test.go @@ -653,14 +653,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", "") 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 +679,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 2b271f96d..ac8292ca2 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. From b0086c47c189dcfc22945014507b56d0a19f7fb0 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Tue, 14 Jul 2026 23:16:12 +0200 Subject: [PATCH 2/3] fix(sandbox): tolerate env assignments in configured sensitive keys A config value mistakenly given as a full assignment (e.g. "COMPANY_LLM_SECRET=...") would never match the real env key during scrubbing, silently re-exposing the credential to sandboxed commands. normalizeSensitiveEnvKeys now keeps only the name part before '=' and re-trims it, per PR review. Co-Authored-By: Claude Fable 5 --- internal/sandbox/runner.go | 5 +++++ internal/sandbox/runner_test.go | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/sandbox/runner.go b/internal/sandbox/runner.go index 6a4b2465d..814554aa3 100644 --- a/internal/sandbox/runner.go +++ b/internal/sandbox/runner.go @@ -1033,6 +1033,11 @@ func normalizeSensitiveEnvKeys(keys []string) []string { 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 diff --git a/internal/sandbox/runner_test.go b/internal/sandbox/runner_test.go index 36aa436f4..a0d63b07a 100644 --- a/internal/sandbox/runner_test.go +++ b/internal/sandbox/runner_test.go @@ -660,7 +660,7 @@ func TestScrubSensitiveEnv(t *testing.T) { "AWS_PROFILE=staging", "SAFE_VAR=hello", } - scrubbed := scrubSensitiveEnv(inputEnv, " COMPANY_LLM_SECRET ", "company_llm_secret", "") + scrubbed := scrubSensitiveEnv(inputEnv, " COMPANY_LLM_SECRET ", "company_llm_secret", "GITHUB_TOKEN=ghp_pasted-assignment", "=", "") expected := map[string]string{ "PATH": "/usr/bin", "SAFE_VAR": "hello", From 7b59fc83aec1b618c6d0c95bd9791c1f3e554bb9 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Wed, 15 Jul 2026 04:21:37 +0200 Subject: [PATCH 3/3] fix(sandbox): scrub credentials on direct/degraded command plans directCommandPlan (used when the native sandbox backend is unavailable, disabled, or not required, including the EnforcementDegraded fallback) left Env nil whenever spec.Env was nil, so exec.Cmd inherited the full caller environment unscrubbed. Configured apiKeyEnv credentials and dynamically named ZERO_OAUTH__CLIENT_SECRET variables leaked through that path even though the wrapped sandbox plans already scrubbed them. Route direct plans through scrubSensitiveEnv the same way wrapped plans do, snapshotting os.Environ() when spec.Env is nil so scrubbing still applies. Adds a regression test covering the degraded-fallback path with both a configured and a dynamically named secret. Co-Authored-By: Claude Sonnet 5 --- internal/sandbox/runner.go | 19 ++++++++++++++- internal/sandbox/runner_test.go | 41 +++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/internal/sandbox/runner.go b/internal/sandbox/runner.go index 814554aa3..506acb393 100644 --- a/internal/sandbox/runner.go +++ b/internal/sandbox/runner.go @@ -257,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 == "" { diff --git a/internal/sandbox/runner_test.go b/internal/sandbox/runner_test.go index a0d63b07a..67b66866c 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{