Prevent apply_patch from widening write permissions (#39614)

## Why

Deriving permissions from the parent of an already-writable patch target can
unnecessarily grant write access outside the intended workspace.

## What changed

Skip targets already covered by the active filesystem sandbox policy before
deriving additional parent-directory permissions. Targets outside the writable
area continue to request the required parent access.

## Testing

Added unit and CLI regression coverage for workspace-directory targets,
already-writable parents, and symlink escapes outside the workspace.

GitOrigin-RevId: 187109ff0b56a1a399cb8a9981b7e822977025d5

codex-rs/core/src/tools/handlers/apply_patch.rs

@@ -239,6 +239,11 @@ fn write_permissions_for_paths(
 ) -> Option<AdditionalPermissionProfile> {
     let write_paths = file_paths
         .iter()
+        // Skip already-writable targets before deriving parent permissions.
+        // Otherwise, a writable directory could grant access to its parent.
+        .filter(|path| {
+            !file_system_sandbox_policy.can_write_path_with_cwd(path.as_path(), cwd.as_path())
+        })
         .map(|path| {
             path.parent()
                 .unwrap_or_else(|| path.clone())

codex-rs/core/src/tools/handlers/apply_patch_tests.rs

@@ -1,6 +1,8 @@
 use super::*;
 use codex_apply_patch::MaybeApplyPatchVerified;
 use codex_exec_server::LOCAL_FS;
+use codex_protocol::permissions::FileSystemAccessMode;
+use codex_protocol::permissions::FileSystemSandboxEntry;
 use codex_protocol::permissions::FileSystemSandboxPolicy;
 use codex_protocol::protocol::FileChange;
 use core_test_support::PathBufExt;
@@ -312,3 +314,35 @@ fn write_permissions_for_paths_keep_dirs_outside_workspace_root() {
         Some(vec![expected_outside])
     );
 }
+
+#[test]
+fn write_permissions_for_paths_do_not_widen_workspace_root_target() {
+    let tmp = TempDir::new().expect("tmp");
+    let cwd = tmp.path().join("workspace").abs();
+    std::fs::create_dir_all(&cwd).expect("create workspace");
+    let sandbox_policy = FileSystemSandboxPolicy::workspace_write(
+        &[],
+        /*exclude_tmpdir_env_var*/ true,
+        /*exclude_slash_tmp*/ true,
+    );
+
+    let permissions =
+        write_permissions_for_paths(std::slice::from_ref(&cwd), &sandbox_policy, &cwd);
+
+    assert_eq!(permissions, None);
+}
+
+#[test]
+fn write_permissions_for_paths_do_not_regrant_an_already_writable_parent() {
+    let tmp = TempDir::new().expect("tmp");
+    let cwd = tmp.path().abs();
+    let file_path = cwd.join("protected.txt");
+    let sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
+        FileSystemSandboxEntry::new(cwd.clone().into(), FileSystemAccessMode::Write),
+        FileSystemSandboxEntry::new(file_path.clone().into(), FileSystemAccessMode::Read),
+    ]);
+
+    let permissions = write_permissions_for_paths(&[file_path], &sandbox_policy, &cwd);
+
+    assert_eq!(permissions, None);
+}

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

@@ -986,6 +986,71 @@ async fn apply_patch_cli_does_not_write_through_symlink_escape_outside_workspace
     Ok(())
 }
 
+#[cfg(any(target_os = "linux", target_os = "macos"))]
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn apply_patch_cli_does_not_widen_permissions_for_workspace_directory_target() -> 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"))?;
+    std::fs::create_dir_all(work_dir.as_path())?;
+    let outside_file = test_root.path().join("victim.txt");
+    let original_contents = "original outside content\n";
+    std::fs::write(&outside_file, original_contents)?;
+
+    let harness_work_dir = work_dir.clone();
+    let harness = apply_patch_harness_with(move |builder| {
+        builder.with_config(move |config| {
+            config.approvals_reviewer = codex_protocol::config_types::ApprovalsReviewer::User;
+            config.workspace_roots = vec![harness_work_dir.clone()];
+            config
+                .permissions
+                .set_workspace_roots(config.workspace_roots.clone());
+            config.cwd = harness_work_dir;
+        })
+    })
+    .await?;
+    let link_path = harness.path("link.txt");
+    create_file_symlink(&outside_file, &link_path)?;
+
+    // The second hunk names the already-writable workspace directory. It must
+    // not grant access to that directory's parent before the first hunk runs.
+    let patch = r#"*** Begin Patch
+*** Update File: link.txt
+@@
+-original outside content
++pwned
+*** Add File: ../work
++decoy
+*** End Patch"#;
+    let call_id = "apply-workspace-directory-decoy";
+    mount_apply_patch(&harness, call_id, patch, "fail").await;
+
+    harness
+        .submit_with_permission_profile(
+            "attempt to widen apply_patch permissions with a directory target",
+            restrictive_workspace_write_profile(),
+        )
+        .await?;
+
+    let out = harness.apply_patch_output(call_id).await;
+    assert_eq!(
+        std::fs::read_to_string(&outside_file)?,
+        original_contents,
+        "directory target must not make the outside victim writable; tool output: {out}",
+    );
+    assert!(
+        std::fs::symlink_metadata(&link_path)?
+            .file_type()
+            .is_symlink()
+    );
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn apply_patch_cli_preserves_existing_hard_link_outside_workspace() -> Result<()> {
     skip_if_no_network!(Ok(()));