Preserve filesystem denials when preparing approved commands (#48155)

## Why

Approved commands need broader write access while retaining explicit read denials. On Linux, binding the filesystem root writable can shadow standard devices, and additional root-metadata mounts can reopen denied symlink targets.

## What changed

- Add `FileSystemSandboxPolicy::for_approved_command` to grant root and root-metadata writes while retaining path and glob denials. Keep the original policy when denials cannot be resolved or deny the root.
- Restore standard devices after Linux writable root binds, then apply explicit deny masks. Avoid redundant root-alias binds and inherit root-metadata writes from the root mount so denied targets stay masked.
- Preserve literal deny paths alongside resolved targets so Linux can reject denials that cross writable symlinks.
- Advertise Linux device and approved-root restriction support through two opt-in exec-server capabilities, omitted from serialization when false.

## Testing

Add regression coverage for approval policy materialization across Unix, Windows drive, and UNC paths; Windows volume expansion; Linux device access and metadata symlink restrictions; and capability serialization compatibility.

GitOrigin-RevId: 40584473da98a15e606bb1a8934308f69528850f

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

@@ -160,6 +160,12 @@ pub struct EnvironmentCapabilities {
     /// Whether requests may explicitly select the MXC Windows sandbox backend.
     #[serde(default)]
     pub windows_mxc: bool,
+    /// Whether a Linux filesystem sandbox preserves standard devices when `/` is writable.
+    #[serde(default, skip_serializing_if = "std::ops::Not::not")]
+    pub linux_root_write_preserves_devices: bool,
+    /// Whether approved Linux root writes preserve devices and denied root-metadata symlink targets.
+    #[serde(default, skip_serializing_if = "std::ops::Not::not")]
+    pub linux_approved_root_write_preserves_restrictions: bool,
 }
 
 /// Status returned by an initialized exec-server connection.
@@ -257,6 +263,8 @@ impl EnvironmentInfo {
                 sandboxed_file_streaming: true,
                 shell_snapshot_v2: cfg!(unix),
                 windows_mxc,
+                linux_root_write_preserves_devices: cfg!(target_os = "linux"),
+                linux_approved_root_write_preserves_restrictions: cfg!(target_os = "linux"),
             },
         }
     }
@@ -1390,10 +1398,46 @@ mod tests {
                 sandboxed_file_streaming: false,
                 shell_snapshot_v2: false,
                 windows_mxc: false,
+                linux_root_write_preserves_devices: false,
+                linux_approved_root_write_preserves_restrictions: false,
             }
         );
     }
 
