Repository navigation
fix(codex): trust the worktree Codex starts in, not a guessed repo root - #23937
Conversation
Codex keys a linked worktree's trust on the main checkout only when that checkout's .git leads back to the common git dir; otherwise (bare repo, --separate-git-dir) it keys on the worktree itself. Orca always wrote the main-checkout key, so Codex showed its trust prompt and worker-start failed at agent_readiness. Mirror trust.rs exactly, and pin it with a real-binary contract that runs in the existing Codex contract CI job. Fixes #23847
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // job pins --no-daemon for Orca's codex shell wrapper. | ||
| // job pins --no-daemon for Orca's codex shell wrapper and the project-trust key. | ||
| const CODEX_INDEX_HEAL_CONTRACT_PREFIXES = [ | ||
| 'src/main/agent-trust-presets', |
There was a problem hiding this comment.
The new trigger covers agent-trust-presets.ts and its test, but not helpers that locate or write Codex trust entries, such as codex-home-paths.ts and config-toml-trust.ts. A change to either helper can skip the pinned-binary trust test, so that test may miss a regression in the path or entry Codex reads. This is a non-blocking CI coverage gap; include those dependencies in the job’s path triggers.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Added src/main/codex/config-toml-trust to the contract triggers in 3315c33 (with a classification test). Leaving out codex-home-paths: it decides which config files Orca writes, and the ordinary (no-binary) run of the same test already asserts the key lands in both the system and runtime config, so a regression there fails in every unit run without the pinned binary.
There was a problem hiding this comment.
That distinction is reasonable. With config-toml-trust now covered, the remaining pinned-binary-specific dependency is exercised; agent-trust-presets.test.ts already verifies that getLocalCodexTrustConfigFiles() causes the workspace key to be written to both the system and runtime config paths, so a codex-home-paths regression should fail the ordinary test run. No further trigger change is needed.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Codex trust root now mirrors Codex's own rule —
resolveCodexProjectTrustRootrequires both the worktree'scommondirand the main checkout's.gitto resolve to the canonical common git dir before returning the main checkout; otherwise it returns the worktree path. NewreadGitDirPointerhelper (symlinked pointers rejected), used for the workspace.gitand the main checkout.git. - Real layout tests —
linked worktree trust keybuilds six real git layouts (standard,.baresibling pointer, bareproj.git,--separate-git-dir, mismatched main.git, plain folder) and asserts the exact single key written to both config files, so the alternative "write both keys" fix would also fail. - Real-binary trust contract — with
ORCA_CODEX_TRUST_CONTRACT_BINARYset, each layout drives Codexapp-serverthread/startand assertsworkspaceWriteunder Orca's config vs.readOnlyunder a cleanCODEX_HOME;REQUIRED=1turns a missing binary into a failure instead of a skip. - CI wiring — new
Verify Codex project-trust contractstep pinned to the no-daemon Codex pin, andcodex_index_heal_contractnow triggers onsrc/main/agent-trust-presets.
I traced the resolver against resolve_root_git_project_for_trust at rust-v0.159.0 branch by branch — the .git-marker discovery, worktrees/commondir checks, non-canonical main_root derivation, and the canonical comparison against the common dir all match, and every new branch can only narrow the written key. The contract assertion is real: ThreadStartResponse.sandbox: SandboxPolicy is the field the test reads, and the empty-CODEX_HOME control means a thread/start that ignored trust would fail the negative case.
ℹ️ Stray trust keys from the old resolver are not cleaned up
The fix stops writing the main-checkout key for bare/--separate-git-dir worktrees, but markCodexProjectTrusted still only upserts and never removes, so entries a previous Orca version wrote for dirname(commonDir) (e.g. the parent directory of proj.git, which can hold unrelated projects) remain in ~/.codex/config.toml and the Orca runtime home. The PR body explicitly accepts leaving them in place, and they are dormant for the worktree that produced them since Codex now looks up the worktree path — this is a carry-forward note, not a request for a migration.
Technical details
# Legacy trust entries persist
## Affected sites
- `src/main/agent-trust-presets.ts:165-180` — `markCodexProjectTrusted` upserts the resolved key into the system and runtime configs; no removal path for the old key.
- `src/main/codex/config-toml-trust.ts:120-139` — `upsertProjectTrustLevel` only adds/updates one project block.
## Why it is dormant
A bare-repo worktree now resolves to the worktree path, so Codex's lookup no longer consults the stray parent key for that workspace. The residual risk is narrow: the parent directory would have to later become a `.bare`-style main checkout (a `.git` pointer to a sibling bare dir) for the stray grant to be read.
## Open question for the human
Is a one-time cleanup worth it? It cannot reliably distinguish an Orca-written key from one the user accepted via the trust prompt, so silence is a defensible default — surfacing only so the decision is deliberate.DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughCodex trust configuration now uses the canonical workspace path, including for linked worktrees. Tests cover multiple Git layouts and, when configured, verify Codex app-server sandbox behavior. The PR workflow runs a required project-trust contract check and classifies trust-related changes to run the contract and packaging jobs. Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Codex trust for linked worktrees now uses the worktree's own path. No merge-blocking issue was found in the supplied changes, and CI runs a contract check against the real Codex binary. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change makes new trust entries workspace-specific rather than checkout-wide and preserves the existing trust-writing controls. Previously unsupported repository layouts now receive effective automatic trust. Older trust entries may remain, and validation is limited to the pinned Codex contract. 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 45.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 9 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 |
Keeps #23744's exported resolveCodexProjectTrustRoot and its configFiles-based markCodexProjectTrusted, with this branch's Codex-mirroring root rule. Drops main's "trusts the main repository root for a linked worktree without reading commondir" test: its fixture has no commondir, a layout Codex's resolve_root_git_project_for_trust rejects (it keys trust on the worktree), so it encoded behaviour Codex does not have. The branch's real-git layout tests cover the case instead. linkGitWorktree now writes commondir as git does, so the execution-host trust tests describe a worktree Codex would recognise.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Merged
origin/maininto the branch — the only commit since the prior review is the mergeea6505d. Conflict resolution preserved this PR'sresolveCodexProjectTrustRootlogic while adoptingmain's refactoredmarkCodexProjectTrusted(workspacePath, configFiles)signature and itsAgentTrustPresettype change. - Test call sites rewired — the test now passes
getLocalCodexTrustConfigFiles(); the six-layoutlinked worktree trust keysuite, the adversarial-metadata test, and the real-binary contract are intact. - Shared fixture updated —
linkGitWorktreenow writescommondir(../..), which the new resolver requires, so the execution-host and relay trust tests using it still exercise the codex path.
The resolver itself is unchanged from the reviewed 860bcc1 and remains a faithful mirror of Codex's trust.rs. I confirmed the merge resolution imports getLocalCodexTrustConfigFiles correctly, that resolveCodexProjectTrustRoot is now also consumed by execution-host-workspace-trust.ts:74, and ran the affected suites locally: agent-trust-presets, execution-host-workspace-trust, relay/agent-workspace-trust-spawn, and codex-trust-config-concurrent-launch all pass (105 passed, 1 skipped real-binary contract).
The carry-forward note from the prior review (previously written stray trust keys under dirname(commonDir) are left in place) is unchanged by this merge and remains deliberately accepted in the PR body, so it is not re-raised here.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Codex looks up the cwd's own [projects] entry before any repo root (config_toml.rs get_active_project, loader decision_for_dir), so trusting the workspace realpath satisfies every git layout. Drops the resolve_root_git_project_for_trust mirror: simpler, cannot drift from Codex, and never widens trust past the folder Orca launched in. Cost is one config entry per worktree; entries older Orca wrote on main checkouts stay valid. The six real-git layout tests now assert the workspace key and, under the contract, that real Codex starts workspaceWrite for each. The contract probes the binary version once and fails at load when required but missing; its CI path filter now includes config-toml-trust.
Jinwoo-H
changed the title
|
@pullfrog re "every branch matches trust.rs": the mirror is gone as of 3315c33. Codex checks the cwd's own |
|
You're right — my previous review described the superseded revision. At I checked the lookup-order claim against Codex 0.159.0 source rather than taking it on faith:
So a key on the workspace realpath satisfies the cwd lookup for every layout, and the six-layout suite plus the real-binary contract assert exactly that. Ran the three affected suites locally: 95 passed, with the The prior carry-forward note — older
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Resolver deleted; Codex now trusts the workspace path itself —
resolveCodexProjectTrustRootand itsgitdir:/commondirtraversal are removed.markCodexProjectTrustedwrites[projects."<canonicalize(workspacePath)>"]with no git-layout logic, since Codex checks the cwd's own entry before the repo root. The change strictly narrows trust versus the old main-checkout key. - Execution-host path simplified —
storedTrustPathis removed andapplyWorkspaceTrustOnThisHostpassesworkspacePathstraight through to the writer and the home guard (Codex is not an inheriting preset, so the guard never applied to it). - Tests drive real git layouts — the
linked worktree trust keysuite builds six real layouts with thegitCLI (standard worktree, sibling.barepointer, bareproj.git,--separate-git-dir, a parent.gitpointing elsewhere, plain folder) and asserts the exact single key written to both config files. - Real-binary contract — with
ORCA_CODEX_TRUST_CONTRACT_BINARY, each layout runs Codexapp-serverthread/start(cwd= workspace) and assertsworkspaceWriteunder Orca's config againstreadOnlyunder an emptyCODEX_HOME;ORCA_CODEX_TRUST_CONTRACT_REQUIRED=1fails at load when the binary is missing. TheCodex index-heal contractCI job (which now runs this with the pinned 0.158.0) is green on this head. - CI wiring and call-site updates — new
Verify Codex project-trust contractstep,agent-trust-presets/config-toml-trustadded to the job's path triggers (with a classification test), and the execution-host/relay trust tests plus the now-dead resolver mock updated.
I traced the new production path: markCodexProjectTrusted canonicalizes once and the execution-host writer receives the same path, so the key Orca writes and the folder Codex looks up for a worktree launch are identical. The real-binary differential (trusted config vs empty home) is genuine coverage — the only variable between the two runs is the entry Orca wrote — and thread/start's cwd is the same project lookup Orca's own production openCodexThread drives. The prior carry-forward note (legacy keys are never removed) is unchanged by this commit and remains deliberately accepted in the PR body, so it is not re-raised.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Main (#23744, #23937) moved every agent trust write into one dispatcher, applyWorkspaceTrustOnThisHost, run on the host that starts the agent: main for local and WSL launches, the SSH relay for remote ones. The desktop SFTP trust writer, the trust IPC and the per-call-site launch-prep writes are gone. Re-applied on top: - markCodexProjectTrusted attempts every config.toml it is given, so a file Orca refuses to edit never leaves another Codex home untrusted. - The dispatcher reports refused Codex config edits once per file and message. - smol-toml is loaded with a literal require so the relay's esbuild bundle inlines it; relay hosts have no node_modules, and createRequire made the relay fail to load. - Remote trust tests ported to the relay writer; hook-status log-once test moved out of the test file main deleted.
…ership #23930 merged current main, whose #23744/#23937 moved trust writing into applyWorkspaceTrustOnThisHost (local) and the SSH relay. Resolved by taking that design and dropping what it already covers: the per-home trust writes, the launch-prep trust writes and their shared budget, and the worker-start trust diagnosis (moved to a follow-up; #23847 is now referenced, not fixed). Kept on top of it: - Local Codex trust also reaches the per-account home the launch reads: the launch env's CODEX_HOME joins getLocalCodexTrustConfigFiles, as the relay already does. - An OS error that stops a trust write names config.toml, not the atomic writer's temp file; config refusals pass through unchanged. - The WSL hook-trust relocation now logs through warnCodexConfigOnce and writes through withConfigPath, the base's single refusal and write rules.
1 participant
ELI5
Before Codex works in a folder it asks "Trust this folder?". Orca answers that question ahead of time by writing a "trusted" entry into Codex's config. For worktrees of a bare repository, Orca wrote the entry under a folder name Codex never looks up, so Codex still asked, and an Orca-started Codex worker sat at the question until it failed.
What Changed
Before:
orca orchestration worker-start --agent codexinto a worktree of a bare repo (proj.git+ worktrees) or a repo made withgit init --separate-git-dirfailed withagent_readiness/ "Agent startup blocked: agent-trust-workspace". The same happened when opening Codex in such a worktree from the UI: the trust prompt appeared even though Orca had "pre-trusted" it.After: Codex starts with no trust prompt in every worktree layout. Orca now writes the trust entry for the exact folder it starts Codex in.
Mechanism:
markCodexProjectTrusted(src/main/agent-trust-presets.ts) used to guess the folder Codex'sresolve_root_git_project_for_trustwould pick (for a linked worktree, the folder above the shared git dir, "the main checkout") and write[projects."<that folder>"] trust_level = "trusted". For a bare repo that guess was wrong: the folder aboveproj.gitis not a checkout, so Codex looked up the worktree path and found nothing. But Codex checks the folder it runs in before any repo root (get_active_projectincodex-rs/config/src/config_toml.rsanddecision_for_dirincodex-rs/config/src/loader/mod.rs, same order back to 0.80). So Orca now writes the entry for the workspace's realpath, with no git-layout logic at all, and the execution-host home guard (from #23744) checks that same path for every agent.Why
trust.rsfixes the bug too, but it is more code, has to be kept in step with Codex, and still marks a parent folder trusted. Writing the workspace path is simpler, cannot drift from Codex, and never widens trust beyond the folder Orca launched in. The cost is one config entry per worktree instead of one per repo. Entries older Orca wrote for main checkouts stay valid and are left in place.ORCA_CODEX_TRUST_CONTRACT_BINARYis set, each layout test also starts a real Codex app-server with the config Orca wrote and asserts Codex treats the worktree as trusted (thread/startreturns aworkspaceWritesandbox; the same config without the entry returnsreadOnly). Both come from the same project lookup the trust prompt uses. It runs in the existing Codex contract CI job against the already-pinned 0.158.0, and that job now also triggers on changes toagent-trust-presetsandcodex/config-toml-trust.Out of scope (follow-up): when a trust prompt does appear anyway, worker-start still fails at
agent_readinessinstead of offering a recovery path.Linked Issue
Fixes #23847
Visual Proof
N/A — no UI change. The effect is that a trust prompt no longer appears; evidence is the worker-start receipts below.
Testing
Unit tests in
src/main/agent-trust-presets.test.tsbuild real layouts with the git CLI (standard repo + worktree,.bare+.gitpointer, bareproj.git+ worktree,--separate-git-dir, a parent.gitpointing at a different repo, plain folder) and assert the key written is the workspace realpath.Real-binary contract run locally with isolated
CODEX_HOME/HOME: Codex 0.158.0 and 0.150.1 report every layout trusted (workspaceWrite), including a standard linked worktree; a reviewer run with 0.159.1 agreed. WithORCA_CODEX_TRUST_CONTRACT_REQUIRED=1and no binary the suite fails at load.Live (earlier revision of this PR), macOS, isolated headless dev runtime with Codex 0.159.0:
worker-start --agent codexinto a bare-repo worktree →ready(onmain:failed/agent_readiness/agent-trust-workspace).Windows/Linux: the path is
realpathSync.nativeof the workspace, as before; the unit tests run on all CI platforms. SSH: unchanged — this runs wherever Codex's trust preset is written today.pnpm tc:node, oxlint andcheck:code-quality:changedpass.I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Security: trust can only narrow compared with before (the workspace path instead of a parent folder). Cross-platform, SSH, mobile, and backwards compatibility: no wire or UI changes; existing trusted entries are left in place. Performance: no extra file reads; the git-metadata reads are gone.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)