Skip to content

Commit 70e3daf

Browse files
committed
fix: ensure cwd for conversation and sandbox are separate concerns
1 parent 7103838 commit 70e3daf

File tree

12 files changed

+209
-36
lines changed

12 files changed

+209
-36
lines changed

codex-rs/cli/src/debug_sandbox.rs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ async fn run_command_under_sandbox(
6464
sandbox_type: SandboxType,
6565
) -> anyhow::Result<()> {
6666
let sandbox_mode = create_sandbox_mode(full_auto);
67-
let cwd = std::env::current_dir()?;
6867
let config = Config::load_with_cli_overrides(
6968
config_overrides
7069
.parse_overrides()
@@ -75,13 +74,29 @@ async fn run_command_under_sandbox(
7574
..Default::default()
7675
},
7776
)?;
77+
78+
// In practice, this should be `std::env::current_dir()` because this CLI
79+
// does not support `--cwd`, but let's use the config value for consistency.
80+
let cwd = config.cwd.clone();
81+
// For now, we always use the same cwd for both the command and the
82+
// sandbox policy. In the future, we could add a CLI option to set them
83+
// separately.
84+
let sandbox_policy_cwd = cwd.clone();
85+
7886
let stdio_policy = StdioPolicy::Inherit;
7987
let env = create_env(&config.shell_environment_policy);
8088

8189
let mut child = match sandbox_type {
8290
SandboxType::Seatbelt => {
83-
spawn_command_under_seatbelt(command, &config.sandbox_policy, cwd, stdio_policy, env)
84-
.await?
91+
spawn_command_under_seatbelt(
92+
command,
93+
cwd,
94+
&config.sandbox_policy,
95+
sandbox_policy_cwd.as_path(),
96+
stdio_policy,
97+
env,
98+
)
99+
.await?
85100
}
86101
SandboxType::Landlock => {
87102
#[expect(clippy::expect_used)]
@@ -91,8 +106,9 @@ async fn run_command_under_sandbox(
91106
spawn_command_under_linux_sandbox(
92107
codex_linux_sandbox_exe,
93108
command,
94-
&config.sandbox_policy,
95109
cwd,
110+
&config.sandbox_policy,
111+
sandbox_policy_cwd.as_path(),
96112
stdio_policy,
97113
env,
98114
)

codex-rs/core/src/codex.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::borrow::Cow;
22
use std::collections::HashMap;
33
use std::collections::HashSet;
4+
use std::path::Path;
45
use std::path::PathBuf;
56
use std::sync::Arc;
67
use std::sync::atomic::AtomicU64;
@@ -898,6 +899,7 @@ impl Session {
898899
exec_args.params,
899900
exec_args.sandbox_type,
900901
exec_args.sandbox_policy,
902+
exec_args.sandbox_cwd,
901903
exec_args.codex_linux_sandbox_exe,
902904
exec_args.stdout_stream,
903905
)
@@ -2691,6 +2693,7 @@ pub struct ExecInvokeArgs<'a> {
26912693
pub params: ExecParams,
26922694
pub sandbox_type: SandboxType,
26932695
pub sandbox_policy: &'a SandboxPolicy,
2696+
pub sandbox_cwd: &'a Path,
26942697
pub codex_linux_sandbox_exe: &'a Option<PathBuf>,
26952698
pub stdout_stream: Option<StdoutStream>,
26962699
}
@@ -2882,6 +2885,7 @@ async fn handle_container_exec_with_params(
28822885
params: params.clone(),
28832886
sandbox_type,
28842887
sandbox_policy: &turn_context.sandbox_policy,
2888+
sandbox_cwd: &turn_context.cwd,
28852889
codex_linux_sandbox_exe: &sess.codex_linux_sandbox_exe,
28862890
stdout_stream: if exec_command_context.apply_patch.is_some() {
28872891
None
@@ -3016,6 +3020,7 @@ async fn handle_sandbox_error(
30163020
params,
30173021
sandbox_type: SandboxType::None,
30183022
sandbox_policy: &turn_context.sandbox_policy,
3023+
sandbox_cwd: &turn_context.cwd,
30193024
codex_linux_sandbox_exe: &sess.codex_linux_sandbox_exe,
30203025
stdout_stream: if exec_command_context.apply_patch.is_some() {
30213026
None

codex-rs/core/src/exec.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::os::unix::process::ExitStatusExt;
33

44
use std::collections::HashMap;
55
use std::io;
6+
use std::path::Path;
67
use std::path::PathBuf;
78
use std::process::ExitStatus;
89
use std::time::Duration;
@@ -82,6 +83,7 @@ pub async fn process_exec_tool_call(
8283
params: ExecParams,
8384
sandbox_type: SandboxType,
8485
sandbox_policy: &SandboxPolicy,
86+
sandbox_cwd: &Path,
8587
codex_linux_sandbox_exe: &Option<PathBuf>,
8688
stdout_stream: Option<StdoutStream>,
8789
) -> Result<ExecToolCallOutput> {
@@ -94,12 +96,16 @@ pub async fn process_exec_tool_call(
9496
SandboxType::None => exec(params, sandbox_policy, stdout_stream.clone()).await,
9597
SandboxType::MacosSeatbelt => {
9698
let ExecParams {
97-
command, cwd, env, ..
99+
command,
100+
cwd: command_cwd,
101+
env,
102+
..
98103
} = params;
99104
let child = spawn_command_under_seatbelt(
100105
command,
106+
command_cwd,
101107
sandbox_policy,
102-
cwd,
108+
sandbox_cwd,
103109
StdioPolicy::RedirectForShellTool,
104110
env,
105111
)
@@ -108,7 +114,10 @@ pub async fn process_exec_tool_call(
108114
}
109115
SandboxType::LinuxSeccomp => {
110116
let ExecParams {
111-
command, cwd, env, ..
117+
command,
118+
cwd: command_cwd,
119+
env,
120+
..
112121
} = params;
113122

114123
let codex_linux_sandbox_exe = codex_linux_sandbox_exe
@@ -117,8 +126,9 @@ pub async fn process_exec_tool_call(
117126
let child = spawn_command_under_linux_sandbox(
118127
codex_linux_sandbox_exe,
119128
command,
129+
command_cwd,
120130
sandbox_policy,
121-
cwd,
131+
sandbox_cwd,
122132
StdioPolicy::RedirectForShellTool,
123133
env,
124134
)

codex-rs/core/src/landlock.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,22 @@ use tokio::process::Child;
1616
pub async fn spawn_command_under_linux_sandbox<P>(
1717
codex_linux_sandbox_exe: P,
1818
command: Vec<String>,
19+
command_cwd: PathBuf,
1920
sandbox_policy: &SandboxPolicy,
20-
cwd: PathBuf,
21+
sandbox_policy_cwd: &Path,
2122
stdio_policy: StdioPolicy,
2223
env: HashMap<String, String>,
2324
) -> std::io::Result<Child>
2425
where
2526
P: AsRef<Path>,
2627
{
27-
let args = create_linux_sandbox_command_args(command, sandbox_policy, &cwd);
28+
let args = create_linux_sandbox_command_args(command, sandbox_policy, sandbox_policy_cwd);
2829
let arg0 = Some("codex-linux-sandbox");
2930
spawn_child_async(
3031
codex_linux_sandbox_exe.as_ref().to_path_buf(),
3132
args,
3233
arg0,
33-
cwd,
34+
command_cwd,
3435
sandbox_policy,
3536
stdio_policy,
3637
env,
@@ -42,10 +43,13 @@ where
4243
fn create_linux_sandbox_command_args(
4344
command: Vec<String>,
4445
sandbox_policy: &SandboxPolicy,
45-
cwd: &Path,
46+
sandbox_policy_cwd: &Path,
4647
) -> Vec<String> {
4748
#[expect(clippy::expect_used)]
48-
let sandbox_policy_cwd = cwd.to_str().expect("cwd must be valid UTF-8").to_string();
49+
let sandbox_policy_cwd = sandbox_policy_cwd
50+
.to_str()
51+
.expect("cwd must be valid UTF-8")
52+
.to_string();
4953

5054
#[expect(clippy::expect_used)]
5155
let sandbox_policy_json =

codex-rs/core/src/seatbelt.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,20 @@ const MACOS_PATH_TO_SEATBELT_EXECUTABLE: &str = "/usr/bin/sandbox-exec";
1818

1919
pub async fn spawn_command_under_seatbelt(
2020
command: Vec<String>,
21+
command_cwd: PathBuf,
2122
sandbox_policy: &SandboxPolicy,
22-
cwd: PathBuf,
23+
sandbox_policy_cwd: &Path,
2324
stdio_policy: StdioPolicy,
2425
mut env: HashMap<String, String>,
2526
) -> std::io::Result<Child> {
26-
let args = create_seatbelt_command_args(command, sandbox_policy, &cwd);
27+
let args = create_seatbelt_command_args(command, sandbox_policy, sandbox_policy_cwd);
2728
let arg0 = None;
2829
env.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string());
2930
spawn_child_async(
3031
PathBuf::from(MACOS_PATH_TO_SEATBELT_EXECUTABLE),
3132
args,
3233
arg0,
33-
cwd,
34+
command_cwd,
3435
sandbox_policy,
3536
stdio_policy,
3637
env,
@@ -41,7 +42,7 @@ pub async fn spawn_command_under_seatbelt(
4142
fn create_seatbelt_command_args(
4243
command: Vec<String>,
4344
sandbox_policy: &SandboxPolicy,
44-
cwd: &Path,
45+
sandbox_policy_cwd: &Path,
4546
) -> Vec<String> {
4647
let (file_write_policy, extra_cli_args) = {
4748
if sandbox_policy.has_full_disk_write_access() {
@@ -51,7 +52,7 @@ fn create_seatbelt_command_args(
5152
Vec::<String>::new(),
5253
)
5354
} else {
54-
let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd);
55+
let writable_roots = sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd);
5556

5657
let mut writable_folder_policies: Vec<String> = Vec::new();
5758
let mut cli_args: Vec<String> = Vec::new();

codex-rs/core/src/shell.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,7 @@ mod tests {
349349
},
350350
SandboxType::None,
351351
&SandboxPolicy::DangerFullAccess,
352+
temp_home.path(),
352353
&None,
353354
None,
354355
)
@@ -455,6 +456,7 @@ mod macos_tests {
455456
},
456457
SandboxType::None,
457458
&SandboxPolicy::DangerFullAccess,
459+
temp_home.path(),
458460
&None,
459461
None,
460462
)

codex-rs/core/tests/suite/exec.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ async fn run_test_cmd(tmp: TempDir, cmd: Vec<&str>) -> Result<ExecToolCallOutput
3939

4040
let policy = SandboxPolicy::new_read_only_policy();
4141

42-
process_exec_tool_call(params, sandbox_type, &policy, &None, None).await
42+
process_exec_tool_call(params, sandbox_type, &policy, tmp.path(), &None, None).await
4343
}
4444

4545
/// Command succeeds with exit code 0 normally

codex-rs/core/tests/suite/exec_stream_events.rs

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,10 @@ async fn test_exec_stdout_stream_events_echo() {
4949
"printf 'hello-world\n'".to_string(),
5050
];
5151

52+
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
5253
let params = ExecParams {
5354
command: cmd,
54-
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
55+
cwd: cwd.clone(),
5556
timeout_ms: Some(5_000),
5657
env: HashMap::new(),
5758
with_escalated_permissions: None,
@@ -64,6 +65,7 @@ async fn test_exec_stdout_stream_events_echo() {
6465
params,
6566
SandboxType::None,
6667
&policy,
68+
cwd.as_path(),
6769
&None,
6870
Some(stdout_stream),
6971
)
@@ -99,9 +101,10 @@ async fn test_exec_stderr_stream_events_echo() {
99101
"printf 'oops\n' 1>&2".to_string(),
100102
];
101103

104+
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
102105
let params = ExecParams {
103106
command: cmd,
104-
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
107+
cwd: cwd.clone(),
105108
timeout_ms: Some(5_000),
106109
env: HashMap::new(),
107110
with_escalated_permissions: None,
@@ -114,6 +117,7 @@ async fn test_exec_stderr_stream_events_echo() {
114117
params,
115118
SandboxType::None,
116119
&policy,
120+
cwd.as_path(),
117121
&None,
118122
Some(stdout_stream),
119123
)
@@ -152,9 +156,10 @@ async fn test_aggregated_output_interleaves_in_order() {
152156
"printf 'O1\\n'; sleep 0.01; printf 'E1\\n' 1>&2; sleep 0.01; printf 'O2\\n'; sleep 0.01; printf 'E2\\n' 1>&2".to_string(),
153157
];
154158

159+
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
155160
let params = ExecParams {
156161
command: cmd,
157-
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
162+
cwd: cwd.clone(),
158163
timeout_ms: Some(5_000),
159164
env: HashMap::new(),
160165
with_escalated_permissions: None,
@@ -163,9 +168,16 @@ async fn test_aggregated_output_interleaves_in_order() {
163168

164169
let policy = SandboxPolicy::new_read_only_policy();
165170

166-
let result = process_exec_tool_call(params, SandboxType::None, &policy, &None, None)
167-
.await
168-
.expect("process_exec_tool_call");
171+
let result = process_exec_tool_call(
172+
params,
173+
SandboxType::None,
174+
&policy,
175+
cwd.as_path(),
176+
&None,
177+
None,
178+
)
179+
.await
180+
.expect("process_exec_tool_call");
169181

170182
assert_eq!(result.exit_code, 0);
171183
assert_eq!(result.stdout.text, "O1\nO2\n");
@@ -182,9 +194,10 @@ async fn test_exec_timeout_returns_partial_output() {
182194
"printf 'before\\n'; sleep 2; printf 'after\\n'".to_string(),
183195
];
184196

197+
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
185198
let params = ExecParams {
186199
command: cmd,
187-
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
200+
cwd: cwd.clone(),
188201
timeout_ms: Some(200),
189202
env: HashMap::new(),
190203
with_escalated_permissions: None,
@@ -193,7 +206,15 @@ async fn test_exec_timeout_returns_partial_output() {
193206

194207
let policy = SandboxPolicy::new_read_only_policy();
195208

196-
let result = process_exec_tool_call(params, SandboxType::None, &policy, &None, None).await;
209+
let result = process_exec_tool_call(
210+
params,
211+
SandboxType::None,
212+
&policy,
213+
cwd.as_path(),
214+
&None,
215+
None,
216+
)
217+
.await;
197218

198219
let Err(CodexErr::Sandbox(SandboxErr::Timeout { output })) = result else {
199220
panic!("expected timeout error");

codex-rs/core/tests/suite/seatbelt.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,8 @@ async fn python_getpwuid_works_under_seatbelt() {
171171

172172
// ReadOnly is sufficient here since we are only exercising user lookup.
173173
let policy = SandboxPolicy::ReadOnly;
174+
let command_cwd = std::env::current_dir().expect("getcwd");
175+
let sandbox_cwd = command_cwd.clone();
174176

175177
let mut child = spawn_command_under_seatbelt(
176178
vec![
@@ -179,8 +181,9 @@ async fn python_getpwuid_works_under_seatbelt() {
179181
// Print the passwd struct; success implies lookup worked.
180182
"import pwd, os; print(pwd.getpwuid(os.getuid()))".to_string(),
181183
],
184+
command_cwd,
182185
&policy,
183-
std::env::current_dir().expect("should be able to get current dir"),
186+
sandbox_cwd.as_path(),
184187
StdioPolicy::RedirectForShellTool,
185188
HashMap::new(),
186189
)
@@ -216,13 +219,16 @@ fn create_test_scenario(tmp: &TempDir) -> TestScenario {
216219
/// Note that `path` must be absolute.
217220
async fn touch(path: &Path, policy: &SandboxPolicy) -> bool {
218221
assert!(path.is_absolute(), "Path must be absolute: {path:?}");
222+
let command_cwd = std::env::current_dir().expect("getcwd");
223+
let sandbox_cwd = command_cwd.clone();
219224
let mut child = spawn_command_under_seatbelt(
220225
vec![
221226
"/usr/bin/touch".to_string(),
222227
path.to_string_lossy().to_string(),
223228
],
229+
command_cwd,
224230
policy,
225-
std::env::current_dir().expect("should be able to get current dir"),
231+
sandbox_cwd.as_path(),
226232
StdioPolicy::RedirectForShellTool,
227233
HashMap::new(),
228234
)

0 commit comments

Comments
 (0)