diff --git a/README.md b/README.md index 9b7dfd20..93491c25 100644 --- a/README.md +++ b/README.md @@ -370,7 +370,10 @@ the [security policy](SECURITY.md). - `--model claude` or `--model codex` lets another coding agent drive the run. Its own tools are denied, and it can call only the tools Flynn bridges to it, so every action is still checked and recorded. This works on Linux, macOS and WSL2. Native Windows - refuses it, since Flynn cannot yet gate the child's network there. + refuses it, since Flynn cannot yet gate the child's network there. Flynn runs `claude` + and `codex` against homes of their own, kept in Flynn's data directory and separate from + yours. The first run prints the command that signs the CLI in with its own login; + Flynn never reads or copies the credential. ```sh flynn --model claude # interactive session through the CLI diff --git a/cmd/flynn/claudehome_test.go b/cmd/flynn/claudehome_test.go new file mode 100644 index 00000000..1dc9f464 --- /dev/null +++ b/cmd/flynn/claudehome_test.go @@ -0,0 +1,24 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + "testing" +) + +// TestMain points the claude and codex backends' homes at temporary directories for the +// whole package. Building either spawner creates its home, and without this every test that +// assembles one would create it under the real user's data directory. +func TestMain(m *testing.M) { + dir, err := os.MkdirTemp("", "flynn-agent-homes-") + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + claudeHome = func() string { return filepath.Join(dir, "claude") } + codexHome = func() string { return filepath.Join(dir, "codex") } + code := m.Run() + _ = os.RemoveAll(dir) + os.Exit(code) +} diff --git a/cmd/flynn/externagent.go b/cmd/flynn/externagent.go index c9519b84..5b9cb29d 100644 --- a/cmd/flynn/externagent.go +++ b/cmd/flynn/externagent.go @@ -7,6 +7,7 @@ import ( "io" "os" "path/filepath" + "runtime" "sort" "strings" "time" @@ -130,37 +131,82 @@ var codexAllowedHosts = []string{ ".chatgpt.com", } -// codexAuthDir is the codex CLI's credential and config home, granted read to the -// confined child so an auth-status probe and a live episode can see the subscription -// token without it being copied into the workspace. CODEX_HOME overrides the default -// (~/.codex), matching the CLI's own resolution. Empty when no home directory is -// resolvable, which grants no extra read. -func codexAuthDir() string { - if v := os.Getenv("CODEX_HOME"); v != "" { - return v +// codexHome is the codex home Flynn keeps for the codex backend, apart from the user's own +// (~/.codex or their CODEX_HOME). The user signs the CLI in to it once with the CLI's own +// login ([codexLoginCommand]), and every probe and episode points the CLI at it through +// CODEX_HOME. Flynn never reads, copies or stores the token inside: the CLI writes it at +// sign-in and refreshes it in place. +// +// Keeping it apart from the user's home is the codex half of the HostConfig posture the +// adapter declares. The user's config.toml, where they keep their MCP servers, model +// providers and settings, is never on the child's path, so whatever the person who +// launched the run happens to carry cannot steer an episode or hand the harness servers +// this run neither hosts nor governs. Anything but the sign-in found in Flynn's own home is removed +// before a spawner is built ([clearOwnHome]), because nothing Flynn does writes one +// there. A variable so tests can point it at a temporary directory. +var codexHome = defaultCodexHome + +// defaultCodexHome is where [codexHome] points outside tests. +func defaultCodexHome() string { return filepath.Join(defaultDataDir(), "codex") } + +// codexLoginCommand is the command that signs the codex CLI in to home with its own +// subscription login. +func codexLoginCommand(home string) string { + return homeLoginCommand("CODEX_HOME", home, "codex login") +} + +// codexAuthFiles and claudeAuthFiles are the entries of Flynn's own CLI homes that +// [clearOwnHome] keeps: the sign-in each CLI writes with its file credential store. +// Everything else a CLI keeps there is rebuilt when it needs it, and is also what a harness +// would write to steer a later run: config.toml, AGENTS.md, prompts and skills for codex; +// settings.json (hooks, permission allow rules), CLAUDE.md, agents, commands, plugins and +// .claude.json (user-scoped MCP servers and trust decisions) for claude. Claude's account +// details also live in .claude.json, but the token is in .credentials.json, so losing it +// costs the CLI a re-read of the account and nothing else. +var ( + codexAuthFiles = map[string]bool{"auth.json": true} + claudeAuthFiles = map[string]bool{".credentials.json": true} +) + +// clearOwnHome removes everything from one of Flynn's CLI homes except the entries in keep, +// so an episode's configuration and instructions are what its command line passes and +// nothing else. An episode's child may write the home it runs in, so without this a harness +// could leave settings, instructions or plugins there that steer every later run. A sign-in +// held in the OS keyring is outside the home and untouched. A home that does not exist has +// nothing to clear. +func clearOwnHome(label, home string, keep map[string]bool) error { + entries, err := os.ReadDir(home) + if errors.Is(err, os.ErrNotExist) { + return nil } - home, err := os.UserHomeDir() if err != nil { - return "" + return fmt.Errorf("%s home: %w", label, err) + } + for _, e := range entries { + if keep[e.Name()] { + continue + } + if err := os.RemoveAll(filepath.Join(home, e.Name())); err != nil { + return fmt.Errorf("%s home: %w", label, err) + } } - return filepath.Join(home, ".codex") + return nil } -// codexAuthSeedFiles are the only files an episode copies out of the codex home into the -// per-episode credential home the confined child writes: the OAuth token it authenticates -// with, and nothing else. Everything else in the home (session rollouts, logs, caches) is -// state the CLI regenerates, so it is left behind rather than handed to an untrusted -// harness. See externagent.SandboxConfig.AuthSeedFiles for why an episode gets a copy -// instead of the real directory. -// -// The operator's own config.toml is deliberately not among them, which is the codex half -// of the HostConfig posture the adapter declares. That file is where a codex user keeps -// their MCP servers, model providers and settings, so seeding it would let whatever the -// person who launched the run happens to carry steer the episode, and would give the -// harness servers this run neither hosts nor governs. Everything an episode needs is -// passed on the command line instead, so the child's configuration is a function of the -// run. -var codexAuthSeedFiles = []string{"auth.json"} +// ensureOwnHome creates a CLI home Flynn keeps and sets it to owner-only. MkdirAll leaves +// an existing directory with the mode it already had, and the home holds a bearer token, +// so the mode is set explicitly. On Windows Chmod changes only the read-only attribute and +// the directory's access comes from its ACL, which the per-user data directory sets. +func ensureOwnHome(label, home string) error { + if err := os.MkdirAll(home, 0o700); err != nil { + return fmt.Errorf("%s home: %w", label, err) + } + //nolint:gosec // G302: 0700 is the owner-only mode for a directory; gosec's 0600 ceiling is for files + if err := os.Chmod(home, 0o700); err != nil { + return fmt.Errorf("%s home: %w", label, err) + } + return nil +} // claudeAllowedHosts are the destination names a claude episode's confined child may // reach at the egress waist: the Anthropic API the subscription drives inference over and @@ -176,34 +222,62 @@ var claudeAllowedHosts = []string{ ".claude.ai", } -// claudeSeedPaths are the files the confined child needs to authenticate on the owner's -// subscription, gathered into one credential home. Claude Code splits them: its config -// lives in the home directory (.claude.json) and its OAuth token in a subdirectory -// (.claude/.credentials.json), and pointing the CLI at a single CLAUDE_CONFIG_DIR makes it -// look for both, flat, under that directory. So the two are seeded by base name into one -// per-episode home the run writes and deletes; nothing else from the home (history, caches, -// project state) is copied, so the operator's hooks, plugins and settings are not on the -// child's path to find. The config file is seeded because the CLI reads its onboarding and -// account state from it and reports itself unusable without it; the MCP servers it also -// carries are neutralized by --strict-mcp-config, which is what lets the adapter declare the -// operator's configuration denied. CLAUDE_CONFIG_DIR overrides where both files live, matching -// the CLI's own resolution. Empty when no home directory is resolvable, which seeds nothing and -// leaves detection to report the CLI as logged out. -func claudeSeedPaths() []string { - if v := os.Getenv("CLAUDE_CONFIG_DIR"); v != "" { - return []string{ - filepath.Join(v, ".claude.json"), - filepath.Join(v, ".credentials.json"), - } +// claudeHome is the Claude Code home Flynn keeps for the claude backend, apart from the +// user's own. The user signs the CLI in to it once with the CLI's own login +// ([claudeLoginCommand]), and every probe and episode points the CLI at it through +// CLAUDE_CONFIG_DIR. Flynn never reads, copies or stores the token inside: the CLI writes it +// at sign-in and refreshes it in place, and the sign-in completes in the CLI's own flow. +// +// Keeping it apart from the user's home means none of the user's own settings, hooks, +// plugins or MCP servers are in it to steer an episode, which is what makes it acceptable +// to grant an episode write on it ([externagent.SandboxConfig.AuthDirWritable]). The grant +// still lets a harness sign Flynn's copy out or leave settings, CLAUDE.md, agents, commands +// or plugins behind, so everything but the sign-in is removed before a spawner is built +// ([clearOwnHome]). It sits under the per-user data +// directory, so a development build and a release build keep separate sign-ins. A variable +// so tests can point it at a temporary directory. +var claudeHome = defaultClaudeHome + +// defaultClaudeHome is where [claudeHome] points outside tests. +func defaultClaudeHome() string { return filepath.Join(defaultDataDir(), "claude") } + +// claudeLoginCommand is the command that signs the claude CLI in to home with its own +// subscription login. +func claudeLoginCommand(home string) string { + return homeLoginCommand("CLAUDE_CONFIG_DIR", home, "claude auth login --claudeai") +} + +// homeLoginCommand is login run with env pointing the CLI at home, written for the shell +// the user is most likely in. The PowerShell form clears the variable afterwards, so the +// user's next plain run of the CLI in that shell is back on their own home. +func homeLoginCommand(env, home, login string) string { + if runtime.GOOS == "windows" { + return "$env:" + env + " = '" + strings.ReplaceAll(home, "'", "''") + + "'; " + login + "; Remove-Item Env:" + env } - home, err := os.UserHomeDir() - if err != nil { - return nil + return env + "='" + strings.ReplaceAll(home, "'", `'\''`) + "' " + login +} + +// withHomeLogin rewrites a not-signed-in readiness for an external backend so its next step +// signs in the home Flynn keeps for that CLI rather than the user's: the adapter's generic +// advice (`claude auth login`, `codex login`) would sign in the home Flynn does not use and +// leave the backend reporting itself logged out. A refusal, a missing install, and a ready +// CLI pass through. An unknown backend passes through too. +func withHomeLogin(name string, r externagent.Readiness) externagent.Readiness { + if !r.Available || r.Refuse || r.LoggedIn { + return r } - return []string{ - filepath.Join(home, ".claude.json"), - filepath.Join(home, ".claude", ".credentials.json"), + switch name { + case "claude": + home := claudeHome() + r.Reason = "claude is not signed in to a Claude subscription for Flynn. Flynn keeps its own Claude Code sign-in, separate from yours, at " + + home + ". Sign in to it once with Claude Code's own login: " + claudeLoginCommand(home) + case "codex": + home := codexHome() + r.Reason = "codex is not signed in for Flynn. Flynn keeps its own Codex sign-in, separate from yours, at " + + home + ". Sign in to it once with Codex's own login: " + codexLoginCommand(home) } + return r } // externalProbeTimeout caps a single detection probe (a version or auth-status check) @@ -226,43 +300,79 @@ func externalSpawner(name string) (externagent.Spawner, error) { if err != nil && !errors.Is(err, externagent.ErrProgramNotFound) { return nil, err } - return externagent.NewSandboxSpawner(externagent.SandboxConfig{ - AllowedHosts: codexAllowedHosts, - AuthDir: codexAuthDir(), - AuthEnv: "CODEX_HOME", - AuthSeedFiles: codexAuthSeedFiles, - ProgramDirs: prog.ReadableDirs, - ProbeTimeout: externalProbeTimeout, - // The codex CLI is a Rust program: it canonicalizes paths on startup, which no - // process can do inside a Windows AppContainer, so it runs under the tier that - // confines its writes to the workspace and leaves the host readable. Its writes, - // its egress, and its tools stay gated; only the read posture is traded, and the - // episode records the tier that ran. - HostReadable: true, - }), nil + // The CLI refuses a CODEX_HOME that does not exist, and the user's first sign-in runs + // against the path Flynn prints, so the home is created before anything points at it. + home := codexHome() + if err := ensureOwnHome("codex", home); err != nil { + return nil, err + } + if err := clearOwnHome("codex", home, codexAuthFiles); err != nil { + return nil, err + } + return externagent.NewSandboxSpawner(codexSandboxConfig(home, prog)), nil case "claude": prog, err := externagent.LocateClaude("") if err != nil && !errors.Is(err, externagent.ErrProgramNotFound) { return nil, err } - return externagent.NewSandboxSpawner(externagent.SandboxConfig{ - AllowedHosts: claudeAllowedHosts, - AuthEnv: "CLAUDE_CONFIG_DIR", - AuthSeedPaths: claudeSeedPaths(), - ProgramDirs: prog.ReadableDirs, - ProbeTimeout: externalProbeTimeout, - // Claude Code ships as a single compiled native binary that canonicalizes paths on - // startup, the same operation no process can perform inside a Windows AppContainer, - // so it runs under the tier that confines its writes to the workspace and leaves the - // host readable. Its writes, its egress, and its tools stay gated; only the read - // posture is traded, and the episode records the tier that ran. - HostReadable: true, - }), nil + // The CLI would create its home at sign-in, but detection grants the child read on it + // before then, and a grant on a directory that does not exist fails the probe that + // reports how to sign in. + home := claudeHome() + if err := ensureOwnHome("claude", home); err != nil { + return nil, err + } + if err := clearOwnHome("claude", home, claudeAuthFiles); err != nil { + return nil, err + } + return externagent.NewSandboxSpawner(claudeSandboxConfig(home, prog)), nil default: return nil, fmt.Errorf("unknown external agent backend %q", name) } } +// codexSandboxConfig is the confinement envelope for the codex backend over its own home. The +// home is granted write ([externagent.SandboxConfig.AuthDirWritable]) because the CLI +// refreshes its token in place there. That grant is safe only because the home is Flynn's +// own and holds nothing of the user's, so a test holds it to true here. +func codexSandboxConfig(home string, prog externagent.Program) externagent.SandboxConfig { + return externagent.SandboxConfig{ + AllowedHosts: codexAllowedHosts, + AuthDir: home, + AuthEnv: "CODEX_HOME", + AuthDirWritable: true, + ProgramDirs: prog.ReadableDirs, + ProbeTimeout: externalProbeTimeout, + // The codex CLI is a Rust program: it canonicalizes paths on startup, which no + // process can do inside a Windows AppContainer, so it runs under the tier that + // confines its writes to the workspace and leaves the host readable. Its writes, + // its egress, and its tools stay gated; only the read posture is traded, and the + // episode records the tier that ran. + HostReadable: true, + } +} + +// claudeSandboxConfig is the confinement envelope for the claude backend over its own home. The +// home is granted write ([externagent.SandboxConfig.AuthDirWritable]) because the CLI +// refreshes its token in place there. That grant is safe only because the home is Flynn's +// own and holds nothing of the user's, so a test holds it to true here. +func claudeSandboxConfig(home string, prog externagent.Program) externagent.SandboxConfig { + return externagent.SandboxConfig{ + AllowedHosts: claudeAllowedHosts, + AuthDir: home, + AuthEnv: "CLAUDE_CONFIG_DIR", + AuthDirWritable: true, + ProgramDirs: prog.ReadableDirs, + ProbeTimeout: externalProbeTimeout, + // Claude Code ships as a single compiled native binary that canonicalizes paths on + // startup, the same operation no process can perform inside a Windows AppContainer, + // so it runs under the tier that confines its writes to the workspace and leaves the + // host readable. Its writes, its egress, and its tools stay gated; only the read + // posture is traded, and the episode records the tier that ran. + HostReadable: true, + } +} + // externalAdapter builds the CLI adapter for the named backend over spawner. The // adapter describes how to detect the CLI and drive one episode; the codex adapter is // the first. @@ -337,6 +447,7 @@ func resolveExternalAgent(ctx context.Context, name, model, workdir string) (*ex if err != nil { return nil, fmt.Errorf("detect %s: %w", name, err) } + r = withHomeLogin(name, r) if rerr := readinessError(name, r); rerr != nil { return nil, rerr } diff --git a/cmd/flynn/externagent_provenance_test.go b/cmd/flynn/externagent_provenance_test.go index 5acbbcc9..0658db84 100644 --- a/cmd/flynn/externagent_provenance_test.go +++ b/cmd/flynn/externagent_provenance_test.go @@ -3,7 +3,9 @@ package main import ( "context" "errors" + "os" "path/filepath" + "runtime" "strings" "testing" @@ -178,67 +180,125 @@ func TestExternalAdapterIsBuiltForEveryBackend(t *testing.T) { } } -// TestCodexAuthDirFollowsTheCLIsOwnResolution checks the credential home granted to the -// confined child: CODEX_HOME wins, and the default is the CLI's own home directory. -func TestCodexAuthDirFollowsTheCLIsOwnResolution(t *testing.T) { - home := t.TempDir() - t.Setenv("CODEX_HOME", home) - if got := codexAuthDir(); got != home { - t.Errorf("codexAuthDir = %q, want the CODEX_HOME override", got) - } +// TestCodexHomeIsFlynnsOwnAndNeverTheUsers checks the codex backend runs the CLI against the +// home Flynn keeps for it, whatever the user's own CODEX_HOME says, that the home exists once +// the spawner is built (the CLI refuses a CODEX_HOME that does not), and that a config.toml, +// AGENTS.md or skills directory left in it is gone before any episode can read it while the sign-in beside it is kept. +func TestCodexHomeIsFlynnsOwnAndNeverTheUsers(t *testing.T) { + withNoExecutables(t) + home := filepath.Join(t.TempDir(), "codex") + prev := codexHome + codexHome = func() string { return home } + t.Cleanup(func() { codexHome = prev }) + userDir := t.TempDir() + t.Setenv("CODEX_HOME", userDir) - t.Setenv("CODEX_HOME", "") - got := codexAuthDir() - if got != "" && filepath.Base(got) != ".codex" { - t.Errorf("codexAuthDir = %q, want the CLI's default home", got) + if _, err := externalSpawner("codex"); err != nil { + t.Fatalf("externalSpawner(codex): %v", err) } -} - -// TestClaudeSeedPathsGatherBothCredentialFiles checks the two files the confined child needs -// to authenticate are seeded by base name into one home: with the config dir overridden they -// are both taken from it, flat, matching where the CLI looks for them. -func TestClaudeSeedPathsGatherBothCredentialFiles(t *testing.T) { - dir := t.TempDir() - t.Setenv("CLAUDE_CONFIG_DIR", dir) - got := claudeSeedPaths() - want := []string{filepath.Join(dir, ".claude.json"), filepath.Join(dir, ".credentials.json")} - if len(got) != len(want) { - t.Fatalf("claudeSeedPaths = %v, want both credential files", got) - } - for i := range want { - if got[i] != want[i] { - t.Errorf("seed path %d = %q, want %q", i, got[i], want[i]) + if fi, err := os.Stat(home); err != nil || !fi.IsDir() { + t.Fatalf("the codex home must exist after the spawner is built: %v", err) + } + for name, body := range map[string]string{"config.toml": "[mcp_servers.x]\n", "auth.json": "{}"} { + if err := os.WriteFile(filepath.Join(home, name), []byte(body), 0o600); err != nil { + t.Fatal(err) } } - - t.Setenv("CLAUDE_CONFIG_DIR", "") - def := claudeSeedPaths() - if len(def) != 2 { - t.Fatalf("the default must still seed both files, got %v", def) + if _, err := externalSpawner("codex"); err != nil { + t.Fatalf("externalSpawner(codex): %v", err) + } + for _, name := range []string{"config.toml", "AGENTS.md", "skills"} { + if _, err := os.Stat(filepath.Join(home, name)); !errors.Is(err, os.ErrNotExist) { + t.Errorf("%s in Flynn's codex home must be removed before an episode, got %v", name, err) + } } - if filepath.Base(def[0]) != ".claude.json" || filepath.Base(def[1]) != ".credentials.json" { - t.Errorf("the default seed paths name the wrong files: %v", def) + if _, err := os.Stat(filepath.Join(home, "auth.json")); err != nil { + t.Errorf("the sign-in in Flynn's codex home must be left alone: %v", err) + } + def := defaultCodexHome() + if filepath.Dir(def) != defaultDataDir() { + t.Errorf("the default codex home %q must sit under the data directory", def) + } + if def == userDir { + t.Errorf("the codex home must not follow the user's CODEX_HOME, got %q", def) } } -// TestCodexSeedCarriesTheCredentialAndNoOperatorConfig is the host half of the codex -// adapter's declared host-config posture. The adapter states that an episode's child runs -// under a per-episode home holding the credential and nothing else; the seed list is where -// that is either true or quietly false. config.toml is the file that would break it: it is -// where a codex user keeps their MCP servers, model providers and settings, so seeding it -// would hand the harness servers this run neither hosts nor governs and let the launching -// operator's configuration steer the episode. -func TestCodexSeedCarriesTheCredentialAndNoOperatorConfig(t *testing.T) { - if len(codexAuthSeedFiles) != 1 || codexAuthSeedFiles[0] != "auth.json" { - t.Fatalf("codexAuthSeedFiles = %v, want only the credential", codexAuthSeedFiles) - } - for _, f := range codexAuthSeedFiles { - if f == "config.toml" { - t.Error("the operator's codex config must not be seeded into an episode's home") - } +// TestClaudeHomeIsFlynnsOwnAndNeverTheUsers checks the claude backend runs the CLI against +// the home Flynn keeps for it, whatever the user's own CLAUDE_CONFIG_DIR says, and that the +// home exists once the spawner is built so detection can be granted read on it before the +// user has signed in. +func TestClaudeHomeIsFlynnsOwnAndNeverTheUsers(t *testing.T) { + withNoExecutables(t) + home := filepath.Join(t.TempDir(), "claude") + prev := claudeHome + claudeHome = func() string { return home } + t.Cleanup(func() { claudeHome = prev }) + userDir := t.TempDir() + t.Setenv("CLAUDE_CONFIG_DIR", userDir) + + if _, err := externalSpawner("claude"); err != nil { + t.Fatalf("externalSpawner(claude): %v", err) } - if got := externagent.NewCodex("codex", nil).Lockdown().HostConfig; !got.Stripped() { - t.Errorf("codex host-config posture = %v, want a stripped one to match the seed", got) + if fi, err := os.Stat(home); err != nil || !fi.IsDir() { + t.Fatalf("the claude home must exist after the spawner is built: %v", err) + } + def := defaultClaudeHome() + if filepath.Dir(def) != defaultDataDir() { + t.Errorf("the default claude home %q must sit under the data directory", def) + } + if def == userDir { + t.Errorf("the claude home must not follow the user's CLAUDE_CONFIG_DIR, got %q", def) + } +} + +// TestLoginSignsInFlynnsHome checks a not-signed-in claude or codex is pointed at a login +// that signs in Flynn's home for that CLI through the CLI's own subscription flow, and that +// the other readiness states pass through untouched. +func TestLoginSignsInFlynnsHome(t *testing.T) { + for _, tc := range []struct { + name string + home *func() string + env string + login string + }{ + {"claude", &claudeHome, "CLAUDE_CONFIG_DIR", "claude auth login --claudeai"}, + {"codex", &codexHome, "CODEX_HOME", "codex login"}, + } { + t.Run(tc.name, func(t *testing.T) { + home := filepath.Join(t.TempDir(), "it's here") + prev := *tc.home + *tc.home = func() string { return home } + t.Cleanup(func() { *tc.home = prev }) + + out := withHomeLogin(tc.name, externagent.Readiness{Available: true, Reason: "run the CLI's login"}) + for _, want := range []string{tc.env, tc.login, "separate from yours"} { + if !strings.Contains(out.Reason, want) { + t.Errorf("login reason %q lacks %q", out.Reason, want) + } + } + if strings.Contains(strings.ToLower(out.Reason), "api key") { + t.Errorf("the login must never ask for an API key, got %q", out.Reason) + } + cmd := homeLoginCommand(tc.env, home, tc.login) + if runtime.GOOS == "windows" { + if !strings.Contains(cmd, "it''s here") || !strings.HasSuffix(cmd, "Remove-Item Env:"+tc.env) { + t.Errorf("PowerShell login must quote the path and clear the variable, got %q", cmd) + } + } else if !strings.Contains(cmd, `it'\''s here`) { + t.Errorf("sh login must quote the path, got %q", cmd) + } + + for _, r := range []externagent.Readiness{ + {Available: false, Reason: "install it"}, + {Available: true, Refuse: true, Reason: "update it"}, + {Available: true, LoggedIn: true}, + } { + if got := withHomeLogin(tc.name, r); got != r { + t.Errorf("withHomeLogin(%+v) = %+v, want it unchanged", r, got) + } + } + }) } } @@ -251,7 +311,6 @@ func TestResolveExternalAgentReportsAMissingCLI(t *testing.T) { // simply not installed. t.Setenv("PATH", t.TempDir()) t.Setenv("CODEX_HOME", t.TempDir()) - t.Setenv("CLAUDE_CONFIG_DIR", t.TempDir()) for _, name := range externalAgentNames() { ea, err := resolveExternalAgent(context.Background(), name, "", t.TempDir()) @@ -273,3 +332,188 @@ func TestResolveExternalAgentReportsAMissingCLI(t *testing.T) { } } } + +// TestClearOwnHomeReportsAnUnreadableHome checks a home that cannot be listed for a reason +// other than being absent is an error rather than a silent pass, since a config left in it +// would then steer the next episode; an absent home has nothing to clear. +func TestClearOwnHomeReportsAnUnreadableHome(t *testing.T) { + if err := clearOwnHome("codex", filepath.Join(t.TempDir(), "absent"), codexAuthFiles); err != nil { + t.Errorf("an absent home has nothing to clear, got %v", err) + } + if runtime.GOOS == "windows" { + t.Skip("Windows reports listing a file as a missing path") + } + file := filepath.Join(t.TempDir(), "home") + if err := os.WriteFile(file, nil, 0o600); err != nil { + t.Fatal(err) + } + if err := clearOwnHome("codex", file, codexAuthFiles); err == nil || !strings.Contains(err.Error(), "codex home") { + t.Errorf("a home that is a file must be reported, got %v", err) + } +} + +// TestClearOwnHomeReportsAnUnremovableEntry checks a leftover the process cannot delete is +// reported: a read-only directory's contents cannot be unlinked on unix. +func TestClearOwnHomeReportsAnUnremovableEntry(t *testing.T) { + if runtime.GOOS == "windows" || os.Geteuid() == 0 { + t.Skip("directory permissions do not block removal here") + } + home := t.TempDir() + sub := filepath.Join(home, "skills") + if err := os.MkdirAll(filepath.Join(sub, "x"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.Chmod(sub, 0o500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(sub, 0o700) }) + if err := clearOwnHome("codex", home, codexAuthFiles); err == nil { + t.Error("an entry that cannot be removed must be reported") + } +} + +// TestExternalHomesAreOwnerOnly checks an existing home left with a wider mode is tightened +// to 0700 when the spawner is built, since MkdirAll leaves it as it found it. +func TestExternalHomesAreOwnerOnly(t *testing.T) { + withNoExecutables(t) + for _, tc := range []struct { + name string + home *func() string + }{{"claude", &claudeHome}, {"codex", &codexHome}} { + t.Run(tc.name, func(t *testing.T) { + dir := filepath.Join(t.TempDir(), tc.name) + if err := os.Mkdir(dir, 0o755); err != nil { + t.Fatal(err) + } + prev := *tc.home + *tc.home = func() string { return dir } + t.Cleanup(func() { *tc.home = prev }) + if _, err := externalSpawner(tc.name); err != nil { + t.Fatal(err) + } + if runtime.GOOS == "windows" { + t.Skip("Windows permission bits do not express owner-only access") + } + fi, err := os.Stat(dir) + if err != nil { + t.Fatal(err) + } + if fi.Mode().Perm() != 0o700 { + t.Errorf("home mode = %v, want 0700", fi.Mode().Perm()) + } + }) + } +} + +// TestEnsureOwnHomeReportsAFailure checks a home that cannot be created is an error that +// names the backend. +func TestEnsureOwnHomeReportsAFailure(t *testing.T) { + file := filepath.Join(t.TempDir(), "f") + if err := os.WriteFile(file, nil, 0o600); err != nil { + t.Fatal(err) + } + if err := ensureOwnHome("claude", filepath.Join(file, "home")); err == nil || !strings.Contains(err.Error(), "claude home") { + t.Errorf("got %v", err) + } +} + +// TestOwnHomeSpawnersGrantWriteOnTheirHome checks the claude and codex envelopes mark their +// home writable and point the CLI at it. Both CLIs refresh their token in place, so a read-only +// home would sign Flynn out at the first refresh; the grant is safe only because the home is +// Flynn's own, which is why it must not be dropped or copied to a backend that uses the +// user's. +func TestOwnHomeSpawnersGrantWriteOnTheirHome(t *testing.T) { + home := t.TempDir() + for _, tc := range []struct { + name string + cfg externagent.SandboxConfig + env string + }{ + {"claude", claudeSandboxConfig(home, externagent.Program{}), "CLAUDE_CONFIG_DIR"}, + {"codex", codexSandboxConfig(home, externagent.Program{}), "CODEX_HOME"}, + } { + if !tc.cfg.AuthDirWritable { + t.Errorf("%s: AuthDirWritable must be true", tc.name) + } + if tc.cfg.AuthDir != home || tc.cfg.AuthEnv != tc.env { + t.Errorf("%s: AuthDir/AuthEnv = %q/%q, want %q/%q", tc.name, tc.cfg.AuthDir, tc.cfg.AuthEnv, home, tc.env) + } + } +} + +// TestExternalSpawnerRefusesAHomeItCannotPrepare checks a home that cannot be created, or a +// codex home that cannot be cleared, fails the spawner instead of running an episode against +// an unprepared one. +func TestExternalSpawnerRefusesAHomeItCannotPrepare(t *testing.T) { + withNoExecutables(t) + file := filepath.Join(t.TempDir(), "f") + if err := os.WriteFile(file, nil, 0o600); err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + name string + home *func() string + }{{"claude", &claudeHome}, {"codex", &codexHome}} { + prev := *tc.home + *tc.home = func() string { return filepath.Join(file, "home") } + _, err := externalSpawner(tc.name) + *tc.home = prev + if err == nil || !strings.Contains(err.Error(), tc.name+" home") { + t.Errorf("%s: got %v", tc.name, err) + } + } + + if runtime.GOOS == "windows" || os.Geteuid() == 0 { + return + } + for _, tc := range []struct { + name string + home *func() string + }{{"claude", &claudeHome}, {"codex", &codexHome}} { + home := t.TempDir() + sub := filepath.Join(home, "skills") + if err := os.MkdirAll(filepath.Join(sub, "x"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.Chmod(sub, 0o500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(sub, 0o700) }) + prev := *tc.home + *tc.home = func() string { return home } + _, err := externalSpawner(tc.name) + *tc.home = prev + if err == nil { + t.Errorf("a %s home that cannot be cleared must fail the spawner", tc.name) + } + } +} + +// TestClaudeHomeKeepsOnlyItsSignIn checks everything a harness could leave in Flynn's claude +// home to steer a later run is gone once the spawner is built, while the sign-in survives. +func TestClaudeHomeKeepsOnlyItsSignIn(t *testing.T) { + withNoExecutables(t) + home := filepath.Join(t.TempDir(), "claude") + prev := claudeHome + claudeHome = func() string { return home } + t.Cleanup(func() { claudeHome = prev }) + if err := os.MkdirAll(filepath.Join(home, "agents"), 0o700); err != nil { + t.Fatal(err) + } + for _, name := range []string{".credentials.json", "settings.json", "CLAUDE.md", ".claude.json", filepath.Join("agents", "x.md")} { + if err := os.WriteFile(filepath.Join(home, name), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + } + if _, err := externalSpawner("claude"); err != nil { + t.Fatal(err) + } + for _, name := range []string{"settings.json", "CLAUDE.md", ".claude.json", "agents"} { + if _, err := os.Stat(filepath.Join(home, name)); !errors.Is(err, os.ErrNotExist) { + t.Errorf("%s in Flynn's claude home must be removed before an episode, got %v", name, err) + } + } + if _, err := os.Stat(filepath.Join(home, ".credentials.json")); err != nil { + t.Errorf("the sign-in in Flynn's claude home must be left alone: %v", err) + } +} diff --git a/externagent/authhome_test.go b/externagent/authhome_test.go index b0231b2e..c05d66a9 100644 --- a/externagent/authhome_test.go +++ b/externagent/authhome_test.go @@ -189,6 +189,34 @@ func TestEpisodeOptionsGrantsWriteOnlyToOwnedHome(t *testing.T) { } } +// TestDedicatedAuthDirIsWrittenInPlace proves a home marked AuthDirWritable is used as it +// is: nothing is copied out of it, so the credential inside is never read here, and the +// child may write it so the CLI can refresh its own token. A home without the mark stays +// read-only, which is what keeps a user's own CLI home safe from the harness. +func TestDedicatedAuthDirIsWrittenInPlace(t *testing.T) { + auth := authDirWithToken(t) + sp := NewSandboxSpawner(SandboxConfig{AuthDir: auth, AuthEnv: "CLAUDE_CONFIG_DIR", AuthDirWritable: true}) + home, seeded, err := sp.episodeAuthHome(workdir(t)) + if err != nil { + t.Fatalf("episodeAuthHome: %v", err) + } + if home != auth || len(seeded) != 0 { + t.Fatalf("a dedicated home must be used in place with nothing copied, got home=%q seeded=%v", home, seeded) + } + if !sp.homeWritable(seeded) { + t.Fatal("a dedicated home must be writable so the CLI can refresh its token") + } + got := writableDirsOf(t, sp.episodeOptions(home, sp.homeWritable(seeded))) + if len(got) != 1 || !sameDir(t, got[0], auth) { + t.Fatalf("the dedicated home must be the one writable grant, got %q", got) + } + + shared := NewSandboxSpawner(SandboxConfig{AuthDir: auth, AuthEnv: "CODEX_HOME"}) + if shared.homeWritable(nil) { + t.Fatal("an unmarked AuthDir is the user's own home and must stay read-only") + } +} + // writableDirsOf builds a sandbox from the options and reports the directories they left // writable, read back through the sandbox's own audit accessor. func writableDirsOf(t *testing.T, opts []sandbox.LocalOption) []string { diff --git a/externagent/codex.go b/externagent/codex.go index fca38436..0b145350 100644 --- a/externagent/codex.go +++ b/externagent/codex.go @@ -40,8 +40,8 @@ func (*Codex) Name() string { return "codex" } // the honest posture to declare, and it is why claude and codex declare different ones // from the same contract. // -// The operator's own configuration is contained: the child runs under a per-episode -// CODEX_HOME holding the credential and nothing else, so the MCP servers, model providers +// The operator's own configuration is contained: the host points the child at a CODEX_HOME +// kept apart from the operator's own and holding no config.toml, so the MCP servers, model providers // and settings in the operator's own codex config are not on the child's path to find, and // the only server it loads is the one this episode configures on the command line. func (*Codex) Lockdown() Lockdown { diff --git a/externagent/lockdown.go b/externagent/lockdown.go index 411b8b7f..4b271868 100644 --- a/externagent/lockdown.go +++ b/externagent/lockdown.go @@ -70,8 +70,8 @@ type Lockdown struct { Commands Strip // HostConfig is how the operator's own CLI configuration (their MCP servers, hooks, // plugins and settings) is kept out of the episode. StripDenied is a CLI told to read - // none of it; StripContained is a CLI pointed at a per-episode home holding only what - // it needs to authenticate. + // none of it; StripContained is a CLI pointed at a home kept apart from the operator's, + // holding only what it needs to authenticate and its own state. HostConfig Strip // Reason states, in one line, why a class is impossible, and is required whenever one // is. It is what the refusal says out loud, so an operator meets an integration gap diff --git a/externagent/spawner.go b/externagent/spawner.go index 7414809d..2b4403f3 100644 --- a/externagent/spawner.go +++ b/externagent/spawner.go @@ -79,6 +79,15 @@ type SandboxConfig struct { // A token refreshed mid-episode is refreshed only in the copy, so the host's refresh // token stays valid. Empty keeps the read-only AuthDir (right for a detection probe). AuthSeedFiles []string + // AuthDirWritable grants an episode's child write on AuthDir itself, for a home that + // exists only for this program's use of the CLI: the user signed the CLI in to it with + // the CLI's own login, so the only credential inside is one the CLI wrote there, and + // nothing in it belongs to the user's own use of the CLI. The CLI then reads and + // refreshes its token in place and this package never reads, copies or deletes it. The + // cost of a harness corrupting that home is a fresh sign-in, not a damaged personal + // credential, which is why this must never be set for the user's own CLI home. It has + // no effect when seeds are configured, because the episode then runs in the copy. + AuthDirWritable bool // ProgramDirs are the directories holding the external CLI's own executable and the // helper binaries shipped beside it. The confined child is granted read (and execute) // on them for the life of the launch and the grant is revoked on teardown. Without it @@ -366,9 +375,8 @@ func (s *SandboxSpawner) Start(ctx context.Context, ep Episode, inv Invocation) } // The home is the run's and outlives this episode (the CLI keeps its conversation // there); the credentials copied into it are this episode's and are deleted when it - // ends. owned says whether this run created the home, which is what the sandbox grants - // write access to. - owned := len(seeded) > 0 + // ends. + writable := s.homeWritable(seeded) // A per-episode writable temp directory outside the recorded workspace. A read-only host // leaves the system temp directory unwritable, and a CLI whose runtime needs scratch // space (a compiled JavaScript runtime writes to its temp on startup) exits without a @@ -380,7 +388,7 @@ func (s *SandboxSpawner) Start(ctx context.Context, ep Episode, inv Invocation) removeSeeds(seeded) return nil, fmt.Errorf("externagent: episode temp dir: %w", err) } - opts := s.episodeOptions(home, owned) + opts := s.episodeOptions(home, writable) opts = append(opts, sandbox.WithWritableDir(tmpDir)) // When the child is confined to its own network namespace, the host-loopback bridge is // unreachable from inside it, so the run's bridge is forwarded in: the child dials an @@ -420,6 +428,13 @@ func (s *SandboxSpawner) Start(ctx context.Context, ep Episode, inv Invocation) return &confinedProcess{proc: proc, closer: loc, tier: tier, seeds: seeded, tmpScratch: tmpDir}, nil } +// homeWritable reports whether an episode's child may write the credential home it was +// given: the run's own copy when seeds were copied in, or an AuthDir kept for this +// program's exclusive use of the CLI. A user's own CLI home is read-only to the child. +func (s *SandboxSpawner) homeWritable(seeded []string) bool { + return len(seeded) > 0 || s.cfg.AuthDirWritable +} + // tempEnv points the child's temp-directory variables at the per-episode writable temp, so a // runtime that writes scratch on startup finds a directory it can write. Both the Unix name // (TMPDIR) and the Windows names (TMP, TEMP) are set, so the same launch works on either. @@ -450,9 +465,11 @@ func removeScratch(dir string) { // allowlisted provider and the loopback bridge. // // The credential home is granted write only when it is the per-episode copy this Spawner -// owns and deletes; the host's own AuthDir is never made writable, so an untrusted harness -// cannot rewrite the credential it authenticates with. -func (s *SandboxSpawner) episodeOptions(home string, owned bool) []sandbox.LocalOption { +// owns and deletes, or an AuthDir the caller marked AuthDirWritable because it holds no +// credential but the one the CLI wrote there for this program. A user's own CLI home is +// never made writable, so an untrusted harness cannot rewrite the credential the user +// relies on outside this program. +func (s *SandboxSpawner) episodeOptions(home string, writable bool) []sandbox.LocalOption { opts := []sandbox.LocalOption{ sandbox.WithReadOnlyFS(), sandbox.WithSeccomp(), @@ -463,7 +480,7 @@ func (s *SandboxSpawner) episodeOptions(home string, owned bool) []sandbox.Local } if home != "" { opts = append(opts, sandbox.WithReadableDir(home)) - if owned { + if writable { opts = append(opts, sandbox.WithWritableDir(home)) } }