test(sandbox): tighten symlink hardening coverage

Co-authored-by: Codex noreply@openai.com

codex-rs/config/src/loader/tests.rs

@@ -10,6 +10,8 @@ use codex_file_system::RemoveOptions;
 use pretty_assertions::assert_eq;
 use std::path::Path;
 use tempfile::tempdir;
+#[cfg(unix)]
+use tokio::io::AsyncReadExt;
 
 struct TestFileSystem;
 
@@ -48,7 +50,18 @@ impl ExecutorFileSystem for TestFileSystem {
         path: &AbsolutePathBuf,
         _sandbox: Option<&FileSystemSandboxContext>,
     ) -> FileSystemResult<Vec<u8>> {
-        tokio::fs::read(path.as_path()).await
+        #[cfg(unix)]
+        {
+            let file = codex_utils_path::open_file_for_read_no_follow(path.as_path())?;
+            let mut file = tokio::fs::File::from_std(file);
+            let mut contents = Vec::new();
+            file.read_to_end(&mut contents).await?;
+            Ok(contents)
+        }
+        #[cfg(not(unix))]
+        {
+            self.read_file(path, /*sandbox*/ None).await
+        }
     }
 
     async fn write_file(

codex-rs/exec-server/src/protocol.rs

@@ -165,6 +165,7 @@ pub struct TerminateResponse {
 pub struct FsReadFileParams {
     pub path: AbsolutePathBuf,
     pub sandbox: Option<FileSystemSandboxContext>,
+    /// Reject symlinks in every path component on Unix executors.
     #[serde(default, skip_serializing_if = "std::ops::Not::not")]
     pub no_follow_symlinks: bool,
 }

codex-rs/exec-server/tests/file_system.rs

@@ -757,15 +757,14 @@ async fn file_system_read_file_no_follow_rejects_symlinked_parent(use_remote: bo
         .with_context(|| format!("mode={use_remote}"))?;
     assert_eq!(contents, b"original");
 
-    if file_system
+    if let Ok(contents) = file_system
         .read_file_no_follow(
             &absolute_path(symlinked_dir.join("payload")),
             /*sandbox*/ None,
         )
         .await
-        .is_ok()
     {
-        anyhow::bail!("symlinked parent should fail");
+        anyhow::bail!("symlinked parent should fail, but read {contents:?}");
     }
 
     Ok(())

codex-rs/file-system/src/lib.rs

@@ -156,7 +156,9 @@ pub trait ExecutorFileSystem: Send + Sync {
         sandbox: Option<&FileSystemSandboxContext>,
     ) -> FileSystemResult<Vec<u8>>;
 
-    /// Reads a file while rejecting symlinks in any path component.
+    /// Reads a file while rejecting symlinks in any path component on Unix.
+    ///
+    /// Non-Unix executors currently fall back to [`Self::read_file`].
     async fn read_file_no_follow(
         &self,
         path: &AbsolutePathBuf,
@@ -173,7 +175,7 @@ pub trait ExecutorFileSystem: Send + Sync {
         String::from_utf8(bytes).map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))
     }
 
-    /// Reads a file without following symlinks and decodes it as UTF-8 text.
+    /// Reads a file with [`Self::read_file_no_follow`] and decodes it as UTF-8 text.
     async fn read_file_text_no_follow(
         &self,
         path: &AbsolutePathBuf,

codex-rs/protocol/src/permissions.rs

@@ -1809,10 +1809,7 @@ fn has_explicit_write_entry_for_metadata_path(
 }
 
 fn path_exists_or_is_symlink(path: &Path) -> bool {
-    path.exists()
-        || std::fs::symlink_metadata(path)
-            .map(|metadata| metadata.file_type().is_symlink())
-            .unwrap_or(false)
+    std::fs::symlink_metadata(path).is_ok()
 }
 
 fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool {

codex-rs/utils/path-utils/src/lib.rs

@@ -145,6 +145,8 @@ pub fn write_atomically(write_path: &Path, contents: &str) -> io::Result<()> {
 }
 
 /// Opens a file for reading without following symlinks in any path component.
+///
+/// Relative paths are resolved from the process's current directory.
 #[cfg(unix)]
 pub fn open_file_for_read_no_follow(path: &Path) -> io::Result<File> {
     open_file_no_follow(path, libc::O_RDONLY, /*mode*/ 0)

codex-rs/utils/path-utils/src/path_utils_tests.rs

@@ -39,6 +39,22 @@ mod symlinks {
         Ok(())
     }
 
+    #[test]
+    fn no_follow_read_rejects_symlinked_file() -> std::io::Result<()> {
+        let dir = tempfile::tempdir()?;
+        let dir = dir.path().canonicalize()?;
+        let target = dir.join("target");
+        let symlink_path = dir.join("link");
+        std::fs::write(&target, "secret")?;
+        symlink(&target, &symlink_path)?;
+
+        let err =
+            open_file_for_read_no_follow(&symlink_path).expect_err("symlinked file should fail");
+
+        assert_eq!(err.raw_os_error(), Some(libc::ELOOP));
+        Ok(())
+    }
+
     #[test]
     fn no_follow_read_reads_regular_file() -> std::io::Result<()> {
         let dir = tempfile::tempdir()?;