+    #[test]
+    fn linux_approved_root_write_support_is_opt_in_on_the_wire() {
+        let mut capabilities = EnvironmentCapabilities::default();
+        let legacy = serde_json::to_value(&capabilities).unwrap();
+        assert!(legacy.get("linuxRootWritePreservesDevices").is_none());
+        assert!(
+            legacy
+                .get("linuxApprovedRootWritePreservesRestrictions")
+                .is_none()
+        );
+        capabilities.linux_root_write_preserves_devices = true;
+        let device_only = serde_json::from_value::<EnvironmentCapabilities>(
+            serde_json::to_value(&capabilities).unwrap(),
+        )
+        .unwrap();
+        assert!(!device_only.linux_approved_root_write_preserves_restrictions);
+        capabilities.linux_approved_root_write_preserves_restrictions = true;
+        assert_eq!(
+            serde_json::from_value::<EnvironmentCapabilities>(
+                serde_json::to_value(&capabilities).unwrap()
+            )
+            .unwrap(),
+            capabilities
+        );
+        let local = EnvironmentInfo::local().capabilities;
+        assert_eq!(
+            (
+                local.linux_root_write_preserves_devices,
+                local.linux_approved_root_write_preserves_restrictions,
+            ),
+            (cfg!(target_os = "linux"), cfg!(target_os = "linux"))
+        );
+    }
+
     #[test]
     fn environment_info_preserves_executor_metadata() {
         let expected = serde_json::json!({

codex-rs/linux-sandbox/src/bwrap.rs

@@ -416,7 +416,8 @@ fn create_bwrap_flags(
 /// 3. Unreadable ancestors of writable roots are masked before their child
 ///    mounts are rebound so nested writable carveouts can be reopened safely.
 /// 4. `--bind <root> <root>` re-enables writes for allowed roots, including
-///    writable subpaths under `/dev` (for example, `/dev/shm`).
+///    writable subpaths under `/dev` (for example, `/dev/shm`). Binding `/`
+///    recreates the minimal `/dev` before applying any carveouts below it.
 /// 5. `--ro-bind <subpath> <subpath>` re-applies read-only protections under
 ///    those writable roots so protected subpaths win.
 /// 6. Nested unreadable carveouts under a writable root are masked after that
@@ -438,7 +439,7 @@ fn create_filesystem_args(
     // roots so mixed-platform configs can keep harmless paths for other
     // environments without breaking Linux command startup.
     let mut writable_roots = file_system_sandbox_policy
-        .get_writable_roots_with_cwd(cwd)
+        .get_writable_roots_with_cwd_inheriting_root_metadata(cwd)
         .into_iter()
         .filter(|writable_root| writable_root.root.as_path().exists())
         .collect::<Vec<_>>();
@@ -486,7 +487,7 @@ fn create_filesystem_args(
             })
             .collect();
     let mut unreadable_roots = file_system_sandbox_policy
-        .get_unreadable_roots_with_cwd(cwd)
+        .get_unreadable_roots_with_cwd_preserving_symlinks(cwd)
         .into_iter()
         .map(AbsolutePathBuf::into_path_buf)
         .collect::<Vec<_>>();
@@ -606,6 +607,9 @@ fn create_filesystem_args(
     let unreadable_paths: HashSet<PathBuf> = unreadable_roots.iter().cloned().collect();
     let mut sorted_writable_roots = writable_roots;
     sorted_writable_roots.sort_by_key(|writable_root| path_depth(writable_root.root.as_path()));
+    let binds_file_system_root = sorted_writable_roots
+        .iter()
+        .any(|writable_root| writable_root.root.as_path() == Path::new("/"));
     // Mask only the unreadable ancestors that sit outside every writable root.
     // Unreadable paths nested under a broader writable root are applied after
     // that broader root is bound, then reopened by any deeper writable child.
@@ -643,10 +647,27 @@ fn create_filesystem_args(
         }
 
         let mount_root = symlink_target.as_deref().unwrap_or(root);
-        bwrap_args.args.push("--bind".to_string());
-        bwrap_args.args.push(path_to_string(mount_root));
-        bwrap_args.args.push(path_to_string(mount_root));
-        append_daemon_socket_masks(&mut bwrap_args.args, mount_root, &daemon_directories)?;
+        // Rebinding a root alias would also undo masks already applied to `/`.
+        // The physical root is already writable; only the alias's carveouts remain.
+        let redundant_root_alias =
+            binds_file_system_root && root != Path::new("/") && mount_root == Path::new("/");
+        if !redundant_root_alias {
+            bwrap_args.args.push("--bind".to_string());
+            bwrap_args.args.push(path_to_string(mount_root));
+            bwrap_args.args.push(path_to_string(mount_root));
+            if mount_root == Path::new("/") {
+                // The root bind shadows the earlier device tree and applies nodev.
+                // Recreate only standard devices before applying explicit deny masks.
+                bwrap_args.args.extend([
+                    "--dev".to_string(),
+                    "/dev".to_string(),
+                    "--bind-try".to_string(),
+                    "/dev/shm".to_string(),
+                    "/dev/shm".to_string(),
+                ]);
+            }
+            append_daemon_socket_masks(&mut bwrap_args.args, mount_root, &daemon_directories)?;
+        }
 
         let mut read_only_subpaths: Vec<PathBuf> = writable_root
             .read_only_subpaths
@@ -1631,6 +1652,8 @@ mod tests {
         let temp_dir = TempDir::new().expect("temp dir");
         let root_env = temp_dir.path().join(".env");
         std::fs::write(&root_env, "secret").expect("write env");
+        let root_alias = temp_dir.path().join("root-alias");
+        std::os::unix::fs::symlink("/", &root_alias).expect("create root symlink");
         let policy = FileSystemSandboxPolicy::restricted(vec![
             FileSystemSandboxEntry {
                 path: FileSystemPath::Special {
@@ -1640,6 +1663,18 @@ mod tests {
                 missing_path_behavior: None,
             },
             unreadable_glob_entry(format!("{}/**/*.env", temp_dir.path().display())),
+            FileSystemSandboxEntry::new(
+                AbsolutePathBuf::from_absolute_path(root_alias)
+                    .expect("absolute root alias")
+                    .into(),
+                FileSystemAccessMode::Write,
+            ),
+            FileSystemSandboxEntry::new(
+                AbsolutePathBuf::try_from("/dev/zero")
+                    .expect("device path")
+                    .into(),
+                FileSystemAccessMode::Deny,
+            ),
         ]);
         let command = vec!["/bin/true".to_string()];
 
@@ -1657,6 +1692,30 @@ mod tests {
             "full-write policy with unreadable globs must still use bwrap"
         );
         assert_file_masked(&args.args, &root_env);
+        assert_file_masked(&args.args, Path::new("/dev/zero"));
+        assert_eq!(
+            args.args
+                .windows(3)
+                .filter(|window| *window == ["--bind", "/", "/"])
+                .count(),
+            1,
+        );
+        let writable_root = args
+            .args
+            .windows(3)
+            .position(|window| window == ["--bind", "/", "/"])
+            .expect("writable root");
+        let devices = args
+            .args
+            .windows(5)
+            .rposition(|window| window == ["--dev", "/dev", "--bind-try", "/dev/shm", "/dev/shm"])
+            .expect("minimal device tree with shared memory");
+        let device_mask = args
+            .args
+            .windows(3)
+            .position(|window| window[0] == "--ro-bind-data" && window[2] == "/dev/zero")
+            .expect("explicit device deny mask");
+        assert!(writable_root < devices && devices < device_mask);
     }
 
     #[cfg(unix)]
@@ -2262,11 +2321,15 @@ mod tests {
                 PathBuf::from("/dev/.codex"),
             ]
         );
-        let dev_mount = args
+        let dev_mounts = args
             .args
             .windows(2)
-            .position(|args| args == ["--dev", "/dev"])
-            .expect("/dev mount");
+            .enumerate()
+            .filter_map(|(index, args)| (args == ["--dev", "/dev"]).then_some(index))
+            .collect::<Vec<_>>();
+        let [initial_dev_mount, restored_dev_mount] = dev_mounts.as_slice() else {
+            panic!("expected exactly two /dev mounts, got {dev_mounts:?}");
+        };
         let root_bind = args
             .args
             .windows(3)
@@ -2277,7 +2340,8 @@ mod tests {
             .windows(3)
             .position(|args| args == ["--bind", "/dev", "/dev"])
             .expect("/dev bind");
-        assert!(dev_mount < root_bind && root_bind < dev_bind);
+        assert!(*initial_dev_mount < root_bind && root_bind < *restored_dev_mount);
+        assert!(*restored_dev_mount < dev_bind);
     }
 
     #[test]

codex-rs/linux-sandbox/tests/suite/landlock.rs

@@ -31,6 +31,9 @@ mod wslg_tests;
 #[path = "nested_metadata_tests.rs"]
 mod nested_metadata_tests;
 
+#[path = "root_metadata_tests.rs"]
+mod root_metadata_tests;
+
 // At least on GitHub CI, the arm64 tests appear to need longer timeouts.
 
 #[cfg(not(target_arch = "aarch64"))]

codex-rs/linux-sandbox/tests/suite/managed_proxy.rs

@@ -14,6 +14,7 @@ use codex_protocol::permissions::FileSystemAccessMode;
 use codex_protocol::permissions::FileSystemPath;
 use codex_protocol::permissions::FileSystemSandboxEntry;
 use codex_protocol::permissions::FileSystemSandboxPolicy;
+use codex_protocol::permissions::FileSystemSandboxPolicyContext;
 use codex_protocol::permissions::FileSystemSpecialPath;
 use codex_protocol::permissions::NetworkSandboxPolicy;
 use codex_utils_absolute_path::AbsolutePathBuf;
@@ -633,6 +634,62 @@ async fn managed_proxy_bridges_release_command_output_after_exit() {
     assert_eq!(output.stdout, b"bridge output closed\n");
 }
 
+#[tokio::test]
+async fn approved_command_with_denied_reads_preserves_standard_devices() {
+    if should_skip_bwrap_tests().await {
+        eprintln!("skipping bwrap test: bubblewrap is unavailable");
+        return;
+    }
+    let files = tempfile::tempdir().unwrap();
+    let denied = files.path().join("secret");
+    let output_file = files.path().join("output");
+    std::fs::write(&denied, "secret").unwrap();
+    let mut filesystem = FileSystemSandboxPolicy::read_only();
+    filesystem.entries.push(FileSystemSandboxEntry::new(
+        AbsolutePathBuf::try_from(denied.clone()).unwrap().into(),
+        FileSystemAccessMode::Deny,
+    ));
+    let cwd = AbsolutePathBuf::try_from(std::env::current_dir().unwrap())
+        .unwrap()
+        .into();
+    let context = FileSystemSandboxPolicyContext {
+        cwd: &cwd,
+        workspace_roots: std::slice::from_ref(&cwd),
+        user_home_dir: None,
+        temporary_directories: Some(&[]),
+    };
+    let profile = PermissionProfile::from_runtime_permissions(
+        &filesystem.for_approved_command(&context),
+        NetworkSandboxPolicy::Restricted,
+    );
+    let mut env = create_env_from_core_vars();
+    strip_proxy_env(&mut env);
+    env.insert(
+        "CODEX_TEST_DENIED".into(),
+        denied.to_string_lossy().into_owned(),
+    );
+    env.insert(
+        "CODEX_TEST_OUTPUT".into(),
+        output_file.to_string_lossy().into_owned(),
+    );
+    let output = run_linux_sandbox_direct(
+        &["bash", "-c", concat!(
+            "set -e; printf test >/dev/null; ",
+            "head -c 1 /dev/zero >/dev/null; head -c 1 /dev/urandom >/dev/null; ",
+            "printf sink >\"$CODEX_TEST_OUTPUT\"; test \"$(cat \"$CODEX_TEST_OUTPUT\")\" = sink; ",
+            "if cat \"$CODEX_TEST_DENIED\" >\"$CODEX_TEST_OUTPUT\" 2>&1; then exit 1; fi; ",
+            "grep -q 'Permission denied' \"$CODEX_TEST_OUTPUT\"",
+        )],
+        &profile, /*allow_network_for_proxy*/ false, env, NETWORK_TIMEOUT_MS,
+    ).await;
+    assert_eq!(
+        output.status.code(),
+        Some(0),
+        "{}",
+        String::from_utf8_lossy(&output.stderr)
+    );
+}
+
 #[tokio::test]
 async fn managed_proxy_readiness_survives_closed_standard_descriptors() {
     if let Some(skip_reason) = managed_proxy_skip_reason().await {

codex-rs/linux-sandbox/tests/suite/root_metadata_tests.rs

@@ -0,0 +1,170 @@
+//! Approved root metadata writes must not remount targets hidden by executor-side denials.
+
+use super::codex_linux_sandbox_exe;
+use super::should_skip_bwrap_tests;
+use codex_protocol::models::PermissionProfile;
+use codex_protocol::permissions::FileSystemAccessMode;
+use codex_protocol::permissions::FileSystemSandboxEntry;
+use codex_protocol::permissions::FileSystemSandboxPolicy;
+use codex_protocol::permissions::FileSystemSandboxPolicyContext;
+use codex_protocol::permissions::NetworkSandboxPolicy;
+use codex_utils_absolute_path::AbsolutePathBuf;
+use pretty_assertions::assert_eq;
+use std::fs;
+use std::process::Command;
+
+#[tokio::test]
+async fn root_metadata_symlinks_cannot_reopen_approved_denials() {
+    if should_skip_bwrap_tests().await {
+        return;
+    }
+    let Some(bwrap) = codex_sandboxing::find_system_bwrap_in_path() else {
+        eprintln!("skipping root metadata test: system bubblewrap is unavailable");
+        return;
+    };
+    let fixture = tempfile::tempdir_in("/tmp").unwrap();
+    let work = fixture.path().join("work");
+    let private = fixture.path().join("private");
+    fs::create_dir_all(work.join("public")).unwrap();
+    fs::create_dir_all(private.join("reopened")).unwrap();
+    fs::write(work.join("public/secret"), "outside").unwrap();
+    fs::write(private.join("reopened/secret"), "private").unwrap();
+    fs::write(private.join("sibling"), "sibling").unwrap();
+    fs::copy(codex_linux_sandbox_exe(), work.join("sandbox")).unwrap();
+
+    let path_entry = |path: &str, access| {
+        FileSystemSandboxEntry::new(
+            AbsolutePathBuf::from_absolute_path(path).unwrap().into(),
+            access,
+        )
+    };
+    let cwd = AbsolutePathBuf::from_absolute_path("/tmp").unwrap().into();
+    let context = FileSystemSandboxPolicyContext {
+        cwd: &cwd,
+        workspace_roots: std::slice::from_ref(&cwd),
+        user_home_dir: None,
+        temporary_directories: Some(&[]),
+    };
+    let approved = |denied| {
+        let mut policy = FileSystemSandboxPolicy::read_only();
+        policy
+            .entries
+            .push(path_entry(denied, FileSystemAccessMode::Deny));
+        policy.for_approved_command(&context)
+    };
+    let run = |policy: &FileSystemSandboxPolicy,
+               private_mount: &str,
+               codex_target: &str,
+               script: &str| {
+        let profile =
+            PermissionProfile::from_runtime_permissions(policy, NetworkSandboxPolicy::Enabled);
+        let mut command = Command::new(&bwrap);
+        command
+            .args("--unshare-user --uid 0 --gid 0 --die-with-parent --tmpfs /".split_whitespace());
+        command.args(
+            "--ro-bind /usr /usr --ro-bind /etc /etc --proc /proc --dev /dev".split_whitespace(),
+        );
+        command.args("--dir /run --dir /.agents --symlink /private-link /.git".split_whitespace());
+        command.args(["--symlink", codex_target, "/.codex"]);
+        command.args(["--symlink", "/private/reopened", "/private-link"]);
+        for path in ["/bin", "/sbin", "/lib", "/lib64"] {
+            if let Ok(metadata) = fs::symlink_metadata(path) {
+                if metadata.file_type().is_symlink() {
+                    command
+                        .arg("--symlink")
+                        .arg(fs::read_link(path).unwrap())
+                        .arg(path);
+                } else {
+                    command.args(["--ro-bind", path, path]);
+                }
+            }
+        }
+        command.arg("--bind").arg(&work).arg("/tmp");
+        command.arg("--bind").arg(&private).arg(private_mount);
+        if private_mount != "/private" {
+            command.args(["--symlink", private_mount, "/private"]);
+        }
+        command.args(
+            "--chdir /tmp --setenv TMPDIR /tmp --setenv PATH /usr/bin:/bin".split_whitespace(),
+        );
+        command.args(
+            "-- /tmp/sandbox --sandbox-policy-cwd /tmp --permission-profile".split_whitespace(),
+        );
+        command.arg(serde_json::to_string(&profile).unwrap());
+        command.args(["--", "/bin/sh", "-c", script]);
+        command.output().unwrap()
+    };
+
+    let private_denied = approved("/private");
+    let script = r#"
+        set -eu
+        if cat /private/reopened/secret >/dev/null 2>&1 ||
+           cat /.codex/secret >/dev/null 2>&1 || cat /.git/secret >/dev/null 2>&1; then exit 21; fi
+        if (printf bad > /.codex/changed) 2>/dev/null; then exit 22; fi
+        printf good > /.agents/allowed
+        printf good > /tmp/marker
+        printf 'root-metadata-denials-held\n'
+    "#;
+    let output = run(&private_denied, "/private", "/private/reopened", script);
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    if stderr.starts_with("bwrap: Creating new namespace failed")
+        || stderr.contains("No permissions to create a new namespace")
+        || stderr.contains("setting up uid map")
+    {
+        eprintln!("skipping root metadata test: {stderr}");
+        return;
+    }
+    assert_eq!(output.status.code(), Some(0), "{output:?}");
+    assert_eq!(output.stdout, b"root-metadata-denials-held\n");
+    assert_eq!(fs::read_to_string(work.join("marker")).unwrap(), "good");
+    assert!(!private.join("reopened/changed").exists());
+
+    let script = r#"
+        set -eu
+        printf safe > /.codex/allowed
+        if cat /.git/secret >/dev/null 2>&1; then exit 23; fi
+    "#;
+    let output = run(&private_denied, "/private", "/tmp/public", script);
+    assert_eq!(output.status.code(), Some(0), "{output:?}");
+    assert_eq!(
+        fs::read_to_string(work.join("public/allowed")).unwrap(),
+        "safe"
+    );
+
+    let script = r#"
+        set -eu
+        rm /private
+        ln -s /tmp/public /private
+        cat /.codex/secret
+    "#;
+    let output = run(&private_denied, "/real-private", "/private", script);
+    assert!(!output.status.success(), "{output:?}");
+    assert!(String::from_utf8_lossy(&output.stderr).contains(
+        "cannot enforce sandbox deny-read path /private because it crosses writable symlink /private"
+    ), "{output:?}");
+    assert!(output.stdout.is_empty(), "{output:?}");
+
+    let mut reopened = private_denied;
+    reopened
+        .entries
+        .push(path_entry("/private/reopened", FileSystemAccessMode::Write));
+    let script = r#"
+        set -eu
+        test "$(cat /.codex/secret)" = private
+        if cat /private/sibling >/dev/null 2>&1; then exit 24; fi
+    "#;
+    let output = run(&reopened, "/private", "/private/reopened", script);
+    assert_eq!(output.status.code(), Some(0), "{output:?}");
+
+    let output = run(
+        &approved("/.codex/secret"),
+        "/private",
+        "/private/reopened",
+        "true",
+    );
+    assert!(!output.status.success(), "{output:?}");
+    assert!(String::from_utf8_lossy(&output.stderr).contains(
+        "cannot enforce sandbox deny-read path /.codex/secret because it crosses writable symlink /.codex"
+    ), "{output:?}");
+    assert!(output.stdout.is_empty(), "{output:?}");
+}

codex-rs/mxc-sandbox/src/policy_tests.rs

@@ -82,6 +82,58 @@ fn symbolic_root_precedence_is_independent_of_drive_and_cwd() -> Result<()> {
     Ok(())
 }
 
+#[test]
+fn approved_command_expands_volume_roots_without_retaining_original_grants() -> Result<()> {
+    use FileSystemAccessMode::Deny;
+    use FileSystemAccessMode::Read;
+    use FileSystemAccessMode::Write;
+
+    let volumes = [
+        PathUri::parse("file:///C:/")?,
+        PathUri::parse("file:///D:/")?,
+    ];
+    let cwd = volumes[0].join("work")?;
+    let secret = volumes[0].join("private")?;
+    let formerly_readonly = volumes[1].join("scoped")?;
+    let reopened = secret.join("reopened")?;
+    let root_git = volumes[0].join(".git")?;
+    let denied_root_metadata = volumes[0].join(".agents")?;
+    let workspace_git = cwd.join(".git/FETCH_HEAD")?;
+    let context = FileSystemSandboxPolicyContext {
+        cwd: &cwd,
+        workspace_roots: std::slice::from_ref(&cwd),
+        user_home_dir: None,
+        temporary_directories: Some(&[]),
+    };
+    let mut policy = FileSystemSandboxPolicy::read_only();
+    policy.entries.extend([
+        FileSystemSandboxEntry::new(secret.clone().into(), Deny),
+        FileSystemSandboxEntry::new(denied_root_metadata.clone().into(), Deny),
+        FileSystemSandboxEntry::new(reopened.clone().into(), Write),
+        FileSystemSandboxEntry::new(formerly_readonly.clone().into(), Read),
+    ]);
+    let approved = policy.for_approved_command(&context);
+    assert_eq!(approved.resolve_access(&formerly_readonly, &context), Deny);
+    let translated = materialize_volume_roots(approved, &volumes)?;
+    assert_eq!(
+        [
+            &volumes[0],
+            &volumes[1],
+            &formerly_readonly,
+            &secret,
+            &reopened,
+        ]
+        .map(|path| translated.resolve_access(path, &context)),
+        [Write, Write, Write, Deny, Deny]
+    );
+    assert_eq!(
+        [&root_git, &denied_root_metadata, &workspace_git]
+            .map(|path| translated.can_write_path(path, &context)),
+        [true, false, true]
+    );
+    Ok(())
+}
+
 fn canonical_root(temp: &tempfile::TempDir) -> Result<PathBuf> {
     Ok(
         PathUri::from_host_native_path(std::fs::canonicalize(temp.path())?)?

codex-rs/protocol/src/permissions.rs

@@ -248,6 +248,12 @@ enum WritableRootPathResolution {
     PreserveMutableComponents,
 }
 
+#[derive(Clone, Copy)]
+enum RootMetadataWriteMounts {
+    Separate,
+    InheritWritableRoot,
+}
+
 impl WritableRootPathResolution {
     fn resolve(self, path: AbsolutePathBuf) -> AbsolutePathBuf {
         match self {
@@ -1472,7 +1478,27 @@ impl FileSystemSandboxPolicy {
     /// Returns the writable roots together with read-only carveouts resolved
     /// against the provided cwd.
     pub fn get_writable_roots_with_cwd(&self, cwd: &Path) -> Vec<WritableRoot> {
-        self.get_writable_roots_with_cwd_impl(cwd, WritableRootPathResolution::Effective)
+        self.get_writable_roots_with_cwd_impl(
+            cwd,
+            WritableRootPathResolution::Effective,
+            RootMetadataWriteMounts::Separate,
+        )
+    }
+
+    /// Omits redundant root-metadata mounts when the filesystem root is writable.
+    ///
+    /// Explicit root metadata writes still disable their default protection. Linux
+    /// inherits those writes from its root bind so a metadata symlink cannot cause
+    /// a second bind to reopen its target under an explicitly denied directory.
+    pub fn get_writable_roots_with_cwd_inheriting_root_metadata(
+        &self,
+        cwd: &Path,
+    ) -> Vec<WritableRoot> {
+        self.get_writable_roots_with_cwd_impl(
+            cwd,
+            WritableRootPathResolution::Effective,
+            RootMetadataWriteMounts::InheritWritableRoot,
+        )
     }
 
     /// Reports configured writable roots for diagnostics without inspecting the filesystem.
@@ -1513,6 +1539,7 @@ impl FileSystemSandboxPolicy {
         self.get_writable_roots_with_cwd_impl(
             cwd,
             WritableRootPathResolution::PreserveMutableComponents,
+            RootMetadataWriteMounts::Separate,
         )
     }
 
@@ -1520,6 +1547,7 @@ impl FileSystemSandboxPolicy {
         &self,
         cwd: &Path,
         path_resolution: WritableRootPathResolution,
+        root_metadata_mounts: RootMetadataWriteMounts,
     ) -> Vec<WritableRoot> {
         if self.has_full_disk_write_access() {
             return Vec::new();
@@ -1604,9 +1632,21 @@ impl FileSystemSandboxPolicy {
                 }
             }))
             .collect();
+        let inherits_root_metadata = matches!(
+            root_metadata_mounts,
+            RootMetadataWriteMounts::InheritWritableRoot
+        ) && prepared_entries.iter().any(|entry| {
+            entry.entry.access.can_write() && entry.entry.path.as_path().parent().is_none()
+        });
         let writable_entries: Vec<&PreparedFileSystemEntry<'_>> = prepared_entries
             .iter()
             .filter(|entry| entry.entry.access.can_write())
+            .filter(|entry| {
+                let path = entry.entry.path.as_path();
+                !inherits_root_metadata
+                    || path.parent().is_none_or(|parent| parent.parent().is_some())
+                    || !path.file_name().is_some_and(is_protected_metadata_name)
+            })
             .collect();
 
         let effective_cwd = AbsolutePathBuf::from_absolute_path(cwd)
@@ -1751,6 +1791,27 @@ impl FileSystemSandboxPolicy {
         )
     }
 
+    /// Includes literal denies alongside their resolved targets so Linux can reject a denial
+    /// through a writable symlink instead of only masking the symlink's current target.
+    pub fn get_unreadable_roots_with_cwd_preserving_symlinks(
+        &self,
+        cwd: &Path,
+    ) -> Vec<AbsolutePathBuf> {
+        let mut roots = self.get_unreadable_roots_with_cwd(cwd);
+        if matches!(self.kind, FileSystemSandboxKind::Restricted) {
+            for entry in self.resolved_entries_with_cwd(cwd) {
+                if entry.access == FileSystemAccessMode::Deny
+                    && entry.path.as_path().parent().is_some()
+                    && !self.can_read_local_path_with_cwd(entry.path.as_path(), cwd)
+                    && !roots.contains(&entry.path)
+                {
+                    roots.push(entry.path);
+                }
+            }
+        }
+        roots
+    }
+
     /// Returns unreadable glob patterns resolved against the provided cwd.
     pub fn get_unreadable_globs_with_cwd(&self, cwd: &Path) -> Vec<String> {
         if !matches!(self.kind, FileSystemSandboxKind::Restricted) {

codex-rs/protocol/src/permissions/target.rs

@@ -2,11 +2,14 @@
 
 use super::FileSystemAccessMode;
 use super::FileSystemPath;
+use super::FileSystemSandboxEntry;
 use super::FileSystemSandboxKind;
 use super::FileSystemSandboxPolicy;
 use super::FileSystemSandboxPolicyContext;
 use super::FileSystemSpecialPath;
 use super::InvalidDenyReadGlobBehavior;
+use super::PROJECT_ROOTS_GLOB_PATTERN_PREFIX;
+use super::PROTECTED_METADATA_PATH_NAMES;
 use super::ReadDenyMatcher;
 use super::deny_read_validator::DenyReadValidator;
 use super::deny_read_validator::has_context_read_denials;
@@ -40,6 +43,70 @@ impl ReadDenyMatcher {
 }
 
 impl FileSystemSandboxPolicy {
+    /// Grants root and root-metadata write for an approved command unless explicitly denied.
+    /// If a retained denial cannot be resolved or denies the root, keep the original policy.
+    pub fn for_approved_command(&self, context: &FileSystemSandboxPolicyContext<'_>) -> Self {
+        if self.kind != FileSystemSandboxKind::Restricted {
+            return self.clone();
+        }
+        let mut approved = Self::restricted(vec![FileSystemSandboxEntry::new(
+            FileSystemPath::Special {
+                value: FileSystemSpecialPath::Root,
+            },
+            FileSystemAccessMode::Write,
+        )]);
+        approved.preserve_deny_read_restrictions_from(self);
+        if context.workspace_roots.is_empty()
+            && approved.entries.iter().any(|entry| {
+                matches!(
+                    &entry.path,
+                    FileSystemPath::Special {
+                        value: FileSystemSpecialPath::ProjectRoots { .. }
+                    }
+                ) || matches!(
+                    &entry.path,
+                    FileSystemPath::GlobPattern { pattern }
+                        if pattern.starts_with(PROJECT_ROOTS_GLOB_PATTERN_PREFIX)
+                )
+            })
+        {
+            return self.clone();
+        }
+        let Some(mut approved) =
+            approved.try_materialize_project_roots_with_path_uris(context.workspace_roots)
+        else {
+            return self.clone();
+        };
+        let Some(root) = file_system_root(context) else {
+            return self.clone();
+        };
+        let Ok(matcher) = ReadDenyMatcher::try_new_with_context(&approved, context) else {
+            return self.clone();
+        };
+        if !approved.resolve_access(&root, context).can_write()
+            || matcher
+                .as_ref()
+                .is_some_and(|matcher| matcher.is_read_denied_uri(&root, context))
+        {
+            return self.clone();
+        }
+        for name in PROTECTED_METADATA_PATH_NAMES {
+            let Ok(path) = root.join_descendant(name) else {
+                return self.clone();
+            };
+            if !matcher
+                .as_ref()
+                .is_some_and(|matcher| matcher.is_read_denied_uri(&path, context))
+            {
+                approved.entries.push(FileSystemSandboxEntry::new(
+                    path.into(),
+                    FileSystemAccessMode::Write,
+                ));
+            }
+        }
+        approved
+    }
+
     /// Checks exact-root read access using paths owned by the execution host.
     /// Deny glob enforcement is checked separately with [`ReadDenyMatcher`].
     pub fn can_read_path(
@@ -193,3 +260,7 @@ fn validate_context_paths(
 #[cfg(test)]
 #[path = "target_tests.rs"]
 mod tests;
+
+#[cfg(test)]
+#[path = "target_approved_materialization_tests.rs"]
+mod approved_materialization_tests;

codex-rs/protocol/src/permissions/target_approved_materialization_tests.rs

@@ -0,0 +1,246 @@
+//! Approved commands widen filesystem access without weakening explicit read denials.
+
+use super::*;
+use crate::permissions::FileSystemAccessMode::Deny;
+use crate::permissions::FileSystemAccessMode::Read;
+use crate::permissions::FileSystemAccessMode::Write;
+use crate::permissions::PROTECTED_METADATA_PATH_NAMES;
+use crate::permissions::RawFileSystemSandboxPolicy;
+use crate::permissions::project_roots_glob_pattern;
+use pretty_assertions::assert_eq;
+use std::path::Path;
+
+fn uri(input: &str) -> PathUri {
+    PathUri::parse(input).unwrap()
+}
+
+fn entry(path: impl Into<FileSystemPath>, access: FileSystemAccessMode) -> FileSystemSandboxEntry {
+    FileSystemSandboxEntry::new(path.into(), access)
+}
+
+fn workspace(subpath: &str) -> FileSystemPath {
+    FileSystemPath::Special {
+        value: FileSystemSpecialPath::project_roots(Some(subpath.into())),
+    }
+}
+
+fn glob(pattern: String) -> FileSystemPath {
+    FileSystemPath::GlobPattern { pattern }
+}
+
+fn context<'a>(cwd: &'a PathUri, roots: &'a [PathUri]) -> FileSystemSandboxPolicyContext<'a> {
+    FileSystemSandboxPolicyContext {
+        cwd,
+        workspace_roots: roots,
+        user_home_dir: None,
+        temporary_directories: Some(&[]),
+    }
+}
+
+#[test]
+fn approved_command_grants_root_metadata_unless_explicitly_denied() {
+    for path in [
+        "file:///work/repo",
+        "file:///C:/work/repo",
+        "file://server/share/repo",
+    ] {
+        let cwd = uri(path);
+        let context = context(&cwd, std::slice::from_ref(&cwd));
+        let root = file_system_root(&context).unwrap();
+        let private = cwd.join("private").unwrap();
+        let pattern = project_roots_glob_pattern(Path::new("**/*.env"));
+        let mut policy = FileSystemSandboxPolicy::read_only();
+        policy.entries.extend([
+            entry(cwd.clone(), Write),
+            entry(workspace("private"), Deny),
+            entry(private.join("public").unwrap(), Read),
+            entry(private.join("writable").unwrap(), Write),
+            entry(glob(pattern), Deny),
+        ]);
+        let approved = policy.for_approved_command(&context);
+        let mut expected = FileSystemSandboxPolicy::restricted(vec![
+            entry(
+                FileSystemPath::Special {
+                    value: FileSystemSpecialPath::Root,
+                },
+                Write,
+            ),
+            entry(private, Deny),
+            entry(
+                glob(cwd.join("**/*.env").unwrap().inferred_native_path_string()),
+                Deny,
+            ),
+        ]);
+        expected.entries.extend(
+            PROTECTED_METADATA_PATH_NAMES
+                .iter()
+                .map(|name| entry(root.join_descendant(name).unwrap(), Write)),
+        );
+        assert_eq!(approved, expected);
+        let raw = RawFileSystemSandboxPolicy::try_from(approved.clone()).unwrap();
+        let wire: RawFileSystemSandboxPolicy =
+            serde_json::from_value(serde_json::to_value(raw).unwrap()).unwrap();
+        assert_eq!(FileSystemSandboxPolicy::try_from(wire).unwrap(), approved);
+        for name in PROTECTED_METADATA_PATH_NAMES {
+            assert!(approved.can_write_path(&root.join_descendant(name).unwrap(), &context));
+        }
+        assert!(approved.can_write_path(&cwd.join(".git/FETCH_HEAD").unwrap(), &context));
+        let matcher = ReadDenyMatcher::try_new_with_context(&approved, &context)
+            .unwrap()
+            .unwrap();
+        for denied in [
+            "private/public/file",
+            "private/writable/file",
+            "nested/secret.env",
+        ] {
+            assert!(
+                matcher.is_read_denied_uri(&cwd.join(denied).unwrap(), &context),
+                "{path}: {denied}"
+            );
+        }
+
+        let root_path = |name| root.join_descendant(name).unwrap();
+        let ordinary = FileSystemSandboxPolicy::restricted(vec![
+            entry(root.clone(), Write),
+            entry(root_path(".git"), Deny),
+            entry(
+                glob(root_path(".agent?").inferred_native_path_string()),
+                Deny,
+            ),
+            entry(root_path(".codex/private"), Deny),
+            entry(
+                glob(root_path(".codex/*.env").inferred_native_path_string()),
+                Deny,
+            ),
+        ]);
+        let approved = ordinary.for_approved_command(&context);
+        let metadata_paths = [".git", ".agents", ".codex"].map(root_path);
+        assert_eq!(
+            metadata_paths
+                .each_ref()
+                .map(|path| ordinary.can_write_path(path, &context)),
+            [false, false, false],
+        );
+        assert_eq!(
+            metadata_paths
+                .each_ref()
+                .map(|path| approved.can_write_path(path, &context)),
+            [false, false, true],
+        );
+        assert!(approved.can_write_path(&root_path(".codex/public"), &context));
+        assert!(!approved.can_write_path(&root_path(".codex/private/file"), &context));
+        let matcher = ReadDenyMatcher::try_new_with_context(&approved, &context)
+            .unwrap()
+            .unwrap();
+        assert_eq!(
+            [
+                ".git",
+                ".agents",
+                ".codex/private/file",
+                ".codex/secret.env",
+                ".codex/public"
+            ]
+            .map(|path| matcher.is_read_denied_uri(&root_path(path), &context)),
+            [true, true, true, true, false],
+        );
+    }
+}
+
+#[cfg(unix)]
+#[test]
+fn linux_root_metadata_writes_inherit_the_root_without_reopening_other_paths() {
+    let temp = tempfile::tempdir().unwrap();
+    let cwd = temp.path().canonicalize().unwrap();
+    let root = uri("file:///");
+    let mut policy = FileSystemSandboxPolicy::restricted(vec![
+        entry(root.clone(), Write),
+        entry(
+            PathUri::from_host_native_path(cwd.join("private")).unwrap(),
+            Deny,
+        ),
+    ]);
+    let roots = policy.get_writable_roots_with_cwd_inheriting_root_metadata(&cwd);
+    assert_eq!(
+        roots[0].protected_metadata_names,
+        PROTECTED_METADATA_PATH_NAMES
+            .iter()
+            .map(std::string::ToString::to_string)
+            .collect::<Vec<_>>(),
+    );
+
+    let metadata = PROTECTED_METADATA_PATH_NAMES
+        .iter()
+        .map(|name| entry(root.join_descendant(name).unwrap(), Write))
+        .collect::<Vec<_>>();
+    policy.entries.extend(metadata.clone());
+    let roots = policy.get_writable_roots_with_cwd_inheriting_root_metadata(&cwd);
+    assert_eq!(
+        roots
+            .iter()
+            .map(|root| root.root.as_path())
+            .collect::<Vec<_>>(),
+        [Path::new("/")],
+    );
+    assert!(roots[0].protected_metadata_names.is_empty());
+
+    let standalone = FileSystemSandboxPolicy::restricted(metadata);
+    assert_eq!(
+        standalone.get_writable_roots_with_cwd_inheriting_root_metadata(&cwd),
+        standalone.get_writable_roots_with_cwd(&cwd),
+    );
+}
+
+#[test]
+fn approved_command_discards_other_volume_grants_before_materialization() {
+    let cwd = uri("file:///C:/run");
+    let workspace_root = uri("file:///C:/work");
+    let context = context(&cwd, std::slice::from_ref(&workspace_root));
+    let mut policy = FileSystemSandboxPolicy::read_only();
+    policy.entries.extend([
+        entry(workspace(r"D:\vault"), Read),
+        entry(uri("file:///D:/scoped"), Write),
+        entry(workspace("private"), Deny),
+    ]);
+    let approved = policy.for_approved_command(&context);
+    assert_eq!(
+        [
+            "file:///C:/work/public",
+            "file:///C:/work/private/file",
+            "file:///D:/scoped"
+        ]
+        .map(|path| approved.resolve_access(&uri(path), &context)),
+        [Write, Deny, Deny],
+    );
+}
+
+#[test]
+fn approved_command_declines_root_or_unresolvable_denials() {
+    for path in ["file:///work/repo", "file:///C:/work/repo"] {
+        let cwd = uri(path);
+        let known = context(&cwd, std::slice::from_ref(&cwd));
+        let root = file_system_root(&known).unwrap();
+        for (context, denied) in [
+            (
+                known,
+                FileSystemPath::Special {
+                    value: FileSystemSpecialPath::Root,
+                },
+            ),
+            (known, root.clone().into()),
+            (known, glob(root.inferred_native_path_string())),
+            (known, glob("secrets/[z-a]".into())),
+            (known, workspace("~/private")),
+            (context(&cwd, &[]), workspace("private")),
+            (
+                context(&cwd, &[]),
+                glob(project_roots_glob_pattern(Path::new("**/*.env"))),
+            ),
+        ] {
+            let mut policy = FileSystemSandboxPolicy::read_only();
+            policy
+                .entries
+                .extend([entry(denied, Deny), entry(cwd.clone(), Write)]);
+            assert_eq!(policy.for_approved_command(&context), policy);
+        }
+    }
+}