mirror of
https://github.com/openai/codex.git
synced 2026-09-13 11:47:17 +00:00
Propagate workspace roots to exec-server sandboxes (#32214)
## What changed - Pass configured workspace roots from core to the exec server so filesystem and process sandbox permissions are materialized against the intended roots. - Preserve an explicitly empty workspace-root list instead of treating the sandbox working directory as an implicit root. - Initialize filesystem sandbox contexts with their working directory as the default workspace root. ## Testing - Add end-to-end coverage for patch and command writes inside and outside workspace roots. - Verify remote filesystem and process sandboxes do not grant access through an empty workspace-root list. GitOrigin-RevId: 6684c8f7de50970b45a29e2a6323df954c96f9f6
This commit is contained in:
@@ -511,7 +511,11 @@ impl<'a> SandboxAttempt<'a> {
|
||||
exec_request.exec_server_sandbox = Some(FileSystemSandboxContext {
|
||||
permissions: exec_server_permissions.into(),
|
||||
cwd: Some(exec_request.windows_sandbox_policy_cwd.clone()),
|
||||
workspace_roots: Vec::new(),
|
||||
workspace_roots: self
|
||||
.workspace_roots
|
||||
.iter()
|
||||
.map(PathUri::from_abs_path)
|
||||
.collect(),
|
||||
windows_sandbox_level: self.windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: self.windows_sandbox_private_desktop,
|
||||
use_legacy_landlock: self.use_legacy_landlock,
|
||||
|
||||
@@ -264,7 +264,7 @@ fn exec_server_env_keeps_command_native_and_carries_sandbox_context() {
|
||||
Some(codex_exec_server::FileSystemSandboxContext {
|
||||
permissions: exec_server_permissions.clone().into(),
|
||||
cwd: Some(cwd_uri.clone()),
|
||||
workspace_roots: Vec::new(),
|
||||
workspace_roots: vec![cwd_uri.clone()],
|
||||
windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
use_legacy_landlock: false,
|
||||
|
||||
@@ -142,3 +142,4 @@ mod websocket_fallback;
|
||||
mod window_headers;
|
||||
#[cfg(target_os = "windows")]
|
||||
mod windows_sandbox;
|
||||
mod workspace_roots;
|
||||
|
||||
242
codex-rs/core/tests/suite/workspace_roots.rs
Normal file
242
codex-rs/core/tests/suite/workspace_roots.rs
Normal file
@@ -0,0 +1,242 @@
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use codex_exec_server::RemoveOptions;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use core_test_support::TestTargetOs;
|
||||
use core_test_support::responses::ResponseMock;
|
||||
use core_test_support::responses::ev_apply_patch_custom_tool_call;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::mount_sse_sequence;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_target_windows;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::test_target_os;
|
||||
use serde_json::json;
|
||||
use wiremock::MockServer;
|
||||
|
||||
const PATCH_CALL_ID: &str = "workspace-root-patch";
|
||||
const COMMAND_CALL_ID: &str = "workspace-root-command";
|
||||
|
||||
fn workspace_roots_profile() -> PermissionProfile {
|
||||
PermissionProfile::workspace_write_with(
|
||||
&[],
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
/*exclude_tmpdir_env_var*/ true,
|
||||
/*exclude_slash_tmp*/ true,
|
||||
)
|
||||
}
|
||||
|
||||
async fn workspace_roots_test(server: &MockServer) -> Result<TestCodex> {
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
config.use_experimental_unified_exec_tool = true;
|
||||
config
|
||||
.features
|
||||
.enable(Feature::UnifiedExec)
|
||||
.expect("test config should allow feature update");
|
||||
config.workspace_roots = vec![config.cwd.clone()];
|
||||
});
|
||||
builder.build_with_auto_env(server).await
|
||||
}
|
||||
|
||||
fn outside_workspace_path(test: &TestCodex, file_name: &str) -> Result<PathUri> {
|
||||
let file_name = format!("codex-workspace-roots-{}-{file_name}", std::process::id());
|
||||
PathUri::from_abs_path(&test.config.cwd)
|
||||
.parent()
|
||||
.context("test workspace should have a parent")?
|
||||
.join(&file_name)
|
||||
.map_err(Into::into)
|
||||
}
|
||||
|
||||
fn command_arguments(path: &str, contents: &str) -> Result<String> {
|
||||
let (shell, command) = match test_target_os() {
|
||||
TestTargetOs::Linux | TestTargetOs::MacOs => {
|
||||
("bash", format!("printf %s '{contents}' > '{path}'"))
|
||||
}
|
||||
TestTargetOs::Windows => (
|
||||
"powershell",
|
||||
format!("Set-Content -NoNewline -Path '{path}' -Value '{contents}'"),
|
||||
),
|
||||
};
|
||||
Ok(serde_json::to_string(&json!({
|
||||
"cmd": command,
|
||||
"shell": shell,
|
||||
"login": false,
|
||||
}))?)
|
||||
}
|
||||
|
||||
async fn mount_patch_and_command_calls(
|
||||
server: &MockServer,
|
||||
patch: &str,
|
||||
command_path: &str,
|
||||
command_contents: &str,
|
||||
) -> Result<ResponseMock> {
|
||||
let command_arguments = command_arguments(command_path, command_contents)?;
|
||||
Ok(mount_sse_sequence(
|
||||
server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_custom_tool_call(PATCH_CALL_ID, patch),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_function_call(COMMAND_CALL_ID, "exec_command", &command_arguments),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-3"),
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-3"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await)
|
||||
}
|
||||
|
||||
async fn submit_workspace_turn(test: &TestCodex, prompt: &str) -> Result<()> {
|
||||
test.submit_turn_with_permission_profile(prompt, workspace_roots_profile())
|
||||
.await
|
||||
}
|
||||
|
||||
async fn read_file(test: &TestCodex, path: &PathUri) -> Result<String> {
|
||||
Ok(String::from_utf8(
|
||||
test.fs().read_file(path, /*sandbox*/ None).await?,
|
||||
)?)
|
||||
}
|
||||
|
||||
async fn remove_files(test: &TestCodex, paths: &[&PathUri]) -> Result<()> {
|
||||
for path in paths {
|
||||
test.fs()
|
||||
.remove(
|
||||
path,
|
||||
RemoveOptions {
|
||||
recursive: false,
|
||||
force: true,
|
||||
},
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn workspace_roots_allow_file_and_command_writes() -> Result<()> {
|
||||
const PATCH_CONTENTS: &str = "workspace root patch access";
|
||||
const COMMAND_CONTENTS: &str = "workspace root command access";
|
||||
|
||||
skip_if_target_windows!(
|
||||
Ok(()),
|
||||
"sandboxed process launch is not supported by the exec-server Windows backend"
|
||||
);
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let test = workspace_roots_test(&server).await?;
|
||||
let cwd = PathUri::from_abs_path(&test.config.cwd);
|
||||
let patch_path = cwd.join("workspace-root-patch.txt")?;
|
||||
let command_path = cwd.join("workspace-root-command.txt")?;
|
||||
let patch = format!(
|
||||
"*** Begin Patch\n*** Add File: workspace-root-patch.txt\n+{PATCH_CONTENTS}\n*** End Patch\n"
|
||||
);
|
||||
|
||||
let response_mock = mount_patch_and_command_calls(
|
||||
&server,
|
||||
&patch,
|
||||
"workspace-root-command.txt",
|
||||
COMMAND_CONTENTS,
|
||||
)
|
||||
.await?;
|
||||
submit_workspace_turn(&test, "write files inside the workspace roots").await?;
|
||||
|
||||
let request = response_mock
|
||||
.last_request()
|
||||
.context("model should receive both workspace-root tool results")?;
|
||||
let (_, patch_success) = request
|
||||
.custom_tool_call_output_content_and_success(PATCH_CALL_ID)
|
||||
.context("patch result should be present")?;
|
||||
assert_ne!(patch_success, Some(false));
|
||||
|
||||
let (_, command_success) = request
|
||||
.function_call_output_content_and_success(COMMAND_CALL_ID)
|
||||
.context("command result should be present")?;
|
||||
assert_ne!(command_success, Some(false));
|
||||
assert_eq!(
|
||||
read_file(&test, &patch_path).await?,
|
||||
format!("{PATCH_CONTENTS}\n")
|
||||
);
|
||||
assert_eq!(read_file(&test, &command_path).await?, COMMAND_CONTENTS);
|
||||
|
||||
remove_files(&test, &[&patch_path, &command_path]).await
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn workspace_roots_deny_file_and_command_writes_outside_roots() -> Result<()> {
|
||||
const PATCH_CONTENTS: &str = "outside workspace root patch";
|
||||
const COMMAND_CONTENTS: &str = "outside workspace root command";
|
||||
|
||||
skip_if_target_windows!(
|
||||
Ok(()),
|
||||
"sandboxed process launch is not supported by the exec-server Windows backend"
|
||||
);
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let test = workspace_roots_test(&server).await?;
|
||||
let patch_path = outside_workspace_path(&test, "outside-patch.txt")?;
|
||||
let command_path = outside_workspace_path(&test, "outside-command.txt")?;
|
||||
let patch_path_display = patch_path.inferred_native_path_string();
|
||||
let command_path_display = command_path.inferred_native_path_string();
|
||||
let patch = format!(
|
||||
"*** Begin Patch\n*** Add File: {patch_path_display}\n+{PATCH_CONTENTS}\n*** End Patch\n"
|
||||
);
|
||||
|
||||
let response_mock =
|
||||
mount_patch_and_command_calls(&server, &patch, &command_path_display, COMMAND_CONTENTS)
|
||||
.await?;
|
||||
submit_workspace_turn(&test, "try to write files outside the workspace roots").await?;
|
||||
|
||||
let request = response_mock
|
||||
.last_request()
|
||||
.context("model should receive both denied tool results")?;
|
||||
let (patch_output, patch_success) = request
|
||||
.custom_tool_call_output_content_and_success(PATCH_CALL_ID)
|
||||
.context("denied patch result should be present")?;
|
||||
assert_ne!(patch_success, Some(true));
|
||||
assert!(
|
||||
patch_output
|
||||
.as_deref()
|
||||
.is_some_and(|output| output.contains("outside of the project")),
|
||||
"patch should be denied outside the workspace roots, got {patch_output:?}"
|
||||
);
|
||||
|
||||
let (command_output, _) = request
|
||||
.function_call_output_content_and_success(COMMAND_CALL_ID)
|
||||
.context("denied command result should be present")?;
|
||||
let command_output = command_output.context("denied command output should be present")?;
|
||||
assert!(
|
||||
command_output.contains(&command_path_display),
|
||||
"denied command output should identify {command_path_display}, got {command_output:?}"
|
||||
);
|
||||
assert!(
|
||||
test.fs()
|
||||
.read_file(&patch_path, /*sandbox*/ None)
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
assert!(
|
||||
test.fs()
|
||||
.read_file(&command_path, /*sandbox*/ None)
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -72,11 +72,7 @@ impl FileSystemSandboxRunner {
|
||||
.iter()
|
||||
.map(native_workspace_root)
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
let workspace_roots = if native_workspace_roots.is_empty() {
|
||||
std::slice::from_ref(&cwd.native)
|
||||
} else {
|
||||
native_workspace_roots.as_slice()
|
||||
};
|
||||
let workspace_roots = native_workspace_roots.as_slice();
|
||||
let native_permissions: PermissionProfile =
|
||||
sandbox.permissions.clone().try_into().map_err(|err| {
|
||||
invalid_request(format!("invalid sandbox permission path URI: {err}"))
|
||||
|
||||
@@ -56,11 +56,7 @@ pub(crate) fn prepare_exec_request(
|
||||
.iter()
|
||||
.map(|root| native_path(root, "sandbox workspace root"))
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
let workspace_roots = if native_workspace_roots.is_empty() {
|
||||
std::slice::from_ref(&native_sandbox_policy_cwd)
|
||||
} else {
|
||||
native_workspace_roots.as_slice()
|
||||
};
|
||||
let workspace_roots = native_workspace_roots.as_slice();
|
||||
let permissions = permissions.materialize_project_roots_with_workspace_roots(workspace_roots);
|
||||
let managed_mitm_ca_trust_bundle_path = params.managed_network.as_ref().and_then(|_| {
|
||||
CUSTOM_CA_ENV_KEYS.iter().find_map(|key| {
|
||||
|
||||
@@ -13,26 +13,26 @@ use codex_exec_server::ExecOutputStream;
|
||||
use codex_exec_server::ExecParams;
|
||||
use codex_exec_server::ExecProcess;
|
||||
use codex_exec_server::ExecProcessEvent;
|
||||
#[cfg(target_os = "linux")]
|
||||
#[cfg(unix)]
|
||||
use codex_exec_server::FileSystemSandboxContext;
|
||||
use codex_exec_server::ProcessId;
|
||||
use codex_exec_server::ProcessSignal;
|
||||
use codex_exec_server::ReadResponse;
|
||||
use codex_exec_server::StartedExecProcess;
|
||||
use codex_exec_server::WriteStatus;
|
||||
#[cfg(target_os = "linux")]
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
#[cfg(target_os = "linux")]
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
#[cfg(target_os = "linux")]
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
#[cfg(target_os = "linux")]
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
#[cfg(target_os = "linux")]
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
#[cfg(target_os = "linux")]
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
#[cfg(target_os = "linux")]
|
||||
#[cfg(unix)]
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -240,6 +240,56 @@ async fn remote_tty_process_uses_configured_sandbox_helper_with_hostile_path() -
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_process_preserves_empty_workspace_roots() -> Result<()> {
|
||||
if let Some(warning) = codex_sandboxing::system_bwrap_warning(&PermissionProfile::read_only()) {
|
||||
eprintln!("skipping bwrap test: {warning}");
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let context = create_process_context(/*use_remote*/ true).await?;
|
||||
let tmp = TempDir::new()?;
|
||||
let file = tmp.path().join("excluded.txt");
|
||||
std::fs::write(&file, b"excluded")?;
|
||||
let cwd = PathUri::from_host_native_path(tmp.path())?;
|
||||
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
}]);
|
||||
let mut sandbox = FileSystemSandboxContext::from_permission_profile_with_cwd(
|
||||
PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted),
|
||||
cwd.clone(),
|
||||
);
|
||||
sandbox.workspace_roots.clear();
|
||||
|
||||
let session = context
|
||||
.backend
|
||||
.start(ExecParams {
|
||||
process_id: ProcessId::from("proc-empty-workspace-roots"),
|
||||
argv: vec!["/bin/cat".to_string(), file.to_string_lossy().into_owned()],
|
||||
cwd,
|
||||
env_policy: None,
|
||||
env: HashMap::new(),
|
||||
tty: false,
|
||||
pipe_stdin: false,
|
||||
arg0: None,
|
||||
sandbox: Some(sandbox),
|
||||
enforce_managed_network: false,
|
||||
managed_network: None,
|
||||
})
|
||||
.await?;
|
||||
let (stdout, _stderr, exit_code, closed) =
|
||||
collect_process_output_from_events(session.process).await?;
|
||||
|
||||
assert!(!stdout.contains("excluded"), "unexpected stdout: {stdout}");
|
||||
assert_ne!(exit_code, Some(0));
|
||||
assert!(closed);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn read_process_until_change(
|
||||
session: Arc<dyn ExecProcess>,
|
||||
wake_rx: &mut watch::Receiver<u64>,
|
||||
|
||||
@@ -263,6 +263,35 @@ async fn remote_read_file_materializes_environment_workspace_roots() -> Result<(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_read_file_preserves_empty_workspace_roots() -> Result<()> {
|
||||
let context = create_file_system_context(FileSystemImplementation::Remote).await?;
|
||||
let file_system = context.file_system;
|
||||
let tmp = TempDir::new()?;
|
||||
let file = tmp.path().join("excluded.txt");
|
||||
std::fs::write(&file, b"excluded")?;
|
||||
|
||||
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
}]);
|
||||
let mut sandbox = FileSystemSandboxContext::from_permission_profile_with_cwd(
|
||||
PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted),
|
||||
PathUri::from_host_native_path(tmp.path())?,
|
||||
);
|
||||
sandbox.workspace_roots.clear();
|
||||
|
||||
let error = file_system
|
||||
.read_file(&PathUri::from_host_native_path(&file)?, Some(&sandbox))
|
||||
.await
|
||||
.expect_err("empty workspace roots should not grant cwd access");
|
||||
assert_sandbox_denied(&error);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(FileSystemImplementation::Local ; "local")]
|
||||
#[test_case(FileSystemImplementation::Remote ; "remote")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
|
||||
@@ -171,10 +171,11 @@ impl FileSystemSandboxContext {
|
||||
permissions: PermissionProfile<AbsolutePathBuf>,
|
||||
cwd: Option<PathUri>,
|
||||
) -> Self {
|
||||
let workspace_roots = cwd.iter().cloned().collect();
|
||||
Self {
|
||||
permissions: permissions.into(),
|
||||
cwd,
|
||||
workspace_roots: Vec::new(),
|
||||
workspace_roots,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
windows_sandbox_private_desktop: false,
|
||||
use_legacy_landlock: false,
|
||||
|
||||
Reference in New Issue
Block a user