mirror of
https://github.com/openai/codex.git
synced 2026-10-23 18:08:45 +00:00
Make sandbox mount cleanup tests tolerate file descriptor reuse (#56454)
## Why Concurrent tests can reuse a closed file descriptor number, causing cleanup assertions that require `EBADF` to fail even when the inherited directory descriptor was closed. ## What changed Capture the inherited directory's device and inode before mount verification. Check `/proc/self/fd` afterward and accept either a missing descriptor or one referring to a different inode, while still rejecting a leaked descriptor to the original directory. Apply this check to successful verification and rejected mount markers. GitOrigin-RevId: 42ff3aebcee9c37c481772f8a425a8643708c426
This commit is contained in:
@@ -1,7 +1,26 @@
|
||||
//! Check mount validation and cleanup without assuming closed descriptor numbers stay unused.
|
||||
|
||||
use super::verify_fd_mounts;
|
||||
use pretty_assertions::assert_eq;
|
||||
use pretty_assertions::assert_ne;
|
||||
use std::fs::File;
|
||||
use std::fs::Metadata;
|
||||
use std::os::fd::IntoRawFd;
|
||||
use std::os::fd::RawFd;
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
|
||||
// Another test can reuse the number after closure; only the original directory is a leak.
|
||||
#[track_caller]
|
||||
fn assert_inherited_descriptor_closed(descriptor: RawFd, inherited: &Metadata) {
|
||||
match std::fs::metadata(format!("/proc/self/fd/{descriptor}")) {
|
||||
Ok(current) => assert_ne!(
|
||||
(current.dev(), current.ino()),
|
||||
(inherited.dev(), inherited.ino()),
|
||||
"the inherited directory descriptor must be closed"
|
||||
),
|
||||
Err(error) => assert_eq!(error.kind(), std::io::ErrorKind::NotFound),
|
||||
}
|
||||
}
|
||||
|
||||
/// A matching mount authenticates its original inode and closes the inherited descriptor.
|
||||
#[test]
|
||||
@@ -10,15 +29,14 @@ fn matching_mount_closes_inherited_descriptor() {
|
||||
let descriptor = File::open(root.path())
|
||||
.expect("directory descriptor should open")
|
||||
.into_raw_fd();
|
||||
let descriptor_path = format!("/proc/self/fd/{descriptor}");
|
||||
let inherited = std::fs::metadata(&descriptor_path)
|
||||
.expect("inherited descriptor metadata should be accessible");
|
||||
let marker = format!("{descriptor}:{}", root.path().display());
|
||||
|
||||
verify_fd_mounts(&[marker]).expect("matching mount should verify");
|
||||
|
||||
assert_eq!(unsafe { libc::fcntl(descriptor, libc::F_GETFD) }, -1);
|
||||
assert_eq!(
|
||||
std::io::Error::last_os_error().raw_os_error(),
|
||||
Some(libc::EBADF)
|
||||
);
|
||||
assert_inherited_descriptor_closed(descriptor, &inherited);
|
||||
}
|
||||
|
||||
/// A swapped destination fails closed without leaking the original descriptor.
|
||||
@@ -29,16 +47,15 @@ fn mismatched_mount_closes_inherited_descriptor() {
|
||||
let descriptor = File::open(source.path())
|
||||
.expect("source directory descriptor should open")
|
||||
.into_raw_fd();
|
||||
let descriptor_path = format!("/proc/self/fd/{descriptor}");
|
||||
let inherited = std::fs::metadata(&descriptor_path)
|
||||
.expect("inherited descriptor metadata should be accessible");
|
||||
let marker = format!("{descriptor}:{}", destination.path().display());
|
||||
|
||||
let error = verify_fd_mounts(&[marker]).expect_err("different inodes must be rejected");
|
||||
|
||||
assert_eq!(error.kind(), std::io::ErrorKind::PermissionDenied);
|
||||
assert_eq!(unsafe { libc::fcntl(descriptor, libc::F_GETFD) }, -1);
|
||||
assert_eq!(
|
||||
std::io::Error::last_os_error().raw_os_error(),
|
||||
Some(libc::EBADF)
|
||||
);
|
||||
assert_inherited_descriptor_closed(descriptor, &inherited);
|
||||
}
|
||||
|
||||
/// A symlink to the original inode is not itself the authenticated mount.
|
||||
@@ -53,16 +70,15 @@ fn symlinked_mount_destination_closes_inherited_descriptor() {
|
||||
let descriptor = File::open(&source)
|
||||
.expect("source directory descriptor should open")
|
||||
.into_raw_fd();
|
||||
let descriptor_path = format!("/proc/self/fd/{descriptor}");
|
||||
let inherited = std::fs::metadata(&descriptor_path)
|
||||
.expect("inherited descriptor metadata should be accessible");
|
||||
let marker = format!("{descriptor}:{}", destination.display());
|
||||
|
||||
let error = verify_fd_mounts(&[marker]).expect_err("symlinked mounts must be rejected");
|
||||
|
||||
assert_eq!(error.kind(), std::io::ErrorKind::PermissionDenied);
|
||||
assert_eq!(unsafe { libc::fcntl(descriptor, libc::F_GETFD) }, -1);
|
||||
assert_eq!(
|
||||
std::io::Error::last_os_error().raw_os_error(),
|
||||
Some(libc::EBADF)
|
||||
);
|
||||
assert_inherited_descriptor_closed(descriptor, &inherited);
|
||||
}
|
||||
|
||||
/// Malformed mount markers cannot claim standard streams or relative destinations.
|
||||
@@ -84,11 +100,14 @@ fn malformed_mount_markers_are_rejected() {
|
||||
let descriptor = File::open(root.path())
|
||||
.expect("directory descriptor should open")
|
||||
.into_raw_fd();
|
||||
let descriptor_path = format!("/proc/self/fd/{descriptor}");
|
||||
let inherited = std::fs::metadata(&descriptor_path)
|
||||
.expect("inherited descriptor metadata should be accessible");
|
||||
let error = verify_fd_mounts(&[format!("{descriptor}:relative")])
|
||||
.expect_err("relative mount destinations must be rejected");
|
||||
|
||||
assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput);
|
||||
assert_eq!(unsafe { libc::fcntl(descriptor, libc::F_GETFD) }, -1);
|
||||
assert_inherited_descriptor_closed(descriptor, &inherited);
|
||||
}
|
||||
|
||||
/// A transferred descriptor can be consumed only once even if a marker is repeated.
|
||||
@@ -98,11 +117,14 @@ fn duplicate_mount_descriptors_are_rejected() {
|
||||
let descriptor = File::open(root.path())
|
||||
.expect("directory descriptor should open")
|
||||
.into_raw_fd();
|
||||
let descriptor_path = format!("/proc/self/fd/{descriptor}");
|
||||
let inherited = std::fs::metadata(&descriptor_path)
|
||||
.expect("inherited descriptor metadata should be accessible");
|
||||
let marker = format!("{descriptor}:{}", root.path().display());
|
||||
|
||||
let error = verify_fd_mounts(&[marker.clone(), marker])
|
||||
.expect_err("a mount descriptor must not be consumed twice");
|
||||
|
||||
assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput);
|
||||
assert_eq!(unsafe { libc::fcntl(descriptor, libc::F_GETFD) }, -1);
|
||||
assert_inherited_descriptor_closed(descriptor, &inherited);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user