diff --git a/codex-rs/common/src/sandbox_summary.rs b/codex-rs/common/src/sandbox_summary.rs index e0e309a9d9..7c49bac59d 100644 --- a/codex-rs/common/src/sandbox_summary.rs +++ b/codex-rs/common/src/sandbox_summary.rs @@ -7,22 +7,25 @@ pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String { SandboxPolicy::WorkspaceWrite { writable_roots, network_access, - include_default_writable_roots, + exclude_tmpdir_env_var, + exclude_slash_tmp, } => { let mut summary = "workspace-write".to_string(); - if !writable_roots.is_empty() { - summary.push_str(&format!( - " [{}]", - writable_roots - .iter() - .map(|p| p.to_string_lossy()) - .collect::>() - .join(", ") - )); + + let mut writable_entries = Vec::::new(); + if !*exclude_slash_tmp { + writable_entries.push("/tmp".to_string()); } - if !*include_default_writable_roots { - summary.push_str(" (exact writable roots)"); + if !*exclude_tmpdir_env_var { + writable_entries.push("$TMPDIR".to_string()); } + writable_entries.extend( + writable_roots + .iter() + .map(|p| p.to_string_lossy().to_string()), + ); + + summary.push_str(&format!(" [{}]", writable_entries.join(", "))); if *network_access { summary.push_str(" (network access enabled)"); } diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 63a2e5949f..4970cfd87b 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -364,7 +364,8 @@ impl ConfigToml { Some(s) => SandboxPolicy::WorkspaceWrite { writable_roots: s.writable_roots.clone(), network_access: s.network_access, - include_default_writable_roots: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, }, None => SandboxPolicy::new_workspace_write_policy(), }, @@ -756,7 +757,8 @@ writable_roots = [ SandboxPolicy::WorkspaceWrite { writable_roots: vec![PathBuf::from("/tmp")], network_access: false, - include_default_writable_roots: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, }, sandbox_workspace_write_cfg.derive_sandbox_policy(sandbox_mode_override) ); diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index b141e42aa5..546665b302 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -185,11 +185,16 @@ pub enum SandboxPolicy { #[serde(default)] network_access: bool, - /// When set to `true`, will include defaults like the current working - /// directory and TMPDIR (on macOS). When `false`, only `writable_roots` - /// are used. (Mainly used for testing.) - #[serde(default = "default_true")] - include_default_writable_roots: bool, + /// When set to `true`, will NOT include the per-user `TMPDIR` + /// environment variable among the default writable roots. Defaults to + /// `false`. + #[serde(default)] + exclude_tmpdir_env_var: bool, + + /// When set to `true`, will NOT include the `/tmp` among the default + /// writable roots on UNIX. Defaults to `false`. + #[serde(default)] + exclude_slash_tmp: bool, }, } @@ -203,10 +208,6 @@ pub struct WritableRoot { pub read_only_subpaths: Vec, } -fn default_true() -> bool { - true -} - impl FromStr for SandboxPolicy { type Err = serde_json::Error; @@ -228,7 +229,8 @@ impl SandboxPolicy { SandboxPolicy::WorkspaceWrite { writable_roots: vec![], network_access: false, - include_default_writable_roots: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, } } @@ -263,27 +265,40 @@ impl SandboxPolicy { SandboxPolicy::ReadOnly => Vec::new(), SandboxPolicy::WorkspaceWrite { writable_roots, - include_default_writable_roots, - .. + exclude_tmpdir_env_var, + exclude_slash_tmp, + network_access: _, } => { // Start from explicitly configured writable roots. let mut roots: Vec = writable_roots.clone(); - // Optionally include defaults (cwd and TMPDIR on macOS). - if *include_default_writable_roots { - roots.push(cwd.to_path_buf()); + // Always include defaults: cwd, /tmp (if present on Unix), and + // on macOS, the per-user TMPDIR unless explicitly excluded. + roots.push(cwd.to_path_buf()); - // Also include the per-user tmp dir on macOS. - // Note this is added dynamically rather than storing it in - // `writable_roots` because `writable_roots` contains only static - // values deserialized from the config file. - if cfg!(target_os = "macos") { - if let Some(tmpdir) = std::env::var_os("TMPDIR") { - roots.push(PathBuf::from(tmpdir)); - } + // Include /tmp on Unix unless explicitly excluded. + if cfg!(unix) && !exclude_slash_tmp { + let slash_tmp = PathBuf::from("/tmp"); + if slash_tmp.is_dir() { + roots.push(slash_tmp); } } + // Include $TMPDIR unless explicitly excluded. On macOS, TMPDIR + // is per-user on macOS, so writes to TMPDIR should not be + // readable by other users on the system. + // + // By comparison, TMPDIR is not guaranteed to be defined on + // Linux or Windows, but supporting it here gives users a way + // to provide the model with their own temporary directory + // without having to hardcode it in the config. + if !*exclude_tmpdir_env_var + && let Some(tmpdir) = std::env::var_os("TMPDIR") + && !tmpdir.is_empty() + { + roots.push(PathBuf::from(tmpdir)); + } + // For each root, compute subpaths that should remain read-only. roots .into_iter() diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index 0364840b1a..b1cebb27dd 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -145,12 +145,13 @@ mod tests { root_without_git_canon, } = populate_tmpdir(tmp.path()); - // Build a policy that only includes the two test roots as writable and - // does not automatically include defaults like cwd or TMPDIR. + // Build a policy that includes the two test roots as writable and also + // includes default writable roots (cwd, /tmp, and possibly TMPDIR). let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![root_with_git.clone(), root_without_git.clone()], network_access: false, - include_default_writable_roots: false, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, }; let args = create_seatbelt_command_args( @@ -159,22 +160,50 @@ mod tests { tmp.path(), ); + // Compute dynamic expected bits for defaults. + let cwd_canon = tmp + .path() + .canonicalize() + .expect("canonicalize tmp cwd") + .to_string_lossy() + .to_string(); + let slash_tmp_canon = PathBuf::from("/tmp") + .canonicalize() + .expect("canonicalize /tmp") + .to_string_lossy() + .to_string(); + let tmpdir_env_var = if cfg!(target_os = "macos") { + std::env::var("TMPDIR") + .ok() + .map(PathBuf::from) + .and_then(|p| p.canonicalize().ok()) + .map(|p| p.to_string_lossy().to_string()) + } else { + None + }; + let tmpdir_policy_entry = if tmpdir_env_var.is_some() { + " (subpath (param \"WRITABLE_ROOT_4\"))" + } else { + "" + }; + // Build the expected policy text using a raw string for readability. // Note that the policy includes: // - the base policy, // - read-only access to the filesystem, - // - write access to WRITABLE_ROOT_0 (but not its .git) and WRITABLE_ROOT_1. + // - write access to WRITABLE_ROOT_0 (but not its .git), WRITABLE_ROOT_1, + // WRITABLE_ROOT_2 (cwd), WRITABLE_ROOT_3 (/tmp), and optionally WRITABLE_ROOT_4 (TMPDIR). let expected_policy = format!( r#"{MACOS_SEATBELT_BASE_POLICY} ; allow read-only file operations (allow file-read*) (allow file-write* -(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) ) (subpath (param "WRITABLE_ROOT_1")) +(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) ) (subpath (param "WRITABLE_ROOT_1")) (subpath (param "WRITABLE_ROOT_2")) (subpath (param "WRITABLE_ROOT_3")){tmpdir_policy_entry} ) "#, ); - let expected_args = vec![ + let mut expected_args = vec![ "-p".to_string(), expected_policy, format!( @@ -189,12 +218,21 @@ mod tests { "-DWRITABLE_ROOT_1={}", root_without_git_canon.to_string_lossy() ), + format!("-DWRITABLE_ROOT_2={cwd_canon}"), + format!("-DWRITABLE_ROOT_3={slash_tmp_canon}"), + ]; + + if let Some(p) = tmpdir_env_var { + expected_args.push(format!("-DWRITABLE_ROOT_4={p}")); + } + + expected_args.extend(vec![ "--".to_string(), "/bin/echo".to_string(), "hello".to_string(), - ]; + ]); - assert_eq!(args, expected_args); + assert_eq!(expected_args, args); } #[test] @@ -215,7 +253,8 @@ mod tests { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![], network_access: false, - include_default_writable_roots: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, }; let args = create_seatbelt_command_args( @@ -234,7 +273,7 @@ mod tests { None }; let tempdir_policy_entry = if tmpdir_env_var.is_some() { - " (subpath (param \"WRITABLE_ROOT_1\"))" + " (subpath (param \"WRITABLE_ROOT_2\"))" } else { "" }; @@ -249,7 +288,7 @@ mod tests { ; allow read-only file operations (allow file-read*) (allow file-write* -(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) ){tempdir_policy_entry} +(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) ) (subpath (param "WRITABLE_ROOT_1")){tempdir_policy_entry} ) "#, ); @@ -265,10 +304,17 @@ mod tests { "-DWRITABLE_ROOT_0_RO_0={}", root_with_git_git_canon.to_string_lossy() ), + format!( + "-DWRITABLE_ROOT_1={}", + PathBuf::from("/tmp") + .canonicalize() + .expect("canonicalize /tmp") + .to_string_lossy() + ), ]; if let Some(p) = tmpdir_env_var { - expected_args.push(format!("-DWRITABLE_ROOT_1={p}")); + expected_args.push(format!("-DWRITABLE_ROOT_2={p}")); } expected_args.extend(vec![ @@ -277,7 +323,7 @@ mod tests { "hello".to_string(), ]); - assert_eq!(args, expected_args); + assert_eq!(expected_args, args); } struct PopulatedTmp { diff --git a/codex-rs/core/tests/sandbox.rs b/codex-rs/core/tests/sandbox.rs index e85156bf05..ae5bdc44a6 100644 --- a/codex-rs/core/tests/sandbox.rs +++ b/codex-rs/core/tests/sandbox.rs @@ -76,7 +76,8 @@ async fn if_parent_of_repo_is_writable_then_dot_git_folder_is_writable() { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![test_scenario.repo_parent.clone()], network_access: false, - include_default_writable_roots: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, }; test_scenario @@ -101,7 +102,8 @@ async fn if_git_repo_is_writable_root_then_dot_git_folder_is_read_only() { let policy = SandboxPolicy::WorkspaceWrite { writable_roots: vec![test_scenario.repo_root.clone()], network_access: false, - include_default_writable_roots: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, }; test_scenario diff --git a/codex-rs/linux-sandbox/tests/landlock.rs b/codex-rs/linux-sandbox/tests/landlock.rs index 041e64e208..d8ffdee2a8 100644 --- a/codex-rs/linux-sandbox/tests/landlock.rs +++ b/codex-rs/linux-sandbox/tests/landlock.rs @@ -51,7 +51,7 @@ async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) { let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: writable_roots.to_vec(), network_access: false, - include_default_writable_roots: true, + exclude_tmpdir_env_var: false, }; let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox"); let codex_linux_sandbox_exe = Some(PathBuf::from(sandbox_program)); diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index f30b980da9..5d877253a6 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -453,6 +453,8 @@ impl ChatComposer { new_text.push_str(&text[end_idx..]); self.textarea.set_text(&new_text); + let new_cursor = start_idx.saturating_add(path.len()).saturating_add(1); + self.textarea.set_cursor(new_cursor); } /// Handle key event when no popup is visible. diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 128e1ae8c1..50fa776ec3 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -485,7 +485,7 @@ impl ChatWidget<'_> { EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id, exit_code, - duration, + duration: _, stdout, stderr, }) => { @@ -498,7 +498,6 @@ impl ChatWidget<'_> { exit_code, stdout, stderr, - duration, }, )); } diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 3bd78ff4e5..7ff9cb8580 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -40,7 +40,6 @@ pub(crate) struct CommandOutput { pub(crate) exit_code: i32, pub(crate) stdout: String, pub(crate) stderr: String, - pub(crate) duration: Duration, } pub(crate) enum PatchEventType { @@ -124,7 +123,7 @@ pub(crate) enum HistoryCell { PatchApplyResult { view: TextBlock }, } -const TOOL_CALL_MAX_LINES: usize = 5; +const TOOL_CALL_MAX_LINES: usize = 3; impl HistoryCell { /// Return a cloned, plain representation of the cell's lines suitable for @@ -234,8 +233,11 @@ impl HistoryCell { let command_escaped = strip_bash_lc_and_escape(&command); let lines: Vec> = vec![ - Line::from(vec!["command".magenta(), " running...".dim()]), - Line::from(format!("$ {command_escaped}")), + Line::from(vec![ + "▌ ".cyan(), + "Running command ".magenta(), + command_escaped.into(), + ]), Line::from(""), ]; @@ -249,34 +251,36 @@ impl HistoryCell { exit_code, stdout, stderr, - duration, } = output; let mut lines: Vec> = Vec::new(); - - // Title depends on whether we have output yet. - let title_line = Line::from(vec![ - "command".magenta(), - format!( - " (code: {}, duration: {})", - exit_code, - format_duration(duration) - ) - .dim(), - ]); - lines.push(title_line); + let command_escaped = strip_bash_lc_and_escape(&command); + lines.push(Line::from(vec![ + "⚡Ran command ".magenta(), + command_escaped.into(), + ])); let src = if exit_code == 0 { stdout } else { stderr }; - let cmdline = strip_bash_lc_and_escape(&command); - lines.push(Line::from(format!("$ {cmdline}"))); let mut lines_iter = src.lines(); - for raw in lines_iter.by_ref().take(TOOL_CALL_MAX_LINES) { - lines.push(ansi_escape_line(raw).dim()); + for (idx, raw) in lines_iter.by_ref().take(TOOL_CALL_MAX_LINES).enumerate() { + let mut line = ansi_escape_line(raw); + let prefix = if idx == 0 { " ⎿ " } else { " " }; + line.spans.insert(0, prefix.into()); + line.spans.iter_mut().for_each(|span| { + span.style = span.style.add_modifier(Modifier::DIM); + }); + lines.push(line); } let remaining = lines_iter.count(); if remaining > 0 { - lines.push(Line::from(format!("... {remaining} additional lines")).dim()); + let mut more = Line::from(format!("... +{remaining} lines")); + // Continuation/ellipsis is treated as a subsequent line for prefixing + more.spans.insert(0, " ".into()); + more.spans.iter_mut().for_each(|span| { + span.style = span.style.add_modifier(Modifier::DIM); + }); + lines.push(more); } lines.push(Line::from(""));