Harden bubblewrap synthetic mount registry isolation (#40302)

## Why

The synthetic mount registry must remain private to the sandbox helper. A
writable bind overlapping its temporary directory could expose the registry,
and a pre-existing registry symlink could redirect bookkeeping into a writable
location.

## What changed

- Add the registry's read-only bind while constructing filesystem mounts, but
  only when a writable bind exposes it and before applying deny masks.
- Reject a symlinked registry path before registering mounts or starting the
  sandboxed command.

## Testing

Add coverage for symlink rejection and for denied temporary directories under
both full-root and minimal filesystem policies.

GitOrigin-RevId: 3adff28998f0ef8c8ec26a73e5a1183b4931c0e8

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

@@ -24,6 +24,7 @@ use std::path::Path;
 use std::path::PathBuf;
 use std::process::Command;
 
+use crate::linux_run_main::synthetic_mount_registry_root;
 use codex_protocol::error::CodexErr;
 use codex_protocol::error::Result;
 use codex_protocol::permissions::is_protected_metadata_name;
@@ -608,6 +609,23 @@ fn create_filesystem_args(
         for subpath in read_only_subpaths {
             append_read_only_subpath_args(&mut bwrap_args, &subpath, &allowed_write_paths)?;
         }
+        // Protect the registry only where a writable bind exposes it. Apply
+        // this before deny masks so a denied parent stays hidden, rather than
+        // trying to recreate the registry beneath an already read-only tmpfs.
+        let registry_root = synthetic_mount_registry_root();
+        let registry_mount = if registry_root.starts_with(mount_root) {
+            Some(registry_root.as_path())
+        } else if mount_root.starts_with(&registry_root) {
+            Some(mount_root)
+        } else {
+            None
+        };
+        if let Some(registry_mount) = registry_mount {
+            fs::create_dir_all(&registry_root)?;
+            bwrap_args.args.push("--ro-bind".to_string());
+            bwrap_args.args.push(path_to_string(registry_mount));
+            bwrap_args.args.push(path_to_string(registry_mount));
+        }
         let mut nested_unreadable_roots: Vec<PathBuf> = unreadable_roots
             .iter()
             .filter(|path| path.starts_with(root))
@@ -1687,6 +1705,46 @@ mod tests {
         assert!(message.contains(&real_linked_private_str), "{message}");
     }
 
+    #[test]
+    fn registry_protection_precedes_denied_parent_mask() {
+        let registry_root = synthetic_mount_registry_root();
+        let temp_root = registry_root.parent().expect("registry parent");
+        let policy = FileSystemSandboxPolicy::restricted(vec![
+            FileSystemSandboxEntry {
+                path: FileSystemPath::Special {
+                    value: FileSystemSpecialPath::Root,
+                },
+                access: FileSystemAccessMode::Write,
+                missing_path_behavior: None,
+            },
+            FileSystemSandboxEntry {
+                path: AbsolutePathBuf::try_from(temp_root)
+                    .expect("absolute temp root")
+                    .into(),
+                access: FileSystemAccessMode::Deny,
+                missing_path_behavior: None,
+            },
+        ]);
+
+        let args =
+            create_filesystem_args(&policy, Path::new("/"), NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH)
+                .expect("filesystem args");
+        let registry_path = path_to_string(&registry_root);
+        let temp_path = path_to_string(temp_root);
+        let registry_bind = args
+            .args
+            .windows(3)
+            .position(|args| args == ["--ro-bind", registry_path.as_str(), registry_path.as_str()]);
+        let temp_mask = args
+            .args
+            .windows(2)
+            .position(|args| args == ["--tmpfs", temp_path.as_str()]);
+        assert!(
+            registry_bind.expect("registry read-only bind") < temp_mask.expect("temp deny mask"),
+            "registry protection must not reopen denied temp paths"
+        );
+    }
+
     #[test]
     fn missing_read_only_subpath_uses_empty_file_bind_data() {
         let temp_dir = TempDir::new().expect("temp dir");
@@ -2053,6 +2111,9 @@ mod tests {
                 "/.codex".to_string(),
                 "--remount-ro".to_string(),
                 "/.codex".to_string(),
+                "--ro-bind".to_string(),
+                path_to_string(&synthetic_mount_registry_root()),
+                path_to_string(&synthetic_mount_registry_root()),
                 // Rebind /dev after the root bind so device nodes remain
                 // writable/usable inside the writable root.
                 "--bind".to_string(),

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

@@ -567,7 +567,7 @@ fn run_or_exec_bwrap(bwrap_args: crate::bwrap::BwrapArgs) -> ! {
 
 fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::BwrapArgs) -> ! {
     let crate::bwrap::BwrapArgs {
-        mut args,
+        args,
         preserved_files,
         synthetic_mount_targets,
         protected_create_targets,
@@ -576,20 +576,6 @@ fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::Bwr
     let synthetic_mount_registrations = register_synthetic_mount_targets(&synthetic_mount_targets);
     let protected_create_registrations =
         register_protected_create_targets(&protected_create_targets);
-    let registry_root = synthetic_mount_registry_root()
-        .to_string_lossy()
-        .into_owned();
-    let Some(command_separator) = args.iter().position(|arg| arg == "--") else {
-        panic!("bubblewrap argv is missing command separator '--'");
-    };
-    args.splice(
-        command_separator..command_separator,
-        [
-            "--ro-bind".to_string(),
-            registry_root.clone(),
-            registry_root,
-        ],
-    );
     let exec_start_pipe = create_exec_start_pipe(!protected_create_targets.is_empty());
     let parent_pid = unsafe { libc::getpid() };
     let pid = unsafe { libc::fork() };
@@ -1347,7 +1333,7 @@ fn synthetic_mount_marker_dir(path: &Path) -> PathBuf {
     synthetic_mount_registry_root().join(format!("{:016x}", hash_path(path)))
 }
 
-fn synthetic_mount_registry_root() -> PathBuf {
+pub(crate) fn synthetic_mount_registry_root() -> PathBuf {
     static REGISTRY_ROOT: OnceLock<PathBuf> = OnceLock::new();
 
     REGISTRY_ROOT
@@ -1360,9 +1346,17 @@ fn synthetic_mount_registry_root() -> PathBuf {
                     temp_dir.display()
                 )
             });
-            temp_dir.join(format!(
+            let registry_root = temp_dir.join(format!(
                 "codex-bwrap-synthetic-mount-targets-{effective_uid}"
-            ))
+            ));
+            // A registry symlink can redirect bookkeeping into a writable root
+            // that does not overlap TMPDIR, bypassing its read-only mount.
+            assert!(
+                !registry_root.is_symlink(),
+                "synthetic mount registry must not be a symlink: {}",
+                registry_root.display()
+            );
+            registry_root
         })
         .clone()
 }

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

@@ -730,6 +730,61 @@ async fn sandbox_reports_codex_symlink_build_failure_without_panicking() {
     );
 }
 
+#[tokio::test]
+async fn sandbox_rejects_symlinked_synthetic_mount_registry() {
+    if should_skip_bwrap_tests().await {
+        eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable");
+        return;
+    }
+
+    let temp = tempfile::tempdir().expect("tempdir");
+    let workspace = temp.path().join("workspace");
+    let registry_target = workspace.join("registry");
+    std::fs::create_dir_all(&registry_target).expect("create registry target");
+    let effective_uid = unsafe { libc::geteuid() };
+    let registry = temp.path().join(format!(
+        "codex-bwrap-synthetic-mount-targets-{effective_uid}"
+    ));
+    std::os::unix::fs::symlink(&registry_target, &registry).expect("symlink registry");
+
+    let cwd = AbsolutePathBuf::try_from(workspace).expect("absolute workspace");
+    let permission_profile = PermissionProfile::workspace_write_with(
+        std::slice::from_ref(&cwd),
+        NetworkSandboxPolicy::Enabled,
+        /*exclude_tmpdir_env_var*/ true,
+        /*exclude_slash_tmp*/ true,
+    );
+    let mut env = create_env_from_core_vars();
+    env.insert("TMPDIR".to_string(), temp.path().display().to_string());
+    let output = expect_denied(
+        run_cmd_result_with_permission_profile_for_cwd(
+            &["sh", "-c", "touch registry/forged-marker"],
+            cwd,
+            permission_profile,
+            env,
+            LONG_TIMEOUT_MS,
+            /*use_legacy_landlock*/ false,
+        )
+        .await,
+        "a symlinked registry must not expose writable bookkeeping",
+    );
+    assert!(
+        output
+            .stderr
+            .text
+            .contains("synthetic mount registry must not be a symlink"),
+        "stderr: {}",
+        output.stderr.text
+    );
+    assert_eq!(
+        std::fs::read_dir(registry_target)
+            .expect("read registry target")
+            .count(),
+        0,
+        "the registry symlink must be rejected before registration or command execution"
+    );
+}
+
 #[tokio::test]
 async fn sandbox_keeps_parent_repo_discovery_while_blocking_child_metadata() {
     if should_skip_bwrap_tests().await {
@@ -958,6 +1013,82 @@ async fn sandbox_blocks_explicit_split_policy_carveouts_under_bwrap() {
     assert_ne!(output.exit_code, 0);
 }
 
+#[tokio::test]
+async fn sandbox_starts_with_denied_tmp_without_exposing_registry() {
+    if should_skip_bwrap_tests().await {
+        eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable");
+        return;
+    }
+
+    let temp = tempfile::tempdir().expect("tempdir");
+    std::fs::write(temp.path().join("AGENTS.md"), "project instructions\n")
+        .expect("write instructions");
+    let cwd = AbsolutePathBuf::try_from(temp.path()).expect("absolute workspace");
+    let sandbox_helper = codex_linux_sandbox_exe();
+    let helper_dir = AbsolutePathBuf::try_from(sandbox_helper.parent().expect("helper parent"))
+        .expect("absolute helper directory");
+
+    for tmp_root in [PathBuf::from("/tmp"), temp.path().join("denied-tmp")] {
+        std::fs::create_dir_all(&tmp_root).expect("create temp root");
+        let secret = NamedTempFile::new_in(&tmp_root).expect("denied file");
+        std::fs::write(secret.path(), "private").expect("write denied file");
+        for read_root in [FileSystemSpecialPath::Root, FileSystemSpecialPath::Minimal] {
+            let denied_path = if tmp_root == std::path::Path::new("/tmp") {
+                FileSystemPath::Special {
+                    value: FileSystemSpecialPath::SlashTmp,
+                }
+            } else {
+                AbsolutePathBuf::try_from(tmp_root.as_path())
+                    .expect("absolute temp root")
+                    .into()
+            };
+            let policy = FileSystemSandboxPolicy::restricted(vec![
+                FileSystemSandboxEntry::new(
+                    FileSystemPath::Special { value: read_root },
+                    FileSystemAccessMode::Read,
+                ),
+                FileSystemSandboxEntry::new(helper_dir.clone().into(), FileSystemAccessMode::Read),
+                FileSystemSandboxEntry::new(cwd.clone().into(), FileSystemAccessMode::Write),
+                FileSystemSandboxEntry::new(denied_path, FileSystemAccessMode::Deny),
+            ]);
+            let mut env = create_env_from_core_vars();
+            env.insert("TMPDIR".to_string(), tmp_root.display().to_string());
+            env.insert(
+                "DENIED_SECRET".to_string(),
+                secret.path().display().to_string(),
+            );
+            let output = run_cmd_result_with_permission_profile_for_cwd(
+                &[
+                    "sh",
+                    "-c",
+                    r#"set -e
+cat AGENTS.md
+test ! -r "$DENIED_SECRET"
+if printf modified > "$DENIED_SECRET" 2>/dev/null; then exit 1; fi
+registry="$TMPDIR/codex-bwrap-synthetic-mount-targets-$(id -u)"
+test ! -e "$registry"
+"#,
+                ],
+                cwd.clone(),
+                PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Enabled),
+                env,
+                LONG_TIMEOUT_MS,
+                /*use_legacy_landlock*/ false,
+            )
+            .await
+            .expect("sandbox should start with denied temp directory");
+            assert_eq!(
+                (output.exit_code, output.stdout.text.as_str()),
+                (0, "project instructions\n"),
+                "stderr: {}",
+                output.stderr.text
+            );
+            assert_eq!(std::fs::read_to_string(secret.path()).unwrap(), "private");
+            assert!(!temp.path().join(".git").exists());
+        }
+    }
+}
+
 #[tokio::test]
 async fn sandbox_reenables_writable_subpaths_under_unreadable_parents() {
     if should_skip_bwrap_tests().await {