Repository navigation
test: unit tests can no longer write a developer's real agent or Orca settings - #23979
Conversation
…ent reads The Cursor, Copilot, Qoder and Antigravity writers and the local Codex config list resolved ~ with os.homedir() at write time, so any test that reached them wrote into the developer's real ~/.codex, ~/.cursor, ~/.copilot or ~/.gemini. Each writer now takes the home, derived once from the launch env (HOME, or USERPROFILE on Windows, else this host's home) by launchedAgentHome, which the SSH relay already used.
The structured Codex adoption replay pre-trusted /repos/workspace-1 in the real ~/.codex/config.toml; it now runs with a temp HOME and userData. The Codex session-resume and WSL hook tests created Orca's managed Codex home under the live userData, and the Claude Agent Teams tests wrote their tmux shim into ~/.orca; each now runs against a temp userData or HOME.
A vitest setup file wraps the node:fs mutating calls and refuses a target under the account's real ~/.codex, ~/.claude(.json), ~/.orca, ~/.cursor, ~/.copilot, ~/.gemini, ~/.qoder or Orca userData, found through os.userInfo() so a test that swaps HOME cannot hide it. The refusal is recorded and rethrown after the test, since trust writers swallow errors. Reads are untouched. It stands down only while an opted-in real-agent suite's own switch is set. It also unsets what an Orca terminal exports toward the live app (userData, Codex and Claude homes, and the Codex launch preflight CLI, which a shell test would otherwise run), so a local run matches CI.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| function assertOutsideRealHome(state: GuardState, operation: string, target: unknown): void { | ||
| const path = targetPath(target) | ||
| if (path === null || realAgentSuiteOptedIn()) { |
There was a problem hiding this comment.
If a developer exports any real-agent contract switch before running the full unit suite, this check disables the guard for every test in that process, not just the contract test. An unrelated test can then silently change the developer’s real agent settings. Limit the exemption to the intended contract run.
| join(home, 'Library', 'Application Support', 'orca'), | ||
| join(home, '.config', 'orca'), | ||
| join(home, 'AppData', 'Roaming', 'orca') |
There was a problem hiding this comment.
If XDG_CONFIG_HOME on Linux or APPDATA on Windows points outside the home directory, Orca uses that location for its user data, but these protected roots still assume a location under home. With ORCA_USER_DATA_PATH unset, a unit test can therefore write to the developer’s live Orca settings without the guard refusing it.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe change adds a Vitest guard that blocks filesystem writes to protected agent-home and inherited-state paths. Workspace trust flows now pass the launched agent’s home to trust writers and Codex config-path lookup. Tests add temporary home and user-data paths, explicit home arguments, and guard coverage. Priority: ⬇️ Low Merge Risk: 🔵 Low · up to The new test safety net has two gaps: a symlink, or a recursive delete of a parent directory, can still reach real settings. Production behavior is unaffected, so the change is mergeable, but these gaps should be closed soon. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change makes trust-file ownership explicit and reduces accidental modification of real settings during tests. No introduced security defect was established, but complete launch-path consistency remains unverified, and the test safeguard is not a filesystem sandbox. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 22 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 73f64332-95a3-43f1-b728-16b0f711cc69
📒 Files selected for processing (23)
config/scripts/bun-profile-change-scope.mjsconfig/scripts/vitest-real-agent-home-write-guard.test.tsconfig/scripts/vitest-real-agent-home-write-guard.tsconfig/tsconfig.node.jsonconfig/vitest.config.tssrc/main/agent-trust-presets.test.tssrc/main/agent-trust-presets.tssrc/main/agent-workspace-trust.test.tssrc/main/agent-workspace-trust.tssrc/main/codex/codex-ai-vault-session-resume.test.tssrc/main/codex/codex-home-paths.tssrc/main/codex/codex-legacy-session-resume.test.tssrc/main/codex/codex-trust-config-concurrent-launch.test.tssrc/main/codex/hook-service-wsl-runtime.test.tssrc/main/execution-host-workspace-trust.test.tssrc/main/execution-host-workspace-trust.tssrc/main/qoder/hook-service.test.tssrc/main/qoder/workspace-trust.tssrc/main/runtime/claude-agent-teams-shim-env.test.tssrc/main/runtime/orca-runtime-tests/terminal-creation-and-readiness-part-04.spec.tssrc/main/runtime/rpc/methods/structured-agent-session-adoption-replay.test.tssrc/relay/agent-workspace-trust-spawn.test.tssrc/relay/agent-workspace-trust-spawn.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
| const foldCase = process.platform === 'darwin' || process.platform === 'win32' | ||
|
|
||
| function normalize(path: string): string { | ||
| const resolved = resolve(path) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Resolve symlink destinations before allowing writes.
A temporary fixture can contain a symlink to the real ~/.codex/config.toml. copyFileSync(source, fixtureLink) passes this lexical check, but Node follows the destination symlink and overwrites the real config. No violation is recorded. (nodejs.org)
For operations that follow symlinks, compare canonical targets against canonical protected roots. For new files, resolve the nearest existing parent before checking containment. Add a regression using temporary protected roots.
| return | ||
| } | ||
| const normalized = normalize(path) + sep | ||
| const root = state.roots.find((candidate) => normalized.startsWith(candidate)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Block recursive removal of protected roots through their parents.
With the guard enabled, rmSync(userInfo().homedir, { recursive: true, force: true }) passes this check because the home is not inside any protected root. Node then recursively removes the home, including the protected settings directories. (nodejs.org)
For recursive removal, reject targets that contain a protected root as well as targets inside one. Keep this check operation-specific so unrelated writes under the home remain allowed. Add a regression using a temporary parent and protected child.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one minor suggestion inline.
Reviewed changes
- Trust writers take the launch home, not
os.homedir()at write time.WorkspaceTrustHostgainsagentHome, computed once per launch bylaunchedAgentHome(win32→USERPROFILE, elseHOME, elsehomedir());markCursorWorkspaceTrusted,markCopilotFolderTrusted,markAntigravityWorkspaceTrusted,markQoderWorkspaceTrusted, andgetLocalCodexTrustConfigFiles(agentHome)now receive it. The local and relay paths share the rule, so a launch whose env names a different home writes trust where that agent will read it. Relay wire shape is unchanged. - New vitest guard refuses writes to the real agent/Orca home.
config/scripts/vitest-real-agent-home-write-guard.tswraps mutatingfs/fs.promisescalls and throws on targets under~/.{codex,claude,orca,cursor,copilot,gemini,qoder},~/.claude.json*, and the default Orca userData folders. It usesuserInfo().homedirso swappingHOMEcan't hide the account home, records refusals so a writer that swallows the error still fails the test, and stands down only under the opted-in real-agent suite switches. It also unsets the Orca paths an Orca terminal exports into the test process. - Tests that reached the real home now use temp locations. Adoption replay, Codex session-resume, WSL hooks, Claude Agent Teams shim, and the runtime readiness fragment; the trust unit tests pass an explicit home through the new signatures.
Verified this run: pnpm tc:node clean; guard self-test (8 tests), 11 changed test files (184 tests), and the orca-runtime.test.ts aggregator (1329 tests) all pass.
ℹ️ The guard only covers the process it runs in
The guarantee is narrower than it reads: only writes made through the test process's own node:fs module are checked. A test that spawns a child (a shell, an agent CLI, a shim) can still write the real home, which is why the PR's own ~/.orca-relay/~/.orca-remote cache writes remain open. Worth keeping the follow-up tracked so the boundary doesn't get mistaken for full coverage.
Technical details
# Guard coverage boundary
## Affected sites
- `config/scripts/vitest-real-agent-home-write-guard.ts:8-12` — JSDoc already states child processes are outside the guard.
- `src/relay/pty-handler-*.test.ts`, `src/relay/terminal-history.test.ts`, `src/relay/omp-fresh-launch-environment.test.ts` — still create `~/.orca-relay/...` and `~/.orca-remote/...` under the real home; those prefixes are not in `AGENT_HOME_ENTRIES` or the Orca folder list.
## Required outcome
- The guard's stated scope and the suite's actual guarantee stay in sync: either the relay cache prefixes are protected (or redirected) or the limitation stays documented where reviewers will see it.
## Open questions for the human
- Is the relay follow-up tracked in an issue? If not, a short note in the PR description would anchor it.DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| const folders = realHomes().flatMap((home) => [ | ||
| ...AGENT_HOME_ENTRIES.map((entry) => join(home, entry)), | ||
| join(home, 'Library', 'Application Support', 'orca'), | ||
| join(home, '.config', 'orca'), |
There was a problem hiding this comment.
The Orca userData roots are hardcoded to the default layouts (~/.config/orca, ~/Library/Application Support/orca, ~/AppData/Roaming/orca), but production getOrcaUserDataPath() honors XDG_CONFIG_HOME on Linux and APPDATA on Windows. On a machine that overrides either, the real userData directory is not a protected root, so a test could still write the developer's real Orca settings — the exact failure this guard is meant to close.
Technical details
# Missing XDG_CONFIG_HOME / APPDATA-derived Orca roots
## Affected sites
- `config/scripts/vitest-real-agent-home-write-guard.ts:65-70` — only the default `orca` folders are added.
- `src/main/codex/codex-home-paths.ts:62-75` — `getOrcaUserDataPath()` uses `process.env.XDG_CONFIG_HOME` (linux) and `process.env.APPDATA` (win32) before the home default.
## Required outcome
- The guard protects whichever directory Orca actually uses for userData when those env vars are set, mirroring `getOrcaUserDataPath()`.
## Suggested approach (optional)
- Add roots derived from `process.env.XDG_CONFIG_HOME` and `process.env.APPDATA` (when absolute) alongside the per-home defaults.
… home Semantic port of stablyai/orca 5b3366f (stablyai#23979), parts (b) and (c): the vitest setup guard config/scripts/vitest-real-agent-home-write-guard.ts (+ test) registered first in config/vitest.config.ts and config/tsconfig.node.json, protecting ~/.codex, ~/.claude(.json), ~/.orca, ~/.axiom, ~/.cursor, ~/.copilot, ~/.gemini, ~/.qoder, ~/.config/orca, ~/.config/axiom and the Orca userData; plus temp HOME / userData for the tests that wrote the real home. Part (a) (trust writers taking the launched agent's home) is deferred to the trust-subsystem sync: upstream moved those writers into files this tree does not have. Content prepared by the UPSTREAM-1 attempt-1 Worker (launcher commit 63e71074ea); the Codex sandbox cannot write .git, so the verifier created the commits. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6WthNG1VZrbuiAXy8bpY7
ELI5
Running Orca's unit tests on a developer machine quietly edited that developer's real Codex settings file. Two fake folders from test fixtures,
/tmp/worktree-aand/repos/workspace-1, ended up marked "trusted" in the real~/.codex/config.toml. This PR makes that impossible to do silently: a test that tries to write into a developer's real agent or Orca settings now fails, and the tests that did so now use throwaway folders.What Changed
The problem. On 2026-09-29 two project-trust entries appeared in a developer's real
~/.codex/config.toml:[projects."/tmp/worktree-a"]and[projects."/repos/workspace-1"]. Both are fixture paths from unit tests. Orca writes a "trust this folder" entry for the agent it is about to start (#23744, #23937), and those writers looked up the home folder at write time withos.homedir(). Any test that reached a writer without replacing the home therefore wrote into the real one. The same writers also create~/.cursor/projects/…,~/.copilot/config.json,~/.qoder/settings.jsonand~/.gemini/antigravity-cli/settings.json./repos/workspace-1came fromsrc/main/runtime/rpc/methods/structured-agent-session-adoption-replay.test.ts, which starts a structured Codex chat with folder pre-trust left on./tmp/worktree-ais the shared fixture path of the runtime tests (TEST_WORKTREE_PATH,AGENT_PROMPT_TEST_WORKTREE_PATH). I could not reproduce that write on currentmain, onmainjust before feat(agents): pre-trust the folder wherever Orca starts an agent #23744, at the feat(agents): pre-trust the folder wherever Orca starts an agent #23744 merge, or at the matching point of the branch being tested that morning; it most likely came from an intermediate branch state. The guard below now catches it regardless of which test it was.User-facing change. None for people using Orca. For developers: running the unit tests can no longer change your real Codex, Claude, Cursor, Copilot, Qoder, Gemini or Orca settings, and a test that tries fails with the file path and the call stack that tried.
Mechanism.
config/scripts/vitest-real-agent-home-write-guard.ts, loaded as a vitest setup file) wraps the Node file-system calls that change disk: write, append, mkdir, mkdtemp, rename, copy, symlink, link, rm, rmdir, unlink, truncate, open-for-write and write streams, in their callback, sync and promise forms. A target under the real~/.codex,~/.claude,~/.claude.json(and its temp siblings),~/.orca,~/.cursor,~/.copilot,~/.gemini,~/.qoderor Orca's default user-data folder is refused. Reads are untouched.os.userInfo().homedir, which changingHOMEdoes not move, so a test that pointsHOMEelsewhere cannot hide the real one.ORCA_REAL_CLAUDE_SUPERVISED_STOP,ORCA_CODEX_CONTRACT_BINARY,ORCA_CODEX_NO_DAEMON_CONTRACT_BINARY,ORCA_CODEX_TRUST_CONTRACT_BINARYandORCA_REPRO_CODEX_BINARY.ORCA_USER_DATA_PATH,CODEX_HOME,ORCA_CODEX_HOME,CLAUDE_CONFIG_DIRandORCA_CODEX_LAUNCH_PREFLIGHT. The guard protects the first four and then unsets all five for the test process, so a local run behaves like CI. Before this, runningsrc/main/pty/codex-shell-no-daemon.test.tsfrom an Orca terminal made its child shells run the installed Orca CLI'sagent hooks prepare-codexagainst the live app's user data. The rule: a test run started inside an Orca terminal never targets the live app. This follows the repo's ownvitest-caller-identity-env-setup.ts, which already clears an Orca terminal's caller identity for tests the same way. The opted-in real-agent suites keep these variables while their switch is set.os.homedir(). The home is worked out once per launch from the launch environment (HOME, orUSERPROFILEon Windows, else this machine's home) bylaunchedAgentHome, the rule the SSH relay already used inline. The local Codex config list uses the same home. SSH hosts still work out the home on the relay from the relay's own spawn environment. WSL launches are unchanged: every writer except Claude's still skips them.HOME), the Codex session-resume and WSL hook tests (Orca's managed Codex home under user data), and the Claude Agent Teams tests (their tmux shim under~/.orca).Why
A second guard beside each writer would only cover the writers someone remembered. Refusing the write at the file-system layer covers every present and future writer, including ones that swallow errors. Passing the home in, rather than mocking
os.homedir()in each test, makes the writer's target explicit. It also gives the local and SSH paths one rule for "the home the agent will read". Unsetting the inherited Orca paths removes the cause of the local-only leaks instead of patching each test.What the guard does not cover: child processes a test starts, since each has its own file-system module. Three tests still create folders under
HOMEthat are outside the protected list:src/relay/pty-handler-*.test.tsandsrc/relay/terminal-history.test.ts(~/.orca-relay/shell-ready,~/.orca-remote/terminal-history) andsrc/relay/omp-fresh-launch-environment.test.ts(~/.orca-relay/omp-managed-status-extension). These are the relay's own cache folders, not agent or app settings, and are left for a follow-up.Linked Issue
N/A (found while working on agent pre-trust; no issue filed).
Visual Proof
N/A: this changes tests and where trust files are written, not any screen.
Testing
config/scripts/vitest-real-agent-home-write-guard.test.ts, 8 cases): sync, promise and swallowed writes under the real home are refused; a forcedrmof a missing folder, which would otherwise succeed silently, is refused; the account home stays protected withHOMEpointed elsewhere; a sibling like~/.codex-…is left alone; the guard stands down under a real-agent switch; reads and temp-home writes work. Every probe path sits under a missing folder or is a no-op removal, so nothing is created even with the guard off.rmcase, which then returns without throwing. Nothing was created in the real home. With the check restored, all pass.HOMEset to a scratch folder: 1,696 test files (every unit test matching spawn, pty, runtime, trust, relay, codex, agent hooks and the per-agent writers), 17,347 tests. Before the fixes the guard caught 15 files; after them it catches none. The one remaining failure,src/main/providers/local-pty-provider-spawn-session.test.ts(it cannot load alinux-arm64native module on macOS), also fails the same way on code without this change.USERPROFILE,AppData/Roaming/orca) and case-insensitive comparison are covered by CI.