diff --git a/codex-rs/apply-patch/src/lib.rs b/codex-rs/apply-patch/src/lib.rs index 99a63e3ace..34edad8251 100644 --- a/codex-rs/apply-patch/src/lib.rs +++ b/codex-rs/apply-patch/src/lib.rs @@ -393,6 +393,9 @@ async fn apply_hunks_to_files( let path_abs = hunk.resolve_path(cwd); match hunk { Hunk::AddFile { contents, .. } => { + ensure_sandbox_write_target_is_not_hard_link(&path_abs, fs, sandbox) + .await + .with_context(|| format!("Failed to write file {}", path_abs.display()))?; let overwritten_content = read_optional_file_text_for_delta(&path_abs, fs, sandbox, &mut delta.exact) .await; @@ -455,6 +458,9 @@ async fn apply_hunks_to_files( Hunk::UpdateFile { move_path, chunks, .. } => { + ensure_sandbox_write_target_is_not_hard_link(&path_abs, fs, sandbox) + .await + .with_context(|| format!("Failed to write file {}", path_abs.display()))?; note_existing_path_delta_support(&path_abs, fs, sandbox, &mut delta.exact).await; let AppliedPatch { original_contents, @@ -462,6 +468,9 @@ async fn apply_hunks_to_files( } = derive_new_contents_from_chunks(&path_abs, chunks, fs, sandbox).await?; if let Some(dest) = move_path { let dest_abs = AbsolutePathBuf::resolve_path_against_base(dest, cwd); + ensure_sandbox_write_target_is_not_hard_link(&dest_abs, fs, sandbox) + .await + .with_context(|| format!("Failed to write file {}", dest_abs.display()))?; let overwritten_move_content = read_optional_file_text_for_delta(&dest_abs, fs, sandbox, &mut delta.exact) .await; @@ -550,6 +559,26 @@ async fn apply_hunks_to_files( }) } +async fn ensure_sandbox_write_target_is_not_hard_link( + path: &AbsolutePathBuf, + fs: &dyn ExecutorFileSystem, + sandbox: Option<&FileSystemSandboxContext>, +) -> io::Result<()> { + let Some(sandbox) = sandbox.filter(|sandbox| sandbox.should_run_in_sandbox()) else { + return Ok(()); + }; + + match fs.get_metadata(path, Some(sandbox)).await { + Ok(metadata) if metadata.is_file && metadata.link_count > 1 => Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "refusing to write through hard link", + )), + Ok(_) => Ok(()), + Err(source) if source.kind() == io::ErrorKind::NotFound => Ok(()), + Err(source) => Err(source), + } +} + async fn ensure_not_directory( path: &AbsolutePathBuf, fs: &dyn ExecutorFileSystem, diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index bc51fdd460..a4cf078d57 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -113,6 +113,46 @@ fn restrictive_workspace_write_profile() -> PermissionProfile { ) } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum WorkspaceLinkKind { + Soft, + Hard, +} + +impl WorkspaceLinkKind { + fn name(self) -> &'static str { + match self { + WorkspaceLinkKind::Soft => "soft", + WorkspaceLinkKind::Hard => "hard", + } + } + + fn create(self, source: &std::path::Path, link: &std::path::Path) -> std::io::Result<()> { + match self { + WorkspaceLinkKind::Soft => create_file_symlink(source, link), + WorkspaceLinkKind::Hard => std::fs::hard_link(source, link), + } + } +} + +#[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, @@ -691,6 +731,86 @@ async fn apply_patch_cli_rejects_path_traversal_outside_workspace( Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[test_case(ApplyPatchModelOutput::Freeform, WorkspaceLinkKind::Soft ; "freeform_soft_link")] +#[test_case(ApplyPatchModelOutput::Function, WorkspaceLinkKind::Soft ; "function_soft_link")] +#[test_case(ApplyPatchModelOutput::Shell, WorkspaceLinkKind::Soft ; "shell_soft_link")] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc, WorkspaceLinkKind::Soft ; "shell_heredoc_soft_link")] +#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc, WorkspaceLinkKind::Soft ; "shell_command_heredoc_soft_link")] +#[test_case(ApplyPatchModelOutput::Freeform, WorkspaceLinkKind::Hard ; "freeform_hard_link")] +#[test_case(ApplyPatchModelOutput::Function, WorkspaceLinkKind::Hard ; "function_hard_link")] +#[test_case(ApplyPatchModelOutput::Shell, WorkspaceLinkKind::Hard ; "shell_hard_link")] +#[test_case(ApplyPatchModelOutput::ShellViaHeredoc, WorkspaceLinkKind::Hard ; "shell_heredoc_hard_link")] +#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc, WorkspaceLinkKind::Hard ; "shell_command_heredoc_hard_link")] +async fn apply_patch_cli_rejects_link_escape_outside_workspace( + model_output: ApplyPatchModelOutput, + link_kind: WorkspaceLinkKind, +) -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_remote!( + Ok(()), + "link escape setup needs local filesystem link creation" + ); + + let harness = apply_patch_harness().await?; + let original_contents = "original outside content\n"; + let outside_dir = harness + .test() + .config + .cwd + .parent() + .expect("cwd should have parent") + .join(format!( + "{}-{}-outside", + harness + .test() + .config + .cwd + .file_name() + .expect("cwd should have a file name") + .to_string_lossy(), + link_kind.name() + )); + std::fs::create_dir_all(&outside_dir)?; + let outside_file = outside_dir.join("victim.txt"); + std::fs::write(&outside_file, original_contents)?; + + let link_rel = format!("{}-link.txt", link_kind.name()); + let link_path = harness.path(&link_rel); + match link_kind.create(&outside_file, &link_path) { + Ok(()) => {} + Err(error) if link_kind == WorkspaceLinkKind::Soft && cfg!(windows) => { + eprintln!("Skipping Windows symlink apply_patch sandbox test: {error}"); + return Ok(()); + } + Err(error) => return Err(error.into()), + } + + let patch = format!("*** Begin Patch\n*** Add File: {link_rel}\n+pwned\n*** End Patch"); + let call_id = format!("apply-{}-link-escape", link_kind.name()); + mount_apply_patch(&harness, &call_id, &patch, "fail", model_output).await; + + harness + .submit_with_permission_profile( + "attempt to escape workspace via apply_patch link", + restrictive_workspace_write_profile(), + ) + .await?; + + let out = harness.apply_patch_output(&call_id, model_output).await; + assert!( + !out.contains("Success. Updated the following files"), + "link escape should not report success: {out}" + ); + assert_eq!( + std::fs::read_to_string(&outside_file)?, + original_contents, + "{:?} link escape should not modify the outside victim; tool output: {out}", + link_kind + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] #[test_case(ApplyPatchModelOutput::Function)] diff --git a/codex-rs/exec-server/src/fs_helper.rs b/codex-rs/exec-server/src/fs_helper.rs index 8d91224803..28cec35dcb 100644 --- a/codex-rs/exec-server/src/fs_helper.rs +++ b/codex-rs/exec-server/src/fs_helper.rs @@ -215,6 +215,7 @@ pub(crate) async fn run_direct_request( is_directory: metadata.is_directory, is_file: metadata.is_file, is_symlink: metadata.is_symlink, + link_count: metadata.link_count, created_at_ms: metadata.created_at_ms, modified_at_ms: metadata.modified_at_ms, })) diff --git a/codex-rs/exec-server/src/local_file_system.rs b/codex-rs/exec-server/src/local_file_system.rs index c7d628ce8f..ddbd69ca61 100644 --- a/codex-rs/exec-server/src/local_file_system.rs +++ b/codex-rs/exec-server/src/local_file_system.rs @@ -291,6 +291,7 @@ impl ExecutorFileSystem for DirectFileSystem { is_directory: metadata.is_dir(), is_file: metadata.is_file(), is_symlink: symlink_metadata.file_type().is_symlink(), + link_count: metadata_link_count(&metadata), created_at_ms: metadata.created().ok().map_or(0, system_time_to_unix_ms), modified_at_ms: metadata.modified().ok().map_or(0, system_time_to_unix_ms), }) @@ -513,6 +514,25 @@ fn system_time_to_unix_ms(time: SystemTime) -> i64 { .unwrap_or(0) } +#[cfg(unix)] +fn metadata_link_count(metadata: &std::fs::Metadata) -> u64 { + use std::os::unix::fs::MetadataExt; + + metadata.nlink() +} + +#[cfg(windows)] +fn metadata_link_count(metadata: &std::fs::Metadata) -> u64 { + use std::os::windows::fs::MetadataExt; + + metadata.number_of_links().map(u64::from).unwrap_or(1) +} + +#[cfg(not(any(unix, windows)))] +fn metadata_link_count(_metadata: &std::fs::Metadata) -> u64 { + 1 +} + #[cfg(all(test, unix))] mod tests { use super::*; diff --git a/codex-rs/exec-server/src/protocol.rs b/codex-rs/exec-server/src/protocol.rs index e801a7f437..76dbb6036a 100644 --- a/codex-rs/exec-server/src/protocol.rs +++ b/codex-rs/exec-server/src/protocol.rs @@ -207,6 +207,7 @@ pub struct FsGetMetadataResponse { pub is_directory: bool, pub is_file: bool, pub is_symlink: bool, + pub link_count: u64, pub created_at_ms: i64, pub modified_at_ms: i64, } diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index 4251fa33ec..99b528c163 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -118,6 +118,7 @@ impl ExecutorFileSystem for RemoteFileSystem { is_directory: response.is_directory, is_file: response.is_file, is_symlink: response.is_symlink, + link_count: response.link_count, created_at_ms: response.created_at_ms, modified_at_ms: response.modified_at_ms, }) diff --git a/codex-rs/exec-server/src/sandboxed_file_system.rs b/codex-rs/exec-server/src/sandboxed_file_system.rs index 133a3b7327..922ae12043 100644 --- a/codex-rs/exec-server/src/sandboxed_file_system.rs +++ b/codex-rs/exec-server/src/sandboxed_file_system.rs @@ -139,6 +139,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { is_directory: response.is_directory, is_file: response.is_file, is_symlink: response.is_symlink, + link_count: response.link_count, created_at_ms: response.created_at_ms, modified_at_ms: response.modified_at_ms, }) diff --git a/codex-rs/exec-server/src/server/file_system_handler.rs b/codex-rs/exec-server/src/server/file_system_handler.rs index bef30e975f..f15cde21ea 100644 --- a/codex-rs/exec-server/src/server/file_system_handler.rs +++ b/codex-rs/exec-server/src/server/file_system_handler.rs @@ -101,6 +101,7 @@ impl FileSystemHandler { is_directory: metadata.is_directory, is_file: metadata.is_file, is_symlink: metadata.is_symlink, + link_count: metadata.link_count, created_at_ms: metadata.created_at_ms, modified_at_ms: metadata.modified_at_ms, }) diff --git a/codex-rs/exec-server/tests/file_system.rs b/codex-rs/exec-server/tests/file_system.rs index 0840b2a909..7a57861f50 100644 --- a/codex-rs/exec-server/tests/file_system.rs +++ b/codex-rs/exec-server/tests/file_system.rs @@ -295,8 +295,20 @@ async fn file_system_get_metadata_returns_expected_fields(use_remote: bool) -> R assert_eq!(metadata.is_directory, false); assert_eq!(metadata.is_file, true); assert_eq!(metadata.is_symlink, false); + assert_eq!(metadata.link_count, 1); assert!(metadata.modified_at_ms > 0); + let hard_link_path = tmp.path().join("note-hard-link.txt"); + std::fs::hard_link(&file_path, &hard_link_path)?; + let hard_link_metadata = file_system + .get_metadata(&absolute_path(hard_link_path), /*sandbox*/ None) + .await + .with_context(|| format!("mode={use_remote}"))?; + assert_eq!(hard_link_metadata.is_directory, false); + assert_eq!(hard_link_metadata.is_file, true); + assert_eq!(hard_link_metadata.is_symlink, false); + assert_eq!(hard_link_metadata.link_count, 2); + let symlink_path = tmp.path().join("note-link.txt"); symlink(&file_path, &symlink_path)?; let symlink_metadata = file_system diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index 026523eb2f..bbbb157e8e 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -33,6 +33,7 @@ pub struct FileMetadata { pub is_directory: bool, pub is_file: bool, pub is_symlink: bool, + pub link_count: u64, pub created_at_ms: i64, pub modified_at_ms: i64, }