From 22dea110204c8a07abcbd6495d7eeae65faa0d9f Mon Sep 17 00:00:00 2001 From: pash-openai Date: Fri, 18 Sep 2026 19:01:52 +0000 Subject: [PATCH] Allow unrelated namespace mounts in Linux sandbox socket checks (#46535) ## Why Namespace mounts can have roots such as `mnt:[inode]` or `net:[inode]` that are not filesystem paths. Treating every mount root as a path rejects these unrelated mounts and prevents sandbox startup. ## What changed Parse mount roots as paths only for the daemon socket's filesystem. Continue validating every mount destination and checking mount ancestry, nested mounts, and socket aliases. ## Testing Add regression cases for namespace mounts, invalid destinations, and socket aliases, both with and without a mount ID. Extend the socket-isolation integration fixture with an unrelated network namespace mount to exercise successful startup and rejection of a real socket alias. GitOrigin-RevId: 9a1d2a69e5bd3f8d09b1de7cf998ee6d9c6bb980 --- codex-rs/linux-sandbox/src/daemon_mounts.rs | 14 ++++-- .../linux-sandbox/src/daemon_mounts_tests.rs | 45 +++++++++++++++++++ .../tests/suite/daemon_sockets_tests.rs | 9 ++++ 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/codex-rs/linux-sandbox/src/daemon_mounts.rs b/codex-rs/linux-sandbox/src/daemon_mounts.rs index 50a65636b4..9be0dd9817 100644 --- a/codex-rs/linux-sandbox/src/daemon_mounts.rs +++ b/codex-rs/linux-sandbox/src/daemon_mounts.rs @@ -62,8 +62,13 @@ fn check_mounts( let [id, parent, mount_device, root, destination] = fields.as_slice() else { return Err(invalid()); }; - let root = mount_path(root)?; let destination = mount_path(destination)?; + // Only roots on the socket filesystem can identify aliases. Other + // filesystems can use non-path roots such as nsfs `mnt:[inode]`, but + // their destinations still matter for ancestry and nested-mount checks. + let root = (*mount_device == device.as_bytes()) + .then(|| mount_path(root)) + .transpose()?; mounts.push((*id, *parent, *mount_device, root, destination)); } let (location, containing_mount) = if let Some(mount_id) = mount_id { @@ -77,6 +82,7 @@ fn check_mounts( if *mount_device != device.as_bytes() { return Err(invalid()); } + let root = root.as_ref().ok_or_else(invalid)?; let relative = directory.strip_prefix(destination).map_err(|_| invalid())?; let mut current = Some(selected); let mut visible_child: Option<&Path> = None; @@ -106,8 +112,8 @@ fn check_mounts( // on the backing location, and do not assume any aliases are hidden. let locations: BTreeSet<_> = mounts .iter() - .filter(|(_, _, mount_device, ..)| *mount_device == device.as_bytes()) .filter_map(|(_, _, _, root, destination)| { + let root = root.as_ref()?; directory .strip_prefix(destination) .ok() @@ -119,10 +125,10 @@ fn check_mounts( } (locations.into_iter().next().ok_or_else(invalid)?, None) }; - for (id, _, mount_device, root, destination) in &mounts { + for (id, _, _, root, destination) in &mounts { // Nested mounts can introduce another filesystem (or an individual socket) under the mask. let nested = destination != directory && destination.starts_with(directory); - let alias = if *mount_device == device.as_bytes() { + let alias = if let Some(root) = root { if let Ok(relative) = location.strip_prefix(root) { Some(destination.join(relative)) } else if root.starts_with(&location) { diff --git a/codex-rs/linux-sandbox/src/daemon_mounts_tests.rs b/codex-rs/linux-sandbox/src/daemon_mounts_tests.rs index 058ed2b2e0..f362636727 100644 --- a/codex-rs/linux-sandbox/src/daemon_mounts_tests.rs +++ b/codex-rs/linux-sandbox/src/daemon_mounts_tests.rs @@ -41,6 +41,51 @@ fn rejects_only_mounts_that_compromise_the_directory(root: &str, destination: &s } } +#[test_case("0:2", "mnt:[4026532835]", "/run/snapd/ns/example.mnt", Ok(()); "unrelated mount namespace")] +#[test_case("0:2", "net:[4026531840]", "/run/netns/example", Ok(()); "unrelated network namespace")] +#[test_case("0:2", "mnt:[4026532835]", "/tmp/codex-daemon-1000/ns", Err(io::ErrorKind::PermissionDenied); "nested namespace mount")] +#[test_case("0:1", "mnt:[4026532835]", "/run/snapd/ns/example.mnt", Err(io::ErrorKind::Other); "non-path root on socket filesystem")] +#[test_case("0:2", "mnt:[4026532835]", "relative/ns", Err(io::ErrorKind::Other); "relative destination")] +#[test_case("0:2", "mnt:[4026532835]", "/run/snapd/ns/\\invalid", Err(io::ErrorKind::Other); "invalid destination escape")] +fn validates_namespace_mounts_by_device_and_destination( + device: &str, + root: &str, + destination: &str, + expected: Result<(), io::ErrorKind>, +) { + let mounts = format!( + "1 0 0:1 / / rw - ext4 disk rw\n2 1 {device} {root} {destination} rw - nsfs nsfs rw\n" + ); + for mount_id in [Some("1"), None] { + assert_eq!( + check_mounts( + Path::new("/tmp/codex-daemon-1000"), + "0:1", + mount_id, + mounts.as_bytes() + ) + .map_err(|error| error.kind()), + expected, + "mount_id: {mount_id:?}" + ); + } +} + +#[test] +fn unrelated_namespace_mount_does_not_hide_a_socket_alias() { + let mounts = b"1 0 0:1 / / rw - ext4 disk rw\n\ + 2 1 0:2 net:[4026531840] /run/netns/example rw - nsfs nsfs rw\n\ + 3 1 0:1 /tmp/codex-daemon-1000 /alias rw - ext4 disk rw\n"; + for mount_id in [Some("1"), None] { + assert_eq!( + check_mounts(Path::new("/tmp/codex-daemon-1000"), "0:1", mount_id, mounts) + .map_err(|error| error.kind()), + Err(io::ErrorKind::PermissionDenied), + "mount_id: {mount_id:?}" + ); + } +} + #[test] fn rejects_alias_when_tmp_is_itself_a_bind_mount() { let mounts = b"1 0 0:1 / / rw - ext4 disk rw\n2 1 0:1 /backing/tmp /tmp rw - ext4 disk rw\n"; diff --git a/codex-rs/linux-sandbox/tests/suite/daemon_sockets_tests.rs b/codex-rs/linux-sandbox/tests/suite/daemon_sockets_tests.rs index 30889d0f9d..6de557a87a 100644 --- a/codex-rs/linux-sandbox/tests/suite/daemon_sockets_tests.rs +++ b/codex-rs/linux-sandbox/tests/suite/daemon_sockets_tests.rs @@ -65,6 +65,15 @@ fn private_tmp_fixture() { if !mount.status.success() { std::process::exit(/*code*/ 77); } + // Namespace mounts have non-path roots such as `net:[inode]`. An unrelated + // one must not stop startup or disable the socket-isolation checks below. + let namespace = "/tmp/network-namespace"; + std::fs::File::create(namespace).unwrap(); + let mount = std::process::Command::new("mount") + .args(["--bind", "/proc/self/ns/net", namespace]) + .output() + .unwrap(); + assert!(mount.status.success(), "{mount:?}"); let root = codex_uds::prepare_shared_daemon_socket_directory().unwrap(); let endpoint = root.join("rpc.sock"); let _daemon = UnixListener::bind(&endpoint).unwrap();