Repository navigation
fix(externagent): run claude and codex against their own homes, never copy their tokens - #643
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Review: the change is right, and no token copy path is left. Four things to fix before merge:
|
The claude backend used to read the user's Claude Code credential file and config out of ~/.claude and copy them into a per-run directory for each episode. Flynn handled the OAuth token itself, and a token the CLI refreshed mid-episode was refreshed only in the copy. The backend now keeps a Claude Code home of its own under the data directory. The user signs the CLI in to it once with the CLI's own login, and every probe and episode points the CLI at it through CLAUDE_CONFIG_DIR. Flynn never reads, copies or deletes the token; the CLI writes it at sign-in and refreshes it in place. The user's own home, settings, hooks and MCP servers are no longer read at all. SandboxConfig gains AuthDirWritable so an episode may write such a dedicated home. A user's own CLI home stays read-only to the child, as before. When claude is installed but not signed in, the onboarding message now prints the sign-in command for Flynn's home, quoted for PowerShell on Windows and for sh elsewhere. Signed-off-by: Ion Alpha <[email protected]>
The codex backend copied auth.json out of the user's own codex home into a per-run home for every episode. It now gets the same treatment as claude: Flynn keeps a codex home of its own under its data directory, the user signs the CLI in to it once with `codex login` (Flynn prints the exact command), and probes and episodes point the CLI at it through CODEX_HOME with write access, so the CLI refreshes its token in place and Flynn never reads it. The user's config.toml is never on the child's path. A config.toml found in Flynn's own codex home is removed before a spawner is built, so a harness cannot leave one behind to steer a later run. The not-signed-in message for both backends now comes from one helper, and the login command is built by one function per shell. Existing codex users sign in once more; their own ~/.codex is untouched. Signed-off-by: Ion Alpha <[email protected]>
MkdirAll leaves an existing home with the mode it already had, and the home holds a bearer token, so the claude and codex homes are now chmod to 0700 when the spawner is built. An episode's child can write the codex home, so removing only config.toml left AGENTS.md, prompts and skills free to steer the next run. The home is now cleared of everything except auth.json, the sign-in the CLI keeps in its file credential store. The claude and codex sandbox configs are built by named functions so a test can hold AuthDirWritable to true, and the error paths of the home preparation are tested. Signed-off-by: Ion Alpha <[email protected]>
The claude home is writable by the episode's child, and nothing cleared it between runs, so settings.json hooks, CLAUDE.md, agents, commands or plugins left by one episode would steer the next. clearCodexHome is generalised to clearOwnHome over a keep-set, and the claude branch of externalSpawner now calls it keeping only .credentials.json. .claude.json is cleared too: with CLAUDE_CONFIG_DIR set it lives in the home and holds user-scoped MCP servers and trust decisions. The token is in .credentials.json, so the CLI only re-reads the account details. The claudeHome comment no longer says the worst a harness can do is sign the copy out. Signed-off-by: Ion Alpha <[email protected]>
The spawner test for a home that cannot be cleared now runs against both backends, so the claude branch's error return is exercised. Signed-off-by: Ion Alpha <[email protected]>
f1ded8b to
a66691a
Compare
|
Second review: the codex fixes are good. The claude home still has the problem that was fixed for codex. Please clear the claude home the same way: keep only the sign-in ( |
What
Flynn now keeps a separate home for each external CLI under its data directory (
claude/andcodex/) and runs the CLI against it, instead of copying the user's own credential (~/.claude/.credentials.json, orauth.jsonfrom~/.codexorCODEX_HOME) into a per-run home for every episode. Flynn never reads, copies or stores the token inside.Why
The user's personal sign-in belongs to their own use of the CLI, and Flynn had no reason to handle it. A copied token also lives outside the CLI's control: the CLI cannot refresh the original, and the copy sits in a directory the user never asked for. With a home of its own, each CLI signs in through its own login flow, keeps its token where it put it, and refreshes it in place.
How to verify
go test ./cmd/flynn ./externagentTestDedicatedAuthDirIsWrittenInPlace: a home marked writable (SandboxConfig.AuthDirWritable) is used in place with nothing copied, and an unmarked home stays read-only.TestOwnHomeSpawnersGrantWriteOnTheirHome: the claude and codex envelopes setAuthDirWritable,AuthDirand the right environment variable.TestClaudeHomeIsFlynnsOwnAndNeverTheUsers,TestCodexHomeIsFlynnsOwnAndNeverTheUsers: each backend uses Flynn's home whatever the user's environment variable says, and the home exists before detection. The codex test also checks thatconfig.toml,AGENTS.mdand askillsdirectory are removed whileauth.jsonis kept.TestExternalHomesAreOwnerOnly: an existing home with mode 0755 is 0700 once the spawner is built (the mode assertion is skipped on Windows, where permission bits do not express owner-only access).TestClaudeHomeKeepsOnlyItsSignIn:settings.json,CLAUDE.md,.claude.jsonand anagentsdirectory are removed from the claude home and.credentials.jsonis kept.TestClearOwnHomeReportsAnUnreadableHome,TestClearOwnHomeReportsAnUnremovableEntry,TestExternalSpawnerRefusesAHomeItCannotPrepare,TestEnsureOwnHomeReportsAFailure: the non-ENOENT error paths.TestLoginSignsInFlynnsHome: the sign-in message for both backends names the right variable and login command, quotes the path for sh and PowerShell, and never asks for an API key.flynn --model codex(orclaude) with no sign-in; it prints aCODEX_HOME=... codex login(orCLAUDE_CONFIG_DIR=... claude auth login --claudeai) command. After running it, the backend reports itself ready and~/.codex/~/.claudeare unchanged.Notes for reviewers
--model claudeor--model codexsigns in once more. Their own~/.claudeand~/.codexsign-ins are untouched.config.tomlare never on the child's path. The child may write Flynn's codex home, so before a spawner is built everything in that home exceptauth.jsonis removed (config,AGENTS.md, prompts, skills, history, caches); the CLI recreates what it needs. The claude home gets the same treatment through the sharedclearOwnHome, keeping only.credentials.json:settings.json(hooks, permission allow rules),CLAUDE.md,agents/,commands/,plugins/and.claude.jsonare removed. WithCLAUDE_CONFIG_DIRset, Claude Code keeps.claude.jsonin that directory, and it holds user-scoped MCP servers and trust decisions, so it cannot be kept; the account details in it are re-read from the token. I did not run the claude CLI against a cleared home to confirm its status check still reports signed in, so that is worth confirming by hand. A sign-in held in the OS keyring is outside the home and untouched.MkdirAllleaves an existing directory's mode alone, so both homes are chmod to 0700 explicitly.homeLoginCommandhas no Linux CI coverage because it is selected byruntime.GOOS == "windows", and codecov uploads the Linux coverage profile. The Windows test job does runTestLoginSignsInFlynnsHome, which asserts the quoted path and the trailingRemove-Item. The branch is two lines of string concatenation, and no Linux test can reach it without making the OS a parameter.os.Chmodfailure return inensureOwnHome. It can only fail afterMkdirAllsucceeded on the same path (a race with another process or a filesystem that refuses chmod), so there is no portable way to provoke it. TheRemoveAllfailure return inclearOwnHomeand both spawners' clear failures are covered on Linux and skipped on Windows and as root, because directory permissions do not block removal there.