Compare commits

...

2 Commits

Author SHA1 Message Date
Michael Bolin
7f3c36a32c tests: characterize macOS sandbox link writes 2026-05-08 18:39:35 -07:00
Michael Bolin
3dc8d984bb tests: cover sandbox link write behavior 2026-05-08 18:39:35 -07:00
4 changed files with 437 additions and 1 deletions

View File

@@ -383,9 +383,16 @@ impl TestCodexBuilder {
.exec_server_url
.clone()
.or_else(|| test_env.exec_server_url.clone());
#[cfg(target_os = "linux")]
let codex_linux_sandbox_exe = Some(
crate::find_codex_linux_sandbox_exe()
.context("should find binary for codex-linux-sandbox")?,
);
#[cfg(not(target_os = "linux"))]
let codex_linux_sandbox_exe = None;
let local_runtime_paths = codex_exec_server::ExecServerRuntimePaths::new(
std::env::current_exe()?,
/*codex_linux_sandbox_exe*/ None,
codex_linux_sandbox_exe,
)?;
let environment_manager = Arc::new(
codex_exec_server::EnvironmentManager::create_for_tests(

View File

@@ -15,6 +15,11 @@ use std::time::Duration;
use codex_exec_server::CreateDirectoryOptions;
use codex_features::Feature;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
@@ -23,6 +28,7 @@ use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::user_input::UserInput;
#[cfg(target_os = "linux")]
use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0;
use codex_utils_absolute_path::AbsolutePathBuf;
use core_test_support::assert_regex_match;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
@@ -112,6 +118,45 @@ fn restrictive_workspace_write_profile() -> PermissionProfile {
)
}
fn workspace_write_with_read_only_root(read_only_root: AbsolutePathBuf) -> PermissionProfile {
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: read_only_root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
]);
PermissionProfile::from_runtime_permissions(
&file_system_sandbox_policy,
NetworkSandboxPolicy::Restricted,
)
}
#[cfg(unix)]
fn create_file_symlink(source: &std::path::Path, link: &std::path::Path) -> std::io::Result<()> {
std::os::unix::fs::symlink(source, link)
}
#[cfg(windows)]
fn create_file_symlink(source: &std::path::Path, link: &std::path::Path) -> std::io::Result<()> {
std::os::windows::fs::symlink_file(source, link)
}
#[cfg(not(any(unix, windows)))]
fn create_file_symlink(_source: &std::path::Path, _link: &std::path::Path) -> std::io::Result<()> {
Err(std::io::Error::new(
std::io::ErrorKind::Unsupported,
"file symlinks are unsupported on this platform",
))
}
pub async fn mount_apply_patch(
harness: &TestCodexHarness,
call_id: &str,
@@ -675,6 +720,181 @@ async fn apply_patch_cli_rejects_path_traversal_outside_workspace(
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform ; "freeform")]
#[test_case(ApplyPatchModelOutput::Shell ; "shell")]
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc ; "shell_heredoc")]
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc ; "shell_command_heredoc")]
async fn apply_patch_cli_does_not_write_through_symlink_escape_outside_workspace(
model_output: ApplyPatchModelOutput,
) -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_remote!(
Ok(()),
"link escape setup needs local filesystem link creation"
);
let test_root = tempfile::tempdir_in(std::env::current_dir()?)?;
let work_dir = AbsolutePathBuf::try_from(test_root.path().join("work"))?;
let outside_dir = AbsolutePathBuf::try_from(test_root.path().join("outside"))?;
std::fs::create_dir_all(work_dir.as_path())?;
std::fs::create_dir_all(outside_dir.as_path())?;
let harness_work_dir = work_dir.clone();
let harness = apply_patch_harness_with(move |builder| {
builder.with_config(move |config| {
config.cwd = harness_work_dir;
})
})
.await?;
let original_contents = "original outside content\n";
let outside_file = outside_dir.join("victim.txt");
std::fs::write(&outside_file, original_contents)?;
let link_rel = "soft-link.txt";
let link_path = harness.path(link_rel);
match create_file_symlink(&outside_file, &link_path) {
Ok(()) => {}
Err(error) if cfg!(windows) => {
eprintln!("Skipping Windows symlink apply_patch sandbox test: {error}");
return Ok(());
}
Err(error) => return Err(error.into()),
}
let patch = format!(
r#"*** Begin Patch
*** Update File: {link_rel}
@@
-original outside content
+pwned
*** End Patch"#
);
let call_id = "apply-symlink-escape";
mount_apply_patch(&harness, call_id, &patch, "fail", model_output).await;
harness
.submit_with_permission_profile(
"attempt to escape workspace via apply_patch link",
workspace_write_with_read_only_root(outside_dir.clone()),
)
.await?;
let out = harness.apply_patch_output(call_id, model_output).await;
assert_eq!(
std::fs::read_to_string(&outside_file)?,
original_contents,
"symlink escape should not modify the outside victim; tool output: {out}",
);
let metadata = std::fs::symlink_metadata(&link_path)?;
assert!(metadata.file_type().is_symlink());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform ; "freeform")]
#[test_case(ApplyPatchModelOutput::Shell ; "shell")]
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc ; "shell_heredoc")]
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc ; "shell_command_heredoc")]
async fn apply_patch_cli_preserves_existing_hard_link_outside_workspace(
model_output: ApplyPatchModelOutput,
) -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_remote!(
Ok(()),
"link setup needs local filesystem hard link creation"
);
let test_root = tempfile::tempdir_in(std::env::current_dir()?)?;
let work_dir = AbsolutePathBuf::try_from(test_root.path().join("work"))?;
let outside_dir = AbsolutePathBuf::try_from(test_root.path().join("outside"))?;
std::fs::create_dir_all(work_dir.as_path())?;
std::fs::create_dir_all(outside_dir.as_path())?;
let harness_work_dir = work_dir.clone();
let harness = apply_patch_harness_with(move |builder| {
builder.with_config(move |config| {
config.cwd = harness_work_dir;
})
})
.await?;
let outside_file = outside_dir.join("victim.txt");
std::fs::write(&outside_file, "original outside content\n")?;
let link_rel = "hard-link.txt";
let link_path = harness.path(link_rel);
std::fs::hard_link(&outside_file, &link_path)?;
let patch = format!(
r#"*** Begin Patch
*** Update File: {link_rel}
@@
-original outside content
+updated through existing hard link
*** End Patch"#
);
let call_id = "apply-hard-link";
mount_apply_patch(&harness, call_id, &patch, "ok", model_output).await;
harness
.submit_with_permission_profile(
"update existing hard link via apply_patch",
workspace_write_with_read_only_root(outside_dir.clone()),
)
.await?;
let out = harness.apply_patch_output(call_id, model_output).await;
if cfg!(windows) {
assert!(
out.contains("patch rejected: writing outside of the project"),
"Windows sandboxing intentionally rejects writes through existing hard links to files outside the workspace; tool output: {out}"
);
assert_eq!(
std::fs::read_to_string(&outside_file)?,
"original outside content\n",
"Windows rejection must leave the outside hard-link target unchanged"
);
assert_eq!(
std::fs::read_to_string(&link_path)?,
"original outside content\n",
"Windows rejection must leave the workspace hard-link path unchanged"
);
std::fs::write(&outside_file, "post-reject outside write\n")?;
assert_eq!(
std::fs::read_to_string(&link_path)?,
"post-reject outside write\n",
"Windows rejection must not unlink or replace an existing hard link"
);
return Ok(());
}
assert!(
out.contains("Success. Updated the following files:"),
"apply_patch should intentionally allow updates through existing hard links; tool output: {out}"
);
assert_eq!(
std::fs::read_to_string(&outside_file)?,
"updated through existing hard link\n",
"apply_patch intentionally preserves existing hard-link semantics; the outside path observes the shared inode update"
);
assert_eq!(
std::fs::read_to_string(&link_path)?,
"updated through existing hard link\n",
"apply_patch intentionally preserves existing hard-link semantics; the workspace path observes the same update"
);
std::fs::write(&outside_file, "post-apply outside write\n")?;
assert_eq!(
std::fs::read_to_string(&link_path)?,
"post-apply outside write\n",
"apply_patch must not unlink or replace an existing hard link; later writes through either path should still be visible"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
#[test_case(ApplyPatchModelOutput::Freeform)]
#[test_case(ApplyPatchModelOutput::Shell)]

View File

@@ -2,6 +2,7 @@
mod common;
use std::os::unix::fs::MetadataExt;
#[cfg(target_os = "linux")]
use std::os::unix::fs::PermissionsExt;
use std::os::unix::fs::symlink;
@@ -732,6 +733,53 @@ async fn file_system_sandboxed_write_rejects_symlink_escape(use_remote: bool) ->
Ok(())
}
#[test_case(false ; "local")]
#[test_case(true ; "remote")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn file_system_sandboxed_write_preserves_existing_hard_link(use_remote: bool) -> Result<()> {
let context = create_file_system_context(use_remote).await?;
let file_system = context.file_system;
let tmp = TempDir::new()?;
let allowed_dir = tmp.path().join("allowed");
let outside_dir = tmp.path().join("outside");
std::fs::create_dir_all(&allowed_dir)?;
std::fs::create_dir_all(&outside_dir)?;
let outside_file = outside_dir.join("outside.txt");
let hard_link = allowed_dir.join("hard-link.txt");
std::fs::write(&outside_file, "outside\n")?;
std::fs::hard_link(&outside_file, &hard_link)?;
let sandbox = workspace_write_sandbox(allowed_dir);
file_system
.write_file(
&absolute_path(hard_link.clone()),
b"updated through existing hard link\n".to_vec(),
Some(&sandbox),
)
.await
.with_context(|| format!("mode={use_remote}"))?;
assert_eq!(
std::fs::read_to_string(&outside_file)?,
"updated through existing hard link\n"
);
assert_eq!(
std::fs::read_to_string(&hard_link)?,
"updated through existing hard link\n"
);
let outside_metadata = std::fs::metadata(&outside_file)?;
let link_metadata = std::fs::metadata(&hard_link)?;
assert_eq!(
(link_metadata.dev(), link_metadata.ino()),
(outside_metadata.dev(), outside_metadata.ino())
);
Ok(())
}
#[test_case(false ; "local")]
#[test_case(true ; "remote")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]

View File

@@ -6,6 +6,8 @@ use codex_utils_absolute_path::test_support::PathBufExt;
use std::collections::HashMap;
use std::future::Future;
use std::io;
#[cfg(target_os = "macos")]
use std::path::Path;
use std::process::ExitStatus;
use tokio::fs::create_dir_all;
use tokio::process::Child;
@@ -359,6 +361,124 @@ async fn sandbox_distinguishes_command_and_policy_cwds() {
assert!(allowed_exists, "allowed path should exist");
}
#[cfg(target_os = "macos")]
#[tokio::test]
async fn macos_workspace_write_allows_existing_hard_link_write_through() {
core_test_support::skip_if_sandbox!();
let temp = tempfile::tempdir().expect("should create temp dir");
let workspace_dir = temp.path().join("workspace");
let outside_dir = temp.path().join("outside");
create_dir_all(&workspace_dir)
.await
.expect("mkdir workspace");
create_dir_all(&outside_dir).await.expect("mkdir outside");
let victim = outside_dir.join("victim.txt");
tokio::fs::write(&victim, "original")
.await
.expect("write victim");
std::fs::hard_link(&victim, workspace_dir.join("hard.txt")).expect("create hard link");
let output = run_macos_workspace_write_command(
&workspace_dir,
vec![
"/bin/sh".to_string(),
"-c".to_string(),
"printf pwned > hard.txt".to_string(),
],
)
.await;
assert!(
output.status.success(),
"sandbox should allow writing through an existing hard link: {}",
command_output_details(&output)
);
let victim_contents = tokio::fs::read_to_string(&victim)
.await
.expect("read victim");
assert_eq!(victim_contents, "pwned");
}
#[cfg(target_os = "macos")]
#[tokio::test]
async fn macos_workspace_write_rejects_existing_symlink_write_through() {
core_test_support::skip_if_sandbox!();
let temp = tempfile::tempdir().expect("should create temp dir");
let workspace_dir = temp.path().join("workspace");
let outside_dir = temp.path().join("outside");
create_dir_all(&workspace_dir)
.await
.expect("mkdir workspace");
create_dir_all(&outside_dir).await.expect("mkdir outside");
let victim = outside_dir.join("victim.txt");
tokio::fs::write(&victim, "original")
.await
.expect("write victim");
std::os::unix::fs::symlink(&victim, workspace_dir.join("soft.txt")).expect("create symlink");
let output = run_macos_workspace_write_command(
&workspace_dir,
vec![
"/bin/sh".to_string(),
"-c".to_string(),
"printf pwned > soft.txt".to_string(),
],
)
.await;
assert!(
!output.status.success(),
"sandbox unexpectedly allowed writing through a symlink: {}",
command_output_details(&output)
);
let victim_contents = tokio::fs::read_to_string(&victim)
.await
.expect("read victim");
assert_eq!(victim_contents, "original");
}
#[cfg(target_os = "macos")]
#[tokio::test]
async fn macos_workspace_write_rejects_creating_hard_link_to_outside_file() {
core_test_support::skip_if_sandbox!();
let temp = tempfile::tempdir().expect("should create temp dir");
let workspace_dir = temp.path().join("workspace");
let outside_dir = temp.path().join("outside");
create_dir_all(&workspace_dir)
.await
.expect("mkdir workspace");
create_dir_all(&outside_dir).await.expect("mkdir outside");
let victim = outside_dir.join("victim.txt");
tokio::fs::write(&victim, "original")
.await
.expect("write victim");
let output = run_macos_workspace_write_command(
&workspace_dir,
vec![
"/bin/sh".to_string(),
"-c".to_string(),
"ln \"$1\" hard.txt && printf pwned > hard.txt".to_string(),
"sh".to_string(),
victim.to_string_lossy().into_owned(),
],
)
.await;
assert!(
!output.status.success(),
"sandbox unexpectedly allowed creating a hard link to an outside file: {}",
command_output_details(&output)
);
let victim_contents = tokio::fs::read_to_string(&victim)
.await
.expect("read victim");
assert_eq!(victim_contents, "original");
}
#[tokio::test]
async fn sandbox_blocks_first_time_dot_codex_creation() {
core_test_support::skip_if_sandbox!();
@@ -430,6 +550,47 @@ async fn sandbox_blocks_first_time_dot_codex_creation() {
);
}
#[cfg(target_os = "macos")]
async fn run_macos_workspace_write_command(
workspace_dir: &Path,
command: Vec<String>,
) -> std::process::Output {
let workspace = workspace_dir.to_path_buf().abs();
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
};
let child = match spawn_command_under_sandbox(
command,
workspace.clone(),
&policy,
&workspace,
StdioPolicy::RedirectForShellTool,
HashMap::new(),
)
.await
{
Ok(child) => child,
Err(err) => panic!("should spawn command under sandbox: {err}"),
};
match child.wait_with_output().await {
Ok(output) => output,
Err(err) => panic!("should collect sandboxed command output: {err}"),
}
}
#[cfg(target_os = "macos")]
fn command_output_details(output: &std::process::Output) -> String {
format!(
"status={:?}, stdout={:?}, stderr={:?}",
output.status,
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
)
}
fn unix_sock_body() {
unsafe {
let mut fds = [0i32; 2];