From eff59cd1a5c5db4cb053740c080961405952a4f4 Mon Sep 17 00:00:00 2001 From: Daniel Kim <79390398+sox8502@users.noreply.github.com> Date: Sun, 12 Jul 2026 15:19:24 -0700 Subject: [PATCH] Codex harness: resolve bin symlinks, grant git dir + network in Auto sandbox (v0.1.49) (#51) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Codex harness: resolve bin symlinks, grant git dir + network in Auto sandbox Two fixes for headless codex-cli 0.144 under orx: 1. find_codex() now canonicalizes the PATH hit. Codex >= 0.144 locates its codex-code-mode-host helper as a sibling of the *invoked* path without dereferencing symlinks, and installers drop only a lone `codex` symlink into ~/.local/bin — so every spawn via the symlink died with "No such file or directory" before running a single command. 2. Auto mode (sandbox_mode=workspace-write) now also passes sandbox_workspace_write.network_access=true and declares the repo's `git rev-parse --git-common-dir` as a writable root. Codex's sandbox blocks network by default (DNS included — git fetch/push, package installs, and the orx CLI's localhost API all fail) and denies every git metadata write (in-workspace .git is marked read-only; a worktree's real metadata lives in the hub clone, outside the workspace). The interactive TUI escalates both denials to approval prompts; codex exec has no approval channel, so they were hard failures. All behavior verified against codex-cli 0.144 via `codex sandbox` (no model calls) in plain clones and worktrees. * Bump version to 0.1.49 --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/local/harness/codex.rs | 121 ++++++++++++++++++++++++++++++++++-- src/local/harness/detect.rs | 40 ++++++++++++ 4 files changed, 159 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 81153be9..59dda05b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -928,7 +928,7 @@ checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" [[package]] name = "openresearch-cli" -version = "0.1.48" +version = "0.1.49" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index d5359523..96e2a5e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openresearch-cli" -version = "0.1.48" +version = "0.1.49" edition = "2021" description = "OpenResearch CLI (orx) — Rust port" repository = "https://github.com/alphaXiv/openresearch-cli" diff --git a/src/local/harness/codex.rs b/src/local/harness/codex.rs index ef272365..91d0f098 100644 --- a/src/local/harness/codex.rs +++ b/src/local/harness/codex.rs @@ -12,7 +12,7 @@ //! Detection: `~/.codex/auth.json` holds either an `OPENAI_API_KEY` or an OAuth //! `id_token` JWT we decode (unverified) for the account email and plan. -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::Stdio; use async_trait::async_trait; @@ -21,7 +21,8 @@ use tokio::io::{AsyncBufReadExt, BufReader}; use tokio::process::Command; use super::detect::{ - bin_version, find_on_path, jwt_payload, nonempty_str, read_json, title_case, HarnessInfo, + bin_version, find_on_path, jwt_payload, nonempty_str, read_json, resolve_symlinks, title_case, + HarnessInfo, }; use super::options::{HarnessOptions, PermissionMode}; use super::Harness; @@ -47,8 +48,10 @@ const CODEX_REASONING_LEVELS: [(&str, &str); 4] = [ pub struct Codex; +/// `codex` on PATH, symlinks resolved (see `resolve_symlinks` — codex needs to +/// find its `codex-code-mode-host` helper next to the real binary). pub fn find_codex() -> Option { - find_on_path("codex") + find_on_path("codex").map(resolve_symlinks) } #[async_trait] @@ -118,7 +121,9 @@ impl Harness for Codex { // launches that are the point. (Codex has a real first-class Plan mode, // but only over `app-server`/the TUI — `codex exec` doesn't honor it; // verified `-c collaboration_mode="plan"` is accepted but ignored.) - // * Auto — workspace-write (the balanced default). + // * Auto — workspace-write, plus network and the hub clone's + // `.git` (see `run_turn`), since exec can't approve its way past + // either denial the way the TUI can. // * Bypass— full access (`--dangerously-bypass-approvals-and-sandbox`). HarnessOptions::none() .with_permission_modes( @@ -159,6 +164,52 @@ fn codex_sandbox(mode: Option) -> Option<&'static str> { } } +/// The workspace's git dir (`git rev-parse --git-common-dir`), canonicalized. +/// Codex's `workspace-write` sandbox denies every git metadata write: it marks +/// each writable root's `.git` read-only, and a *worktree's* real metadata +/// (refs, objects, `FETCH_HEAD` under `.git/worktrees//`) lives in the +/// parent clone, outside the workspace entirely — orx session repos are always +/// worktrees of the hub clone. Interactively both denials escalate to approval +/// prompts; `codex exec` has none, so `git fetch`/`commit` just dies with +/// "Operation not permitted". Declaring the common dir as an explicit writable +/// root fixes both shapes — an explicit root beats the built-in `.git` +/// protection (verified against codex-cli 0.144 via `codex sandbox`, plain +/// clone and worktree). Canonicalized because codex requires absolute roots +/// and seatbelt matches real paths (`/var` vs `/private/var`). +async fn shared_git_dir(workspace: &Path) -> Option { + let out = Command::new("git") + .args(["rev-parse", "--git-common-dir"]) + .current_dir(workspace) + .stdin(Stdio::null()) + .output() + .await + .ok()?; + if !out.status.success() { + return None; + } + let line = String::from_utf8_lossy(&out.stdout).trim().to_string(); + if line.is_empty() { + return None; + } + absolute_git_dir(workspace, Path::new(&line)) +} + +/// `dir` as an absolute, symlink-free path; `git rev-parse` answers relative +/// to the workspace for a regular clone (`.git`) and absolute for a worktree. +fn absolute_git_dir(workspace: &Path, dir: &Path) -> Option { + workspace.join(dir).canonicalize().ok() +} + +/// A path as a TOML basic-string literal, for `-c key="value"` overrides. +/// serde_json's escaping emits only sequences TOML also accepts (`\"`, `\\`, +/// control chars as `\uXXXX`) and leaves `/` literal — except DEL (0x7F), +/// which serde_json passes through and TOML forbids unescaped. +fn toml_string(path: &Path) -> String { + serde_json::to_string(&path.to_string_lossy()) + .unwrap_or_else(|_| String::from("\"\"")) + .replace('\u{7f}', "\\u007F") +} + /// Session reasoning id → Codex `model_reasoning_effort` value. The composer only /// offers ids from `CODEX_REASONING_LEVELS`; an unrecognized/absent value omits /// the override and lets Codex apply its configured default. @@ -224,6 +275,33 @@ async fn run_turn(ctx: &mut TurnCtx) -> Result<()> { "-c", "approval_policy=\"never\"", ]); + // workspace-write out of the box is too tight for the orx + // workflow in two ways (both verified via `codex sandbox` against + // codex-cli 0.144; in the TUI both denials escalate to approval + // prompts, which `codex exec` doesn't have): + // * Network is blocked by default — DNS doesn't even resolve, so + // `git fetch`/`push`, package installs, and the `orx` CLI's + // localhost API calls all die. The agent's job is launching + // experiments over that API; Auto must keep the network open. + // * Git metadata isn't writable — codex protects `.git` inside + // the workspace, and a worktree's real metadata (the hub + // clone's `.git`) sits outside it — so `git fetch`/`commit` + // fail outright; grant the common dir (see `shared_git_dir`). + // Note `-c` *replaces* any `writable_roots` from the user's + // config.toml for the turn (there is no append form; `exec + // --add-dir` is unverified on the resume path). + if policy == "workspace-write" { + cmd.args(["-c", "sandbox_workspace_write.network_access=true"]); + if let Some(git_dir) = shared_git_dir(&repo).await { + cmd.args([ + "-c", + &format!( + "sandbox_workspace_write.writable_roots=[{}]", + toml_string(&git_dir) + ), + ]); + } + } } None => { cmd.arg("--dangerously-bypass-approvals-and-sandbox"); @@ -452,6 +530,41 @@ mod tests { assert_eq!(codex_sandbox(None), Some("workspace-write")); } + #[test] + fn git_dir_resolves_relative_and_absolute_rev_parse_answers() { + let base = std::env::temp_dir().join(format!("orx-codex-test-{}", std::process::id())); + let workspace = base.join("worktree"); + let hub_git = base.join("hub").join(".git"); + std::fs::create_dir_all(workspace.join(".git")).unwrap(); + std::fs::create_dir_all(&hub_git).unwrap(); + + // Worktree: rev-parse answers with the hub clone's absolute path. + assert_eq!( + absolute_git_dir(&workspace, &hub_git), + Some(hub_git.canonicalize().unwrap()) + ); + // Regular clone: rev-parse answers `.git`, relative to the workspace. + assert_eq!( + absolute_git_dir(&workspace, Path::new(".git")), + Some(workspace.join(".git").canonicalize().unwrap()) + ); + // No git dir at all → no writable root (flag omitted, fail-safe). + assert_eq!(absolute_git_dir(&workspace, Path::new("missing")), None); + + std::fs::remove_dir_all(&base).ok(); + } + + #[test] + fn toml_string_quotes_and_escapes_paths() { + assert_eq!( + toml_string(Path::new("/a/with space")), + r#""/a/with space""# + ); + assert_eq!(toml_string(Path::new(r#"/a/"q""#)), r#""/a/\"q\"""#); + // DEL is the one char serde_json leaves raw that TOML rejects. + assert_eq!(toml_string(Path::new("/a/\u{7f}b")), r#""/a/\u007Fb""#); + } + #[test] fn reasoning_accepts_only_codex_ids() { assert_eq!(codex_reasoning(Some("low")), Some("low")); diff --git a/src/local/harness/detect.rs b/src/local/harness/detect.rs index 8a1ea882..6bfd002d 100644 --- a/src/local/harness/detect.rs +++ b/src/local/harness/detect.rs @@ -85,6 +85,17 @@ pub(super) fn find_on_path(bin: &str) -> Option { .find(|c| c.is_file()) } +/// Dereference symlinks to the real installed binary. Installers commonly drop +/// a lone symlink into `~/.local/bin`, but some CLIs locate sibling helper +/// executables relative to the path they were *invoked as*, without resolving +/// symlinks — codex >= 0.144 launches `codex-code-mode-host` this way and every +/// command fails with "No such file or directory" when codex is spawned via the +/// symlink. Spawning the resolved path keeps helpers real siblings. Best-effort: +/// a path that can't be resolved is returned unchanged. +pub(super) fn resolve_symlinks(path: PathBuf) -> PathBuf { + path.canonicalize().unwrap_or(path) +} + /// ` --version`, first line, with a timeout (node CLIs can be slow). pub(super) async fn bin_version(bin: &PathBuf) -> Option { let fut = tokio::process::Command::new(bin) @@ -137,3 +148,32 @@ pub(super) fn title_case(word: &str) -> String { None => String::new(), } } + +#[cfg(test)] +mod tests { + use super::*; + + #[cfg(unix)] + #[test] + fn resolve_symlinks_dereferences_to_real_binary() { + let dir = std::env::temp_dir().join(format!("orx-detect-test-{}", std::process::id())); + let install = dir.join("install"); + let bin = dir.join("bin"); + std::fs::create_dir_all(&install).unwrap(); + std::fs::create_dir_all(&bin).unwrap(); + let real = install.join("codex"); + std::fs::write(&real, "").unwrap(); + let link = bin.join("codex"); + std::os::unix::fs::symlink(&real, &link).unwrap(); + + assert_eq!(resolve_symlinks(link), real.canonicalize().unwrap()); + + std::fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn resolve_symlinks_keeps_unresolvable_path() { + let missing = PathBuf::from("/nonexistent/orx-detect-test/codex"); + assert_eq!(resolve_symlinks(missing.clone()), missing); + } +}