diff --git a/codex-rs/cli/src/seatbelt.rs b/codex-rs/cli/src/seatbelt.rs index 6c49d8cc7e..3c7ec2ba93 100644 --- a/codex-rs/cli/src/seatbelt.rs +++ b/codex-rs/cli/src/seatbelt.rs @@ -5,7 +5,8 @@ pub async fn run_seatbelt( command: Vec, sandbox_policy: SandboxPolicy, ) -> anyhow::Result<()> { - let seatbelt_command = create_seatbelt_command(command, &sandbox_policy); + let cwd = std::env::current_dir().expect("failed to get cwd"); + let seatbelt_command = create_seatbelt_command(command, &sandbox_policy, &cwd); let status = tokio::process::Command::new(seatbelt_command[0].clone()) .args(&seatbelt_command[1..]) .spawn() diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index da2c62888d..813f9c9797 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -16,6 +16,7 @@ use codex_apply_patch::ApplyPatchFileChange; use codex_apply_patch::MaybeApplyPatchVerified; use fs_err as fs; use futures::prelude::*; +use serde::Deserialize; use serde::Serialize; use serde_json; use tokio::sync::oneshot; @@ -190,6 +191,11 @@ struct Session { tx_event: Sender, ctrl_c: Arc, + /// The session's current working directory. All relative paths provided by + /// the model as well as sandbox policies are resolved against this path + /// instead of `std::env::current_dir()`. + cwd: PathBuf, + instructions: Option, approval_policy: AskForApproval, sandbox_policy: SandboxPolicy, @@ -202,6 +208,14 @@ struct Session { state: Mutex, } +impl Session { + fn resolve_path(&self, path: Option) -> PathBuf { + path.as_ref() + .map(PathBuf::from) + .map_or_else(|| self.cwd.clone(), |p| self.cwd.join(p)) + } +} + /// Mutable state of the agent #[derive(Default)] struct State { @@ -296,21 +310,14 @@ impl Session { sub_id: &str, call_id: &str, command: Vec, - cwd: Option, + workdir: PathBuf, ) { - let cwd = cwd - .or_else(|| { - std::env::current_dir() - .ok() - .map(|p| p.to_string_lossy().to_string()) - }) - .unwrap_or_else(|| "".to_string()); let event = Event { id: sub_id.to_string(), msg: EventMsg::ExecCommandBegin { call_id: call_id.to_string(), command, - cwd, + cwd: workdir.to_string_lossy().into(), }, }; let _ = self.tx_event.send(event).await; @@ -518,6 +525,7 @@ async fn submission_loop( sandbox_policy, disable_response_storage, notify, + cwd, } => { info!(model, "Configuring session"); let client = ModelClient::new(model.clone()); @@ -539,6 +547,13 @@ async fn submission_loop( }; // update session + // Session working directory – canonicalise so comparisons and + // path joins behave consistently. + let cwd_path = match cwd.canonicalize() { + Ok(p) => p, + Err(_) => cwd.clone(), + }; + sess = Some(Arc::new(Session { client, tx_event: tx_event.clone(), @@ -546,7 +561,8 @@ async fn submission_loop( instructions, approval_policy, sandbox_policy, - writable_roots: Mutex::new(get_writable_roots()), + writable_roots: Mutex::new(get_writable_roots(&cwd_path)), + cwd: cwd_path, notify, state: Mutex::new(state), })); @@ -855,6 +871,18 @@ async fn handle_response_item( Ok(output) } +#[derive(Deserialize, Debug, Clone)] +pub struct ShellToolCallParams { + pub command: Vec, + pub workdir: Option, + + /// This is the maximum time in seconds that the command is allowed to run. + #[serde(rename = "timeout")] + // The wire format uses `timeout`, which has ambiguous units, so we use + // `timeout_ms` as the field name so it is clear in code. + pub timeout_ms: Option, +} + async fn handle_function_call( sess: &Session, sub_id: String, @@ -865,7 +893,7 @@ async fn handle_function_call( match name.as_str() { "container.exec" | "shell" => { // parse command - let params = match serde_json::from_str::(&arguments) { + let params = match serde_json::from_str::(&arguments) { Ok(v) => v, Err(e) => { // allow model to re-sample @@ -904,12 +932,7 @@ async fn handle_function_call( } // this was not a valid patch, execute command - let repo_root = std::env::current_dir().expect("no current dir"); - let workdir: PathBuf = params - .workdir - .as_ref() - .map(PathBuf::from) - .unwrap_or(repo_root.clone()); + let workdir = sess.resolve_path(params.workdir.clone()); // safety checks let safety = { @@ -968,12 +991,16 @@ async fn handle_function_call( &sub_id, &call_id, params.command.clone(), - params.workdir.clone(), + workdir.clone(), ) .await; let output_result = process_exec_tool_call( - params.clone(), + ExecParams { + command: params.command.clone(), + cwd: workdir.clone(), + timeout_ms: params.timeout_ms, + }, sandbox_type, sess.ctrl_c.clone(), &sess.sandbox_policy, @@ -1051,18 +1078,23 @@ async fn handle_function_call( // Emit a fresh Begin event so progress bars reset. let retry_call_id = format!("{call_id}-retry"); + let cwd = sess.resolve_path(params.workdir.clone()); sess.notify_exec_command_begin( &sub_id, &retry_call_id, params.command.clone(), - params.workdir.clone(), + cwd.clone(), ) .await; // This is an escalated retry; the policy will not be // examined and the sandbox has been set to `None`. let retry_output_result = process_exec_tool_call( - params.clone(), + ExecParams { + command: params.command.clone(), + cwd: cwd.clone(), + timeout_ms: params.timeout_ms, + }, SandboxType::None, sess.ctrl_c.clone(), &sess.sandbox_policy, @@ -1162,43 +1194,47 @@ async fn apply_patch( guard.clone() }; - let auto_approved = - match assess_patch_safety(&changes, sess.approval_policy, &writable_roots_snapshot) { - SafetyCheck::AutoApprove { .. } => true, - SafetyCheck::AskUser => { - // Compute a readable summary of path changes to include in the - // approval request so the user can make an informed decision. - let rx_approve = sess - .request_patch_approval(sub_id.clone(), &changes, None, None) - .await; - match rx_approve.await.unwrap_or_default() { - ReviewDecision::Approved | ReviewDecision::ApprovedForSession => false, - ReviewDecision::Denied | ReviewDecision::Abort => { - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: "patch rejected by user".to_string(), - success: Some(false), - }, - }; - } + let auto_approved = match assess_patch_safety( + &changes, + sess.approval_policy, + &writable_roots_snapshot, + &sess.cwd, + ) { + SafetyCheck::AutoApprove { .. } => true, + SafetyCheck::AskUser => { + // Compute a readable summary of path changes to include in the + // approval request so the user can make an informed decision. + let rx_approve = sess + .request_patch_approval(sub_id.clone(), &changes, None, None) + .await; + match rx_approve.await.unwrap_or_default() { + ReviewDecision::Approved | ReviewDecision::ApprovedForSession => false, + ReviewDecision::Denied | ReviewDecision::Abort => { + return ResponseInputItem::FunctionCallOutput { + call_id, + output: FunctionCallOutputPayload { + content: "patch rejected by user".to_string(), + success: Some(false), + }, + }; } } - SafetyCheck::Reject { reason } => { - return ResponseInputItem::FunctionCallOutput { - call_id, - output: FunctionCallOutputPayload { - content: format!("patch rejected: {reason}"), - success: Some(false), - }, - }; - } - }; + } + SafetyCheck::Reject { reason } => { + return ResponseInputItem::FunctionCallOutput { + call_id, + output: FunctionCallOutputPayload { + content: format!("patch rejected: {reason}"), + success: Some(false), + }, + }; + } + }; // Verify write permissions before touching the filesystem. let writable_snapshot = { sess.writable_roots.lock().unwrap().clone() }; - if let Some(offending) = first_offending_path(&changes, &writable_snapshot) { + if let Some(offending) = first_offending_path(&changes, &writable_snapshot, &sess.cwd) { let root = offending.parent().unwrap_or(&offending).to_path_buf(); let reason = Some(format!( @@ -1255,11 +1291,13 @@ async fn apply_patch( ApplyPatchFileChange::Update { .. } => path, }; - // Reuse safety normalisation logic: treat absolute path. + // Reuse safety normalization logic: treat absolute path. let abs = if path_ref.is_absolute() { path_ref.clone() } else { - std::env::current_dir().unwrap_or_default().join(path_ref) + // TODO(mbolin): If workdir was supplied with apply_patch call, + // relative paths should be resolved against it. + sess.cwd.join(path_ref) }; let writable = { @@ -1345,9 +1383,8 @@ async fn apply_patch( fn first_offending_path( changes: &HashMap, writable_roots: &[PathBuf], + cwd: &Path, ) -> Option { - let cwd = std::env::current_dir().unwrap_or_default(); - for (path, change) in changes { let candidate = match change { ApplyPatchFileChange::Add { .. } => path, @@ -1485,7 +1522,7 @@ fn apply_changes_from_apply_patch( }) } -fn get_writable_roots() -> Vec { +fn get_writable_roots(cwd: &Path) -> Vec { let mut writable_roots = Vec::new(); if cfg!(target_os = "macos") { // On macOS, $TMPDIR is private to the user. @@ -1507,9 +1544,7 @@ fn get_writable_roots() -> Vec { } } - if let Ok(cwd) = std::env::current_dir() { - writable_roots.push(cwd); - } + writable_roots.push(cwd.to_path_buf()); writable_roots } diff --git a/codex-rs/core/src/codex_wrapper.rs b/codex-rs/core/src/codex_wrapper.rs index 223b051d5c..1481a01999 100644 --- a/codex-rs/core/src/codex_wrapper.rs +++ b/codex-rs/core/src/codex_wrapper.rs @@ -26,6 +26,7 @@ pub async fn init_codex(config: Config) -> anyhow::Result<(CodexWrapper, Event, sandbox_policy: config.sandbox_policy, disable_response_storage: config.disable_response_storage, notify: config.notify.clone(), + cwd: config.cwd.clone(), }) .await?; diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 0ab77ada8d..1557ce2752 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -52,6 +52,11 @@ pub struct Config { /// /// If unset the feature is disabled. pub notify: Option>, + + /// The directory that should be treated as the current working directory + /// for the session. All relative paths inside the business-logic layer are + /// resolved against this path. + pub cwd: PathBuf, } /// Base config deserialized from ~/.codex/config.toml. @@ -135,6 +140,7 @@ where #[derive(Default, Debug, Clone)] pub struct ConfigOverrides { pub model: Option, + pub cwd: Option, pub approval_policy: Option, pub sandbox_policy: Option, pub disable_response_storage: Option, @@ -158,6 +164,7 @@ impl Config { // Destructure ConfigOverrides fully to ensure all overrides are applied. let ConfigOverrides { model, + cwd, approval_policy, sandbox_policy, disable_response_storage, @@ -180,6 +187,23 @@ impl Config { Self { model: model.or(cfg.model).unwrap_or_else(default_model), + cwd: cwd.map_or_else( + || { + tracing::info!("cwd not set, using current dir"); + std::env::current_dir().expect("cannot determine current dir") + }, + |p| { + if p.is_absolute() { + p + } else { + // Resolve relative paths against the current working directory. + tracing::info!("cwd is relative, resolving against current dir"); + let mut cwd = std::env::current_dir().expect("cannot determine cwd"); + cwd.push(p); + cwd + } + }, + ), approval_policy: approval_policy .or(cfg.approval_policy) .unwrap_or_else(AskForApproval::default), diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index cf5fbd618c..4e69bbe4cc 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -1,13 +1,14 @@ use std::io; #[cfg(target_family = "unix")] use std::os::unix::process::ExitStatusExt; +use std::path::Path; +use std::path::PathBuf; use std::process::ExitStatus; use std::process::Stdio; use std::sync::Arc; use std::time::Duration; use std::time::Instant; -use serde::Deserialize; use tokio::io::AsyncRead; use tokio::io::AsyncReadExt; use tokio::io::BufReader; @@ -40,15 +41,10 @@ const MACOS_SEATBELT_BASE_POLICY: &str = include_str!("seatbelt_base_policy.sbpl /// already has root access. const MACOS_PATH_TO_SEATBELT_EXECUTABLE: &str = "/usr/bin/sandbox-exec"; -#[derive(Deserialize, Debug, Clone)] +#[derive(Debug, Clone)] pub struct ExecParams { pub command: Vec, - pub workdir: Option, - - /// This is the maximum time in seconds that the command is allowed to run. - #[serde(rename = "timeout")] - // The wire format uses `timeout`, which has ambiguous units, so we use - // `timeout_ms` as the field name so it is clear in code. + pub cwd: PathBuf, pub timeout_ms: Option, } @@ -69,7 +65,7 @@ async fn exec_linux( ctrl_c: Arc, sandbox_policy: &SandboxPolicy, ) -> Result { - crate::linux::exec_linux(params, ctrl_c, sandbox_policy).await + crate::linux::exec_linux(params, ctrl_c, sandbox_policy, cwd).await } #[cfg(not(target_os = "linux"))] @@ -97,14 +93,14 @@ pub async fn process_exec_tool_call( SandboxType::MacosSeatbelt => { let ExecParams { command, - workdir, + cwd, timeout_ms, } = params; - let seatbelt_command = create_seatbelt_command(command, sandbox_policy); + let seatbelt_command = create_seatbelt_command(command, sandbox_policy, &cwd); exec( ExecParams { command: seatbelt_command, - workdir, + cwd, timeout_ms, }, ctrl_c, @@ -157,6 +153,7 @@ pub async fn process_exec_tool_call( pub fn create_seatbelt_command( command: Vec, sandbox_policy: &SandboxPolicy, + cwd: &Path, ) -> Vec { let (file_write_policy, extra_cli_args) = { if sandbox_policy.has_full_disk_write_access() { @@ -166,7 +163,7 @@ pub fn create_seatbelt_command( Vec::::new(), ) } else { - let writable_roots = sandbox_policy.get_writable_roots(); + let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd); let (writable_folder_policies, cli_args): (Vec, Vec) = writable_roots .iter() .enumerate() @@ -234,7 +231,7 @@ pub struct ExecToolCallOutput { pub async fn exec( ExecParams { command, - workdir, + cwd, timeout_ms, }: ExecParams, ctrl_c: Arc, @@ -251,9 +248,7 @@ pub async fn exec( if command.len() > 1 { cmd.args(&command[1..]); } - if let Some(dir) = &workdir { - cmd.current_dir(dir); - } + cmd.current_dir(cwd); // Do not create a file descriptor for stdin because otherwise some // commands may hang forever waiting for input. For example, ripgrep has diff --git a/codex-rs/core/src/linux.rs b/codex-rs/core/src/linux.rs index fac3ab3032..00feff1bec 100644 --- a/codex-rs/core/src/linux.rs +++ b/codex-rs/core/src/linux.rs @@ -30,6 +30,8 @@ use seccompiler::SeccompRule; use seccompiler::TargetArch; use tokio::sync::Notify; +use std::path::Path; + pub async fn exec_linux( params: ExecParams, ctrl_c: Arc, @@ -39,6 +41,7 @@ pub async fn exec_linux( // Allow WRITE on /dev/null let ctrl_c_copy = ctrl_c.clone(); let sandbox_policy = sandbox_policy.clone(); + let cwd_buf = cwd.to_path_buf(); // Isolate thread to run the sandbox from let tool_call_output = std::thread::spawn(move || { @@ -48,7 +51,7 @@ pub async fn exec_linux( .expect("Failed to create runtime"); rt.block_on(async { - apply_sandbox_policy_to_current_thread(sandbox_policy)?; + apply_sandbox_policy_to_current_thread(sandbox_policy, &cwd_buf)?; exec(params, ctrl_c_copy).await }) }) @@ -66,13 +69,16 @@ pub async fn exec_linux( /// Apply sandbox policies inside this thread so only the child inherits /// them, not the entire CLI process. -pub fn apply_sandbox_policy_to_current_thread(sandbox_policy: SandboxPolicy) -> Result<()> { +pub fn apply_sandbox_policy_to_current_thread( + sandbox_policy: SandboxPolicy, + cwd: &Path, +) -> Result<()> { if !sandbox_policy.has_full_network_access() { install_network_seccomp_filter_on_current_thread()?; } if !sandbox_policy.has_full_disk_write_access() { - let writable_roots = sandbox_policy.get_writable_roots(); + let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd); install_filesystem_landlock_rules_on_current_thread(writable_roots)?; } diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index d19a538689..12447d23b6 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -4,6 +4,7 @@ //! between user and agent. use std::collections::HashMap; +use std::path::Path; use std::path::PathBuf; use serde::Deserialize; @@ -43,6 +44,15 @@ pub enum Op { #[serde(skip_serializing_if = "Option::is_none")] #[serde(default)] notify: Option>, + + /// Working directory that should be treated as the *root* of the + /// session. All relative paths supplied by the model as well as the + /// execution sandbox are resolved against this directory **instead** + /// of the process-wide current working directory. CLI front-ends are + /// expected to expand this to an absolute path before sending the + /// `ConfigureSession` operation so that the business-logic layer can + /// operate deterministically. + cwd: std::path::PathBuf, }, /// Abort current task. @@ -157,7 +167,7 @@ impl SandboxPolicy { .any(|perm| matches!(perm, SandboxPermission::NetworkFullAccess)) } - pub fn get_writable_roots(&self) -> Vec { + pub fn get_writable_roots_with_cwd(&self, cwd: &Path) -> Vec { let mut writable_roots = Vec::::new(); for perm in &self.permissions { use SandboxPermission::*; @@ -193,12 +203,9 @@ impl SandboxPolicy { writable_roots.push(PathBuf::from("/tmp")); } } - DiskWriteCwd => match std::env::current_dir() { - Ok(cwd) => writable_roots.push(cwd), - Err(err) => { - tracing::error!("Failed to get current working directory: {err}"); - } - }, + DiskWriteCwd => { + writable_roots.push(cwd.to_path_buf()); + } DiskWriteFolder { folder } => { writable_roots.push(folder.clone()); } diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index 50ed3573df..3d98be6ccd 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -22,6 +22,7 @@ pub fn assess_patch_safety( changes: &HashMap, policy: AskForApproval, writable_roots: &[PathBuf], + cwd: &Path, ) -> SafetyCheck { if changes.is_empty() { return SafetyCheck::Reject { @@ -40,7 +41,7 @@ pub fn assess_patch_safety( } } - if is_write_patch_constrained_to_writable_paths(changes, writable_roots) { + if is_write_patch_constrained_to_writable_paths(changes, writable_roots, cwd) { SafetyCheck::AutoApprove { sandbox_type: SandboxType::None, } @@ -115,6 +116,7 @@ pub fn get_platform_sandbox() -> Option { fn is_write_patch_constrained_to_writable_paths( changes: &HashMap, writable_roots: &[PathBuf], + cwd: &Path, ) -> bool { // Early‑exit if there are no declared writable roots. if writable_roots.is_empty() { @@ -141,11 +143,6 @@ fn is_write_patch_constrained_to_writable_paths( // and roots are converted to absolute, normalized forms before the // prefix check. let is_path_writable = |p: &PathBuf| { - let cwd = match std::env::current_dir() { - Ok(cwd) => cwd, - Err(_) => return false, - }; - let abs = if p.is_absolute() { p.clone() } else { @@ -217,19 +214,22 @@ mod tests { assert!(is_write_patch_constrained_to_writable_paths( &add_inside, - &[PathBuf::from(".")] + &[PathBuf::from(".")], + &cwd, )); let add_outside_2 = make_add_change(parent.join("outside.txt")); assert!(!is_write_patch_constrained_to_writable_paths( &add_outside_2, - &[PathBuf::from(".")] + &[PathBuf::from(".")], + &cwd, )); // With parent dir added as writable root, it should pass. assert!(is_write_patch_constrained_to_writable_paths( &add_outside, - &[PathBuf::from("..")] + &[PathBuf::from("..")], + &cwd, )) } } diff --git a/codex-rs/core/tests/live_agent.rs b/codex-rs/core/tests/live_agent.rs index b780a28715..596e8e6ced 100644 --- a/codex-rs/core/tests/live_agent.rs +++ b/codex-rs/core/tests/live_agent.rs @@ -58,6 +58,7 @@ async fn spawn_codex() -> Codex { sandbox_policy: SandboxPolicy::new_read_only_policy(), disable_response_storage: false, notify: None, + cwd: std::env::current_dir().unwrap(), }, }) .await diff --git a/codex-rs/core/tests/previous_response_id.rs b/codex-rs/core/tests/previous_response_id.rs index 9410f7b5ff..830cda09b6 100644 --- a/codex-rs/core/tests/previous_response_id.rs +++ b/codex-rs/core/tests/previous_response_id.rs @@ -98,6 +98,7 @@ async fn keeps_previous_response_id_between_tasks() { sandbox_policy: SandboxPolicy::new_read_only_policy(), disable_response_storage: false, notify: None, + cwd: std::env::current_dir().unwrap(), }, }) .await diff --git a/codex-rs/core/tests/stream_no_completed.rs b/codex-rs/core/tests/stream_no_completed.rs index 858850f947..adadd079e7 100644 --- a/codex-rs/core/tests/stream_no_completed.rs +++ b/codex-rs/core/tests/stream_no_completed.rs @@ -81,6 +81,7 @@ async fn retries_on_early_close() { sandbox_policy: SandboxPolicy::new_read_only_policy(), disable_response_storage: false, notify: None, + cwd: std::env::current_dir().unwrap(), }, }) .await diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 1541102e32..f8b99f111a 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -81,6 +81,7 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { } else { None }, + cwd: None, }; let config = Config::load_with_overrides(overrides)?; let (codex_wrapper, event, ctrl_c) = codex_wrapper::init_codex(config).await?; diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index e23b8c6902..d12e2990d8 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -56,6 +56,7 @@ pub fn run_main(cli: Cli) -> std::io::Result<()> { } else { None }, + cwd: None, }; #[allow(clippy::print_stderr)] match Config::load_with_overrides(overrides) {