Support close-on-exec attachments without changing PTY semantics (#47797)

## Why

Passing extra descriptors to a PTY previously selected behavior intended for inherited escalation sockets: no EOF on stdin close, different signal exit statuses, and no default `SHELL`. Launch attachments need ordinary PTY behavior and must survive execution even when marked `FD_CLOEXEC`.

## What changed

- Add `ChildFds::Attached` alongside `ChildFds::Inherited` to distinguish launch attachments from legacy inherited descriptors.
- Preserve ordinary EOF, default `SHELL`, and signal exit status behavior for PTY attachments.
- Allow explicitly supplied close-on-exec descriptors through pipe and PTY launches without changing the parent's descriptor flags, using child-side flag updates and macOS spawn inheritance actions.
- Update existing callers to use `ChildFds::Inherited`.

## Testing

Add Unix coverage for attachment accessibility, unchanged parent descriptor flags, default `SHELL`, and signal exit statuses. Extend the PTY stdin-draining test to cover attachments and EOF delivery.

GitOrigin-RevId: 864aedc6c5717d8b43c9d71057c9f54409834509
This commit is contained in:
jif
2026-09-24 09:25:19 +00:00
committed by copyberry
parent 29f056c26c
commit 51d4562070
23 changed files with 187 additions and 72 deletions
+1 -1
View File
@@ -367,7 +367,7 @@ async fn pinned_powershell_runs_under_wine_with_a_pty() -> Result<()> {
&env,
/*arg0*/ &None,
TerminalSize::default(),
/*inherited_fds*/ &[],
codex_utils_pty::ChildFds::Inherited(&[]),
)
.await?;
let command_result = timeout(Duration::from_secs(30), async {
+1 -1
View File
@@ -275,7 +275,7 @@ impl CommandExecManager {
&env,
&arg0,
size.unwrap_or_default(),
&[],
codex_utils_pty::ChildFds::Inherited(&[]),
)
.await
} else if stream_stdin {
@@ -314,7 +314,7 @@ impl ProcessExecManager {
&env,
&arg0,
size.unwrap_or_default(),
&[],
codex_utils_pty::ChildFds::Inherited(&[]),
)
.await
} else if stream_stdin {
+1 -1
View File
@@ -196,7 +196,7 @@ async fn daemon_startup(command: &str) -> Result<()> {
rows: 40,
cols: 120,
},
&[],
codex_utils_pty::ChildFds::Inherited(&[]),
)
.await?;
let exit = spawned.exit_rx;
+1 -1
View File
@@ -511,7 +511,7 @@ async fn interactive_tmux_startup_does_not_execute_workspace_helpers() -> Result
rows: 40,
cols: 120,
},
&[],
codex_utils_pty::ChildFds::Inherited(&[]),
)
.await?;
let session = spawned.session;
+1 -1
View File
@@ -108,7 +108,7 @@ async fn run_probe(
env,
/*arg0*/ &None,
TerminalSize::default(),
&[],
codex_utils_pty::ChildFds::Inherited(&[]),
)
.await?;
let mut output = Vec::new();
+2 -2
View File
@@ -89,7 +89,7 @@ async fn rejected_start(
rows: 40,
cols: 120,
},
&[],
codex_utils_pty::ChildFds::Inherited(&[]),
)
.await?;
let mut stdout = spawned.stdout_rx;
@@ -424,7 +424,7 @@ trust_level = "trusted"
rows: 40,
cols: 120,
},
&[],
codex_utils_pty::ChildFds::Inherited(&[]),
)
.await?;
let session = spawned.session;
@@ -1424,7 +1424,7 @@ impl UnifiedExecProcessManager {
windows_sandbox,
tty,
stdin_open: tty,
inherited_fds: &inherited_fds,
inherited_fds: codex_utils_pty::ChildFds::Inherited(&inherited_fds),
})
.await;
spawn_lifecycle.after_spawn();
+1 -1
View File
@@ -422,7 +422,7 @@ impl LocalProcess {
windows_sandbox: prepared.windows_sandbox_spawn_request(),
tty: params.tty,
stdin_open: params.tty || params.pipe_stdin,
inherited_fds: &[],
inherited_fds: codex_utils_pty::ChildFds::Inherited(&[]),
})
.await;
let spawned = match spawned_result {
@@ -64,7 +64,7 @@ async fn hook_shell_startup_does_not_stop_on_controlling_terminal() {
&env,
/*arg0*/ &None,
codex_utils_pty::TerminalSize::default(),
&[],
codex_utils_pty::ChildFds::Inherited(&[]),
)
.await
.expect("spawn test with a controlling terminal");
+3 -3
View File
@@ -37,7 +37,7 @@ pub struct SpawnRequest<'a> {
pub windows_sandbox: Option<WindowsSandboxSpawnRequest<'a>>,
pub tty: bool,
pub stdin_open: bool,
pub inherited_fds: &'a [i32],
pub inherited_fds: codex_utils_pty::ChildFds<'a>,
}
/// Spawn a process using the backend selected by the prepared sandbox request.
@@ -124,7 +124,7 @@ pub async fn spawn_process(request: SpawnRequest<'_>) -> Result<SpawnedProcess>
request.cwd,
request.env,
request.arg0,
request.inherited_fds,
request.inherited_fds.as_slice(),
)
.await
} else {
@@ -134,7 +134,7 @@ pub async fn spawn_process(request: SpawnRequest<'_>) -> Result<SpawnedProcess>
request.cwd,
request.env,
request.arg0,
request.inherited_fds,
request.inherited_fds.as_slice(),
)
.await
};
@@ -79,7 +79,7 @@ async fn direct_terminal_queries_are_answered() -> anyhow::Result<()> {
windows_sandbox: None,
tty: true,
stdin_open: true,
inherited_fds: &[],
inherited_fds: codex_utils_pty::ChildFds::Inherited(&[]),
})
.await?;
let SpawnedProcess {
+9
View File
@@ -209,6 +209,14 @@ impl Command {
self
}
/// Explicit launch attachments, including CLOEXEC descriptors. The caller
/// owns them through spawn; only the child's flags are changed.
#[cfg(unix)]
pub(crate) fn inherit_fds(&mut self, fds: &[std::os::fd::RawFd]) -> &mut Self {
self.inherited_fds = fds.to_vec();
self
}
/// Keep the existing Linux pipe behavior when the spawning parent exits.
#[cfg(target_os = "linux")]
pub fn terminate_on_parent_death(&mut self) -> &mut Self {
@@ -302,6 +310,7 @@ impl Command {
if explicit_fds {
crate::pty::close_inherited_fds_except(&targets);
}
crate::pty::make_fds_inheritable(&targets)?;
Ok(())
});
}
+1
View File
@@ -12,6 +12,7 @@ pub mod pipe;
mod process;
pub mod process_group;
pub mod pty;
pub use pty::ChildFds;
#[cfg(test)]
mod tests;
#[cfg(unix)]
+1 -1
View File
@@ -155,7 +155,7 @@ async fn spawn_process_with_stdin_mode(
command
.process_mode(ProcessMode::NewSession)
.descriptor_policy(DescriptorPolicy::Explicit)
.preserve_fds(inherited_fds);
.inherit_fds(inherited_fds);
#[cfg(target_os = "linux")]
command.terminate_on_parent_death();
#[cfg(not(unix))]
+1 -1
View File
@@ -110,7 +110,7 @@ async fn descriptor_capture_pressure_falls_back_to_direct_spawn() -> anyhow::Res
&HashMap::new(),
&None,
crate::TerminalSize::default(),
&targets,
crate::ChildFds::Inherited(&targets),
)
.await?
}
+11
View File
@@ -185,6 +185,9 @@ impl NativeChild {
))?;
}
for target in &request.inherited_fds {
#[cfg(target_os = "macos")]
let result = posix_spawn_file_actions_addinherit_np(&mut actions.0, *target);
#[cfg(target_os = "linux")]
let result =
libc::posix_spawn_file_actions_adddup2(&mut actions.0, *target, *target);
// macOS file actions can reject valid high descriptors below
@@ -438,6 +441,14 @@ fn cvt_errno(result: libc::c_int) -> io::Result<()> {
}
struct FileActions(libc::posix_spawn_file_actions_t);
#[cfg(target_os = "macos")]
unsafe extern "C" {
fn posix_spawn_file_actions_addinherit_np(
actions: *mut libc::posix_spawn_file_actions_t,
fd: libc::c_int,
) -> libc::c_int;
}
struct Attributes(libc::posix_spawnattr_t);
impl Drop for FileActions {
+53 -11
View File
@@ -41,6 +41,23 @@ use crate::process::TerminalSize;
#[cfg(unix)]
use crate::process::exit_code_from_status;
/// Extra descriptors accompanying a process launch.
#[derive(Clone, Copy)]
pub enum ChildFds<'a> {
/// Existing escalation sockets, with the legacy PTY I/O and status behavior.
Inherited(&'a [i32]),
/// Launch attachments, including CLOEXEC files, with ordinary PTY behavior.
Attached(&'a [i32]),
}
impl<'a> ChildFds<'a> {
pub fn as_slice(self) -> &'a [i32] {
match self {
Self::Inherited(fds) | Self::Attached(fds) => fds,
}
}
}
/// Returns true when ConPTY support is available (Windows only).
#[cfg(windows)]
pub fn conpty_supported() -> bool {
@@ -134,7 +151,7 @@ pub async fn spawn_process(
env: &HashMap<String, String>,
arg0: &Option<String>,
size: TerminalSize,
inherited_fds: &[i32],
inherited_fds: ChildFds<'_>,
) -> Result<SpawnedProcess> {
if program.is_empty() {
anyhow::bail!("missing program for PTY spawn");
@@ -145,7 +162,7 @@ pub async fn spawn_process(
#[cfg(target_os = "linux")]
if crate::spawn_helper::is_available()
&& inherited_fds.is_empty()
&& inherited_fds.as_slice().is_empty()
&& Path::new(program).is_absolute()
&& arg0.is_none()
&& cwd.is_dir()
@@ -156,7 +173,7 @@ pub async fn spawn_process(
.await;
}
#[cfg(unix)]
if !inherited_fds.is_empty() {
if !inherited_fds.as_slice().is_empty() {
return spawn_process_preserving_fds(program, args, cwd, env, arg0, size, inherited_fds)
.await;
}
@@ -309,24 +326,31 @@ async fn spawn_process_preserving_fds(
env: &HashMap<String, String>,
arg0: &Option<String>,
size: TerminalSize,
inherited_fds: &[RawFd],
descriptors: ChildFds<'_>,
) -> Result<SpawnedProcess> {
let (master, slave) = open_unix_pty(size)?;
let io = crate::unix_io::PtyIo::new(master.as_raw_fd())?;
let uses_portable_status = inherited_fds.is_empty();
let stdin_close_behavior = if inherited_fds.is_empty() {
let uses_portable_status = match descriptors {
ChildFds::Inherited(fds) => fds.is_empty(),
ChildFds::Attached(_) => true,
};
let inherited_fds = descriptors.as_slice();
let stdin_close_behavior = if uses_portable_status {
crate::unix_io::StdinCloseBehavior::SendEof
} else {
crate::unix_io::StdinCloseBehavior::NoEof
};
let default_shell = (uses_portable_status && !env.contains_key("SHELL")).then(|| {
let mut builder = CommandBuilder::new(program);
builder.env_clear();
builder.get_shell()
});
#[cfg(target_os = "linux")]
let mut child = {
let mut command = crate::Command::new(program);
if inherited_fds.is_empty() && !env.contains_key("SHELL") {
if let Some(shell) = &default_shell {
// portable-pty supplies the login shell when SHELL is absent.
let mut builder = CommandBuilder::new(program);
builder.env_clear();
command.env("SHELL", builder.get_shell());
command.env("SHELL", shell);
}
command
.args(args)
@@ -334,7 +358,7 @@ async fn spawn_process_preserving_fds(
.envs(env)
.drop_policy(crate::child_command::ChildDropPolicy::ReapOnly)
.descriptor_policy(crate::DescriptorPolicy::Explicit)
.preserve_fds(inherited_fds)
.inherit_fds(inherited_fds)
.stdin(crate::ChildStdin::File(slave.try_clone()?.into()));
if let Some(arg0) = arg0 {
command.arg0(arg0);
@@ -368,6 +392,9 @@ async fn spawn_process_preserving_fds(
command.arg0(arg0);
}
command.current_dir(cwd).env_clear().args(args).envs(env);
if let Some(shell) = &default_shell {
command.env("SHELL", shell);
}
let inherited_fds = inherited_fds.to_vec();
// The child sees the same terminal on stdin, stdout, and stderr.
unsafe {
@@ -378,6 +405,7 @@ async fn spawn_process_preserving_fds(
.pre_exec(move || {
configure_child_terminal()?;
close_inherited_fds_except(&inherited_fds);
make_fds_inheritable(&inherited_fds)?;
Ok(())
});
}
@@ -493,6 +521,20 @@ fn set_cloexec(fd: RawFd) -> std::io::Result<()> {
Ok(())
}
/// Called only in the child: never make attachments inheritable in the parent.
#[cfg(unix)]
pub(crate) fn make_fds_inheritable(fds: &[RawFd]) -> std::io::Result<()> {
for &fd in fds {
// SAFETY: fcntl only changes the current process's descriptor flags.
let flags = unsafe { libc::fcntl(fd, libc::F_GETFD) };
if flags == -1 || unsafe { libc::fcntl(fd, libc::F_SETFD, flags & !libc::FD_CLOEXEC) } == -1
{
return Err(std::io::Error::last_os_error());
}
}
Ok(())
}
// macOS needs a fork-safe sweep because recvmsg cannot set close-on-exec.
// Cleanup is best-effort; enumeration and close failures do not prevent launch.
#[cfg(target_os = "macos")]
+1 -1
View File
@@ -56,7 +56,7 @@ async fn portable_fallback_releases_native_pty_descriptors() -> anyhow::Result<(
&HashMap::from([("SHELL".to_owned(), "/bin/sh".to_owned())]),
&None,
crate::TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?;
drop(occupied);
+2 -2
View File
@@ -581,7 +581,7 @@ async fn pty_helper_establishes_a_controlling_terminal_without_forking() -> anyh
&std::env::vars().collect(),
/*arg0*/ &None,
crate::TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?;
let mut output = Vec::new();
@@ -606,7 +606,7 @@ async fn pty_helper_preserves_portable_signal_exit_status() -> anyhow::Result<()
&env,
/*arg0*/ &None,
crate::TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?;
assert_eq!(spawned.exit_rx.await?, 1);
+86 -34
View File
@@ -363,7 +363,7 @@ async fn pty_python_repl_emits_output_and_exits() -> anyhow::Result<()> {
&env_map,
&None,
TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?;
let (session, mut output_rx, exit_rx) = combine_spawned_output(spawned);
@@ -508,7 +508,7 @@ async fn pipe_and_pty_share_interface() -> anyhow::Result<()> {
&env_map,
&None,
TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?;
let (_pipe_session, pipe_output_rx, pipe_exit_rx) = combine_spawned_output(pipe);
@@ -920,7 +920,7 @@ time.sleep(10)
&env_map,
&None,
TerminalSize::default(),
inherited_fds,
crate::ChildFds::Inherited(inherited_fds),
)
.await?;
let (session, mut output_rx, exit_rx) = combine_spawned_output(spawned);
@@ -988,7 +988,7 @@ fn pty_spawn_without_io_driver_returns_error() -> anyhow::Result<()> {
&env_map,
&None,
TerminalSize::default(),
inherited_fds,
crate::ChildFds::Inherited(inherited_fds),
)) else {
anyhow::bail!("PTY spawn succeeded without a Tokio I/O driver");
};
@@ -1061,7 +1061,7 @@ fn pty_terminate_allows_runtime_shutdown_with_full_output_channel() -> anyhow::R
&env_map,
&None,
TerminalSize::default(),
inherited_fds,
crate::ChildFds::Inherited(inherited_fds),
)
.await?;
tokio::time::timeout(std::time::Duration::from_secs(2), async {
@@ -1125,7 +1125,7 @@ async fn pty_dropped_output_receiver_keeps_draining_child() -> anyhow::Result<()
&env_map,
&None,
TerminalSize::default(),
inherited_fds,
crate::ChildFds::Inherited(inherited_fds),
)
.await?;
drop(stdout_rx);
@@ -1155,30 +1155,35 @@ assert data == b'0123456789abcdefghijklmnopqrstuvwxyz\n' * 32768 + b'\n', len(da
print('__complete__', flush=True)
";
let env_map: HashMap<String, String> = std::env::vars().collect();
let spawned = spawn_pty_process(
&python,
&["-c".to_string(), script.to_string()],
Path::new("."),
&env_map,
&None,
TerminalSize::default(),
&[],
)
.await?;
let (session, mut output_rx, exit_rx) = combine_spawned_output(spawned);
wait_for_output_contains(&mut output_rx, "__ready__", /*timeout_ms*/ 2_000).await?;
let writer = session.writer_sender();
writer
.send(b"0123456789abcdefghijklmnopqrstuvwxyz\n".repeat(32_768))
use std::os::fd::AsRawFd;
let reader = std::fs::File::open("/dev/null")?;
for fds in [&[][..], &[reader.as_raw_fd()][..]] {
let spawned = spawn_pty_process(
&python,
&["-c".to_string(), script.to_string()],
Path::new("."),
&env_map,
&None,
TerminalSize::default(),
crate::ChildFds::Attached(fds),
)
.await?;
drop(writer);
session.close_stdin();
let (session, mut output_rx, exit_rx) = combine_spawned_output(spawned);
wait_for_output_contains(&mut output_rx, "__ready__", /*timeout_ms*/ 2_000).await?;
let writer = session.writer_sender();
writer
.send(b"0123456789abcdefghijklmnopqrstuvwxyz\n".repeat(32_768))
.await?;
drop(writer);
session.close_stdin();
let (output, code) = collect_output_until_exit(output_rx, exit_rx, /*timeout_ms*/ 5_000).await;
assert_eq!(
(code, String::from_utf8_lossy(&output).trim()),
(0, "__complete__")
);
let (output, code) =
collect_output_until_exit(output_rx, exit_rx, /*timeout_ms*/ 5_000).await;
assert_eq!(
(code, String::from_utf8_lossy(&output).trim()),
(0, "__complete__")
);
}
Ok(())
}
@@ -1220,7 +1225,7 @@ fn pty_terminate_reaps_child_when_waiter_is_queued() -> anyhow::Result<()> {
&env_map,
&None,
TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?;
let (session, _output_rx, exit_rx) = combine_spawned_output(spawned);
@@ -1270,6 +1275,53 @@ fn pty_terminate_reaps_child_when_waiter_is_queued() -> anyhow::Result<()> {
})
}
#[cfg(unix)]
#[tokio::test]
async fn pty_attachments_preserve_shell_environment_and_signal_status() -> anyhow::Result<()> {
use std::os::fd::AsRawFd;
let reader = std::fs::File::open("/dev/null")?;
let fds = [reader.as_raw_fd()];
let flags = unsafe { libc::fcntl(reader.as_raw_fd(), libc::F_GETFD) };
assert_eq!(flags & libc::FD_CLOEXEC, libc::FD_CLOEXEC);
let mut output = Vec::new();
for (fds, expected_status) in [
(crate::ChildFds::Inherited(&[]), 1),
(crate::ChildFds::Attached(&fds), 1),
(crate::ChildFds::Inherited(&fds), 143),
] {
let descriptor_check = fds
.as_slice()
.first()
.map(|fd| format!("/bin/cat /dev/fd/{fd} >/dev/null || exit 42; "))
.unwrap_or_default();
let spawned = spawn_pty_process(
"/bin/sh",
&[
"-c".to_string(),
format!("{descriptor_check}/usr/bin/printenv SHELL; kill -TERM $$"),
],
Path::new("/"),
&HashMap::new(),
&None,
TerminalSize::default(),
fds,
)
.await?;
assert_eq!(
unsafe { libc::fcntl(reader.as_raw_fd(), libc::F_GETFD) },
flags
);
let (_session, output_rx, exit_rx) = combine_spawned_output(spawned);
let (bytes, status) =
collect_output_until_exit(output_rx, exit_rx, /*timeout_ms*/ 2_000).await;
assert_eq!(status, expected_status);
output.push(bytes);
}
assert!(!output[0].is_empty());
assert_eq!(output[0], output[1]);
Ok(())
}
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn pty_terminate_kills_background_children_in_same_process_group() -> anyhow::Result<()> {
@@ -1284,7 +1336,7 @@ async fn pty_terminate_kills_background_children_in_same_process_group() -> anyh
&env_map,
&None,
TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?;
let (session, mut output_rx, exit_rx) = combine_spawned_output(spawned);
@@ -1352,7 +1404,7 @@ async fn pty_spawn_can_preserve_inherited_fds() -> anyhow::Result<()> {
&env_map,
&None,
TerminalSize::default(),
&[write_end.as_raw_fd()],
crate::ChildFds::Inherited(&[write_end.as_raw_fd()]),
)
.await?;
@@ -1404,7 +1456,7 @@ async fn pty_preserving_inherited_fds_keeps_python_repl_running() -> anyhow::Res
&env_map,
&None,
TerminalSize::default(),
&[preserved_fd.as_raw_fd()],
crate::ChildFds::Inherited(&[preserved_fd.as_raw_fd()]),
)
.await?;
drop(read_end);
@@ -1470,7 +1522,7 @@ async fn pty_spawn_with_inherited_fds_reports_exec_failures() -> anyhow::Result<
&env_map,
&None,
TerminalSize::default(),
&[write_end.as_raw_fd()],
crate::ChildFds::Inherited(&[write_end.as_raw_fd()]),
)
.await;
@@ -1522,7 +1574,7 @@ async fn pty_spawn_with_inherited_fds_supports_resize() -> anyhow::Result<()> {
rows: 31,
cols: 101,
},
&[write_end.as_raw_fd()],
crate::ChildFds::Inherited(&[write_end.as_raw_fd()]),
)
.await?;
@@ -24,7 +24,7 @@ async fn output_closes_after_client_exit_while_session_is_retained() -> anyhow::
&env,
/*arg0*/ &None,
crate::TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?;
let mut output = spawned.stdout_rx;
@@ -65,7 +65,7 @@ async fn output_remains_open_for_surviving_console_child() -> anyhow::Result<()>
&env,
/*arg0*/ &None,
crate::TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?;
let mut output = spawned.stdout_rx;
@@ -116,7 +116,7 @@ async fn output_closes_after_termination_while_session_is_retained() -> anyhow::
&env,
/*arg0*/ &None,
crate::TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?;
let mut output = spawned.stdout_rx;
+4 -4
View File
@@ -98,7 +98,7 @@ async fn assert_terminate_kills_descendant(
env,
/*arg0*/ &None,
TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?
};
@@ -154,7 +154,7 @@ async fn assert_normal_exit_preserves_descendant(
env,
/*arg0*/ &None,
TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?
};
@@ -335,7 +335,7 @@ async fn conpty_delivers_input_to_foreground_children() -> anyhow::Result<()> {
&env,
/*arg0*/ &None,
TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?;
let (session, mut output_rx, exit_rx) = combine_spawned_output(spawned);
@@ -388,7 +388,7 @@ async fn conpty_ctrl_c_interrupts_powershell_foreground_child() -> anyhow::Resul
&env,
/*arg0*/ &None,
TerminalSize::default(),
&[],
crate::ChildFds::Inherited(&[]),
)
.await?;
let (session, mut output_rx, exit_rx) = combine_spawned_output(spawned);