diff --git a/docs/site/reference/command-reference.md b/docs/site/reference/command-reference.md index ce55c052f..4c6d9776b 100644 --- a/docs/site/reference/command-reference.md +++ b/docs/site/reference/command-reference.md @@ -28,6 +28,8 @@ The task comes first and becomes the launch prompt. Anything after `--` forwards An unsandboxed bootstrap launch carries no safehouse isolation, so per-action permission prompting is friction without a matching safety gain: `spacedock claude` starts in `--permission-mode auto` and Codex starts in `--ask-for-approval on-request` unless you supply an approval mode. A sandboxed bootstrap launch instead skips/bypasses approvals (`--dangerously-skip-permissions` for claude, `--dangerously-bypass-approvals-and-sandbox` for codex) since the sandbox is the gate. Claude suppresses its defaults when you pass your own mode or a resume. Codex suppresses its banner and bootstrap prompt only when its forwarded argv contains the exact `resume` token; an explicit approval mode prevents only a duplicate automatic approval flag. +If `CLAUDE_CODE_SUBPROCESS_ENV_SCRUB` is set in your environment (Claude Code's own subprocess credential-scrubbing hardening), Claude Code forces the launched session's permission mode back to `default` unless you declare `--allowedTools` yourself — `spacedock claude` warns about this at launch, and `spacedock doctor --host claude` notes it too. Declare `--allowedTools` explicitly to keep both the hardening and a friction-free dispatch, or unset the var for the launch. + ## Setup | Command | What it does | diff --git a/docs/specs/env-scrub-permission-mode-warning.md b/docs/specs/env-scrub-permission-mode-warning.md new file mode 100644 index 000000000..c4f614eaf --- /dev/null +++ b/docs/specs/env-scrub-permission-mode-warning.md @@ -0,0 +1,50 @@ +# Flag the CLAUDE_CODE_SUBPROCESS_ENV_SCRUB Permission-Mode Incompatibility + +Status: approved design, ready for implementation. + +Fixes: https://github.com/spacedock-dev/spacedock/issues/504 + +## Problem + +Claude Code's subprocess credential-scrubbing hardening (`CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1`) forces a launched session's permission mode back to `default` unless the launcher declares `--allowedTools` explicitly. Spacedock launches Claude Code as a subprocess in both the unsandboxed (`--permission-mode auto`) and `--safehouse` (`--dangerously-skip-permissions`) forms, so a spacedock user who has this hardening set globally (a reasonable, recommended posture) gets a dispatched first officer silently downgraded to prompt-on-everything. The only warning today is Claude Code's own generic message, printed after spacedock's launch banner, with no acknowledgment from spacedock that this is a known incompatibility. + +Spacedock should not try to silently route around the hardening by guessing an `--allowedTools` allowlist — that is a security-relevant decision that belongs to the operator. Spacedock's job is to flag the incompatibility clearly enough that the operator understands what happened and what their options are. + +## Scope + +Claude-only. `CLAUDE_CODE_SUBPROCESS_ENV_SCRUB` is a Claude Code env var with no Codex or Pi analog. + +## Design + +### Detection helper + +A new helper in `internal/cli/frontdoor.go`, alongside the existing `hasEnv`/`withoutEnv` env helpers: + +```go +func subprocessEnvScrubActive(env []string) bool +``` + +Reports true when `CLAUDE_CODE_SUBPROCESS_ENV_SCRUB` is present in `env` and set to a truthy value (non-empty, not `"0"`). + +### 1. Launch-time warning (`spacedock claude`) + +In `runClaude`, before the launch banner: if `subprocessEnvScrubActive(os.Environ())` is true and the operator has not already declared `--allowedTools`/`--allowed-tools` in `fd.passthrough` (checked the same way `passthroughHasFlag` checks other flags), print a spacedock-attributed stderr warning. + +Fires on **both** the unsandboxed and `--safehouse` launch paths — we have no evidence `--dangerously-skip-permissions` is exempt from the hardening, so the warning errs toward showing rather than silently assuming safety. + +The warning names the mechanism and the two remedies Claude Code's own message points at: declare `--allowedTools` explicitly, or unset `CLAUDE_CODE_SUBPROCESS_ENV_SCRUB` for the launch. It is suppressed when the operator already declared `--allowedTools` themselves (the documented workaround), mirroring the existing "operator wins" suppression pattern used for `--permission-mode`. + +### 2. `spacedock doctor` check + +In `runDoctorWithPi`'s non-pi branch, when `host == "claude"`: after the existing manifest-compatibility report, print the same advisory note if `subprocessEnvScrubActive(env)` is true. This is informational only — it does not change doctor's exit code, which stays governed solely by manifest compatibility. It exists so the incompatibility surfaces during setup/CI, not only mid-launch. + +## Testing + +- `frontdoor_test.go`: table-driven cases on the warning — appears when the var is truthy and no `--allowedTools` given; suppressed when the var is unset, `"0"`, or `--allowedTools` is already present; appears on both the wrap and unwrap launch paths. +- Doctor test: the advisory note appears/is absent analogously, without disturbing the existing exit-code assertions for manifest verdicts. + +## Out of scope + +- Guessing or injecting an `--allowedTools` allowlist on the operator's behalf. +- Any change to Codex's launch path. +- Changing doctor's exit code based on this env var. diff --git a/internal/cli/frontdoor.go b/internal/cli/frontdoor.go index b7290072b..c66b29b31 100644 --- a/internal/cli/frontdoor.go +++ b/internal/cli/frontdoor.go @@ -85,13 +85,34 @@ func withAgentTeams(env []string) []string { } func hasEnv(env []string, key string) bool { + _, ok := envValueOf(env, key) + return ok +} + +// subprocessEnvScrubEnv is Claude Code's own subprocess credential-scrubbing +// hardening var. When set truthy, Claude Code forces the launched session's +// permission mode back to "default" unless --allowedTools is declared +// explicitly — see warnSubprocessEnvScrub (runClaude) and envScrubDoctorNote +// (spacedock doctor). +const subprocessEnvScrubEnv = "CLAUDE_CODE_SUBPROCESS_ENV_SCRUB" + +// subprocessEnvScrubActive reports whether subprocessEnvScrubEnv is present in +// env and set to a truthy value (non-empty, not "0"). +func subprocessEnvScrubActive(env []string) bool { + v, ok := envValueOf(env, subprocessEnvScrubEnv) + return ok && v != "" && v != "0" +} + +// envValueOf looks up key's value in an env slice ("KEY=value" entries) — +// the production env scanner used by hasEnv and subprocessEnvScrubActive. +func envValueOf(env []string, key string) (string, bool) { prefix := key + "=" for _, entry := range env { if strings.HasPrefix(entry, prefix) { - return true + return strings.TrimPrefix(entry, prefix), true } } - return false + return "", false } // launcherBinEnvPassFlags returns the `--env-pass SPACEDOCK_BIN` safehouse flags @@ -377,6 +398,7 @@ func runClaude(ctx context.Context, args []string, dir string, ops hostOps, look } } warnStrayPromptAfterDash(fd, "spacedock claude", stderr) + warnSubprocessEnvScrub(os.Environ(), fd.passthrough, "spacedock claude", stderr) wrap := safehouse.Present(dir) || fd.forceSafehouse || len(fd.safehouseFlags) > 0 resume := containsResume(fd.passthrough) @@ -443,6 +465,43 @@ func warnStrayPromptAfterDash(fd frontDoorArgs, name string, stderr io.Writer) { name, pos, name, pos) } +// warnSubprocessEnvScrub prints a spacedock-attributed advisory when Claude +// Code's subprocess credential-scrubbing hardening is active in the launching +// environment. Claude Code forces the launched session's permission mode back +// to "default" whenever subprocessEnvScrubEnv is set truthy and no +// --allowedTools is declared, regardless of what --permission-mode spacedock +// or the operator requests, and regardless of the wrap/!wrap launch posture — +// there is no evidence --dangerously-skip-permissions is exempt from the +// hardening, so this fires on both paths. Suppressed only when the operator +// already declared --allowedTools/--allowed-tools themselves (the documented +// workaround), mirroring the "operator wins" suppression passthroughHasFlag +// backs elsewhere in this file. +func warnSubprocessEnvScrub(env []string, passthrough []string, name string, w io.Writer) { + if !subprocessEnvScrubActive(env) { + return + } + if passthroughHasFlag(passthrough, "--allowedTools", "--allowed-tools") { + return + } + fmt.Fprintf(w, + "%s: warning: %s is set — Claude Code will force this session's permission mode back to \"default\" unless --allowedTools is declared explicitly. "+ + "Declare --allowedTools yourself (see `claude --help`), or unset %s for this launch to keep the requested permission mode.\n", + name, subprocessEnvScrubEnv, subprocessEnvScrubEnv) +} + +// envScrubDoctorNote returns the advisory spacedock doctor prints when Claude +// Code's subprocess credential-scrubbing hardening is active in the operator's +// environment, or "" when it is not. Informational only — it must never affect +// doctor's exit code, which stays governed solely by contract.ManifestVerdict. +func envScrubDoctorNote(env []string) string { + if !subprocessEnvScrubActive(env) { + return "" + } + return fmt.Sprintf( + "note: %s is set in this environment — `spacedock claude` launches will warn that Claude Code forces permission mode back to \"default\" unless --allowedTools is declared explicitly. See `spacedock claude --help`, or unset the var to avoid this.\n", + subprocessEnvScrubEnv) +} + // launchPrompt returns the inner-argv launch prompt: `base + " " + task` when the // operator fenced a task after `--`, otherwise the bare base prompt. Claude and // Codex suppress it on their respective resume forms. diff --git a/internal/cli/frontdoor_test.go b/internal/cli/frontdoor_test.go index 2e82bdc71..ff6d4960e 100644 --- a/internal/cli/frontdoor_test.go +++ b/internal/cli/frontdoor_test.go @@ -149,14 +149,29 @@ func executableFixture(t *testing.T) string { return real } -func envValue(env []string, key string) (string, bool) { - prefix := key + "=" - for _, entry := range env { - if strings.HasPrefix(entry, prefix) { - return strings.TrimPrefix(entry, prefix), true - } +// TestSubprocessEnvScrubActive covers the truthy/falsy parsing of +// CLAUDE_CODE_SUBPROCESS_ENV_SCRUB that gates the launch warning (Task 2) and +// the doctor note (Task 3). +func TestSubprocessEnvScrubActive(t *testing.T) { + cases := []struct { + name string + env []string + want bool + }{ + {"unset", nil, false}, + {"empty value", []string{"CLAUDE_CODE_SUBPROCESS_ENV_SCRUB="}, false}, + {"explicit zero", []string{"CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=0"}, false}, + {"set to 1", []string{"CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1"}, true}, + {"set to true", []string{"CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=true"}, true}, + {"unrelated env untouched", []string{"OTHER_VAR=1"}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := subprocessEnvScrubActive(tc.env); got != tc.want { + t.Fatalf("subprocessEnvScrubActive(%v) = %v, want %v", tc.env, got, tc.want) + } + }) } - return "", false } // TestClaudeFrontDoorLaunchesOnCompatible: on a compatible contract the front @@ -178,6 +193,79 @@ func TestClaudeFrontDoorLaunchesOnCompatible(t *testing.T) { } } +// TestClaudeFrontDoorWarnsOnSubprocessEnvScrub: when +// CLAUDE_CODE_SUBPROCESS_ENV_SCRUB is set truthy and the operator has not +// declared --allowedTools themselves, spacedock claude prints its own +// attributed warning — on both the unsandboxed and --safehouse launch paths, +// since --dangerously-skip-permissions is not known to be exempt from Claude +// Code's hardening. +func TestClaudeFrontDoorWarnsOnSubprocessEnvScrub(t *testing.T) { + cases := []struct { + name string + args []string // bare --safehouse forces the wrap path, matching TestClaudeForceSafehouseWrapsNoProfile + }{ + {"unsandboxed", []string{"--", "-p", "do the thing"}}, + {"safehouse", []string{"--safehouse"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + withVersion(t, testBinaryVersion) + t.Setenv(subprocessEnvScrubEnv, "1") + fake := &fakeHost{manifest: compatibleManifest(t)} + var stdout, stderr bytes.Buffer + + code := runClaude(context.Background(), tc.args, t.TempDir(), fake, lookFound, &stdout, &stderr) + + if code != 0 { + t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) + } + if !strings.Contains(stderr.String(), "CLAUDE_CODE_SUBPROCESS_ENV_SCRUB") { + t.Fatalf("stderr missing env-scrub warning: %q", stderr.String()) + } + if !strings.Contains(stderr.String(), "--allowedTools") { + t.Fatalf("stderr missing the --allowedTools remedy: %q", stderr.String()) + } + }) + } +} + +// TestClaudeFrontDoorSuppressesSubprocessEnvScrubWarning covers the two +// suppression cases: the var isn't truthy, or the operator already declared +// --allowedTools themselves (the documented workaround). +func TestClaudeFrontDoorSuppressesSubprocessEnvScrubWarning(t *testing.T) { + cases := []struct { + name string + env string // value to set subprocessEnvScrubEnv to ("" means leave unset) + args []string + }{ + {"var unset", "", []string{"--", "-p", "do the thing"}}, + {"var explicit zero", "0", []string{"--", "-p", "do the thing"}}, + {"operator declared allowedTools", "1", []string{"--", "--allowedTools", "Bash(git *)", "-p", "do the thing"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + withVersion(t, testBinaryVersion) + if tc.env == "" { + t.Setenv(subprocessEnvScrubEnv, "") // register restore-on-cleanup, then unset for real + os.Unsetenv(subprocessEnvScrubEnv) + } else { + t.Setenv(subprocessEnvScrubEnv, tc.env) + } + fake := &fakeHost{manifest: compatibleManifest(t)} + var stdout, stderr bytes.Buffer + + code := runClaude(context.Background(), tc.args, t.TempDir(), fake, lookFound, &stdout, &stderr) + + if code != 0 { + t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) + } + if strings.Contains(stderr.String(), "CLAUDE_CODE_SUBPROCESS_ENV_SCRUB") { + t.Fatalf("stderr should not carry the env-scrub warning: %q", stderr.String()) + } + }) + } +} + // TestFrontDoorUpgradeHintOnBehindPlugin is AC-4: the front-door gate prints the // opt-in upgrade hint to stderr when the resolved plugin is contract-compatible // but behind the binary, then proceeds to launch (the hint never blocks). The @@ -258,7 +346,7 @@ func TestClaudeFrontDoorInjectsResolvedLauncherBin(t *testing.T) { if code != 0 { t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) } - got, ok := envValue(fake.launchedEnv, spacedockBinEnv) + got, ok := envValueOf(fake.launchedEnv, spacedockBinEnv) if !ok || got != bin { t.Fatalf("%s in launch env = %q, %v; want %q, true (env=%v)", spacedockBinEnv, got, ok, bin, fake.launchedEnv) } @@ -276,7 +364,7 @@ func TestClaudeFrontDoorOmitsStaleLauncherBinWhenResolutionFails(t *testing.T) { if code != 0 { t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) } - if got, ok := envValue(fake.launchedEnv, spacedockBinEnv); ok { + if got, ok := envValueOf(fake.launchedEnv, spacedockBinEnv); ok { t.Fatalf("%s in launch env = %q, want omitted", spacedockBinEnv, got) } } @@ -297,7 +385,7 @@ func TestClaudeFrontDoorLaunchEnvResolvesSymlink(t *testing.T) { if code != 0 { t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) } - if got, ok := envValue(fake.launchedEnv, spacedockBinEnv); !ok || got != real { + if got, ok := envValueOf(fake.launchedEnv, spacedockBinEnv); !ok || got != real { t.Fatalf("%s = %q, %v; want symlink target %q, true", spacedockBinEnv, got, ok, real) } } @@ -318,7 +406,7 @@ func TestClaudeFrontDoorEnablesAgentTeamsWhenParentUnset(t *testing.T) { if code != 0 { t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) } - got, ok := envValue(fake.launchedEnv, agentTeamsEnv) + got, ok := envValueOf(fake.launchedEnv, agentTeamsEnv) if !ok || got != "1" { t.Fatalf("%s in launch env = %q, %v; want %q, true (env=%v)", agentTeamsEnv, got, ok, "1", fake.launchedEnv) } @@ -338,7 +426,7 @@ func TestClaudeFrontDoorPreservesExplicitAgentTeams(t *testing.T) { if code != 0 { t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) } - got, ok := envValue(fake.launchedEnv, agentTeamsEnv) + got, ok := envValueOf(fake.launchedEnv, agentTeamsEnv) if !ok || got != "0" { t.Fatalf("%s in launch env = %q, %v; want %q, true (env=%v)", agentTeamsEnv, got, ok, "0", fake.launchedEnv) } @@ -845,7 +933,7 @@ func TestCodexFrontDoorInjectsLauncherBinThroughSafehouseResume(t *testing.T) { if !equalArgv(fake.launchedArg, wantArgv) { t.Fatalf("launch argv = %v, want %v", fake.launchedArg, wantArgv) } - got, ok := envValue(fake.launchedEnv, spacedockBinEnv) + got, ok := envValueOf(fake.launchedEnv, spacedockBinEnv) if !ok || got != bin { t.Fatalf("%s in launch env = %q, %v; want %q, true (env=%v)", spacedockBinEnv, got, ok, bin, fake.launchedEnv) } @@ -912,7 +1000,7 @@ func TestCodexFrontDoorDoesNotEnableAgentTeams(t *testing.T) { if code != 0 { t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) } - if got, ok := envValue(fake.launchedEnv, agentTeamsEnv); ok { + if got, ok := envValueOf(fake.launchedEnv, agentTeamsEnv); ok { t.Fatalf("%s in codex launch env = %q, want omitted", agentTeamsEnv, got) } } diff --git a/internal/cli/pi.go b/internal/cli/pi.go index 36df1ef86..5acd62bbf 100644 --- a/internal/cli/pi.go +++ b/internal/cli/pi.go @@ -392,7 +392,13 @@ func runDoctorWithPi(ctx context.Context, args []string, hostOps hostOps, piOps return code } if host != "pi" { - return runDoctor(ctx, args, hostOps, stdout, stderr) + doctorCode := runDoctor(ctx, args, hostOps, stdout, stderr) + if host == "claude" { + if note := envScrubDoctorNote(env); note != "" { + fmt.Fprint(stdout, note) + } + } + return doctorCode } cfg := piRuntimeConfigFromEnv(env, cwd(), pluginDir) check := checkPiRuntime(piOps, cfg) diff --git a/internal/cli/pi_frontdoor_test.go b/internal/cli/pi_frontdoor_test.go index 6c6242b8f..cf40c34a3 100644 --- a/internal/cli/pi_frontdoor_test.go +++ b/internal/cli/pi_frontdoor_test.go @@ -383,6 +383,54 @@ func TestNonPiSetupRejectsPluginDir(t *testing.T) { } } +// TestDoctorNotesSubprocessEnvScrubForClaude: `spacedock doctor --host claude` +// prints the env-scrub advisory when CLAUDE_CODE_SUBPROCESS_ENV_SCRUB is set +// truthy in the operator's environment — informational only, so the exit code +// stays governed by the manifest verdict (compatible here). +func TestDoctorNotesSubprocessEnvScrubForClaude(t *testing.T) { + ops := &fakeHost{manifest: compatibleManifest(t)} + var stdout, stderr bytes.Buffer + env := []string{"CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1"} + + code := runDoctorWithPi(context.Background(), []string{"--host", "claude"}, ops, &fakePiRuntimeOps{}, env, &stdout, &stderr) + + if code != 0 { + t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "CLAUDE_CODE_SUBPROCESS_ENV_SCRUB") { + t.Fatalf("stdout missing env-scrub doctor note: %q", stdout.String()) + } +} + +// TestDoctorOmitsSubprocessEnvScrubNoteWhenInactiveOrNonClaude covers: the var +// unset, and the var set but the host is codex (no Claude Code analog) or pi +// (a different runtime doctor path entirely). +func TestDoctorOmitsSubprocessEnvScrubNoteWhenInactiveOrNonClaude(t *testing.T) { + cases := []struct { + name string + host string + env []string + }{ + {"claude, var unset", "claude", nil}, + {"codex, var set", "codex", []string{"CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ops := &fakeHost{manifest: compatibleManifest(t)} + var stdout, stderr bytes.Buffer + + code := runDoctorWithPi(context.Background(), []string{"--host", tc.host}, ops, &fakePiRuntimeOps{}, tc.env, &stdout, &stderr) + + if code != 0 { + t.Fatalf("exit = %d, want 0 (stderr=%q)", code, stderr.String()) + } + if strings.Contains(stdout.String(), "CLAUDE_CODE_SUBPROCESS_ENV_SCRUB") { + t.Fatalf("stdout should not carry the env-scrub doctor note: %q", stdout.String()) + } + }) + } +} + func TestPiInstallCheckFailsForMissingSupervisorTalkbackPrerequisites(t *testing.T) { repo := t.TempDir() writePiSkillFixtures(t, repo)