From fbcbd085adf01e186d7819ba9810059024fa73d9 Mon Sep 17 00:00:00 2001 From: Zhang <56248212+YonganZhang@users.noreply.github.com> Date: Tue, 23 Jun 2026 03:18:54 +0800 Subject: [PATCH] fix(cli): restore SIGPIPE default on Unix (#3657) Co-authored-by: shanu Co-authored-by: Claude Opus 4.8 (1M context) --- Cargo.lock | 1 + Cargo.toml | 1 + src/main.rs | 15 +++++++++++++ tests/cli_sigpipe.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+) create mode 100644 tests/cli_sigpipe.rs diff --git a/Cargo.lock b/Cargo.lock index 3047650ba..c8a8e09d8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5180,6 +5180,7 @@ dependencies = [ "keyring", "landlock", "lettre", + "libc", "log", "mail-parser", "matrix-sdk", diff --git a/Cargo.toml b/Cargo.toml index dd89a8312..bc5195315 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -55,6 +55,7 @@ tokio = { version = "1", features = ["full", "sync"] } once_cell = "1.19" parking_lot = "0.12" log = "0.4" +libc = "0.2" nu-ansi-term = "0.46" env_logger = "0.11" base64 = "0.22" diff --git a/src/main.rs b/src/main.rs index 8efb09995..81d963c3a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -14,6 +14,8 @@ use regex::Regex; /// information is redacted before being sent to the server. After setup, it /// delegates execution to the core library based on CLI arguments. fn main() { + restore_default_sigpipe(); + // Load `.env` before `sentry::init` so a DSN defined only in the dotenv // file is visible to the Sentry client at startup. `dotenvy::dotenv()` is // a no-op for variables already present in the process environment, and @@ -188,6 +190,19 @@ fn main() { } } +#[cfg(unix)] +fn restore_default_sigpipe() { + // Rust ignores SIGPIPE at startup. That makes writes to a closed pipe + // return EPIPE, which the print macros turn into a panic. CLI tools should + // instead terminate quietly when a downstream reader such as `head` exits. + unsafe { + libc::signal(libc::SIGPIPE, libc::SIG_DFL); + } +} + +#[cfg(not(unix))] +fn restore_default_sigpipe() {} + // --------------------------------------------------------------------------- // Release / environment resolution for Sentry // --------------------------------------------------------------------------- diff --git a/tests/cli_sigpipe.rs b/tests/cli_sigpipe.rs new file mode 100644 index 000000000..cc2d61021 --- /dev/null +++ b/tests/cli_sigpipe.rs @@ -0,0 +1,50 @@ +#[cfg(unix)] +#[test] +fn help_output_closed_pipe_does_not_panic() { + use std::io::{BufRead, BufReader}; + use std::os::unix::process::ExitStatusExt; + use std::process::{Command, Stdio}; + + let mut child = Command::new(env!("CARGO_BIN_EXE_openhuman-core")) + .arg("--help") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn openhuman-core --help"); + + // Mirror the issue repro (`openhuman-core --help | head -n 1`): read a + // single line, then drop the read end mid-stream so the child's next write + // lands on a closed pipe. Closing before reading (as a naive test does) + // lets the child buffer its entire few-KB `--help` output in one successful + // write well under the 64 KB pipe buffer and exit cleanly, never exercising + // the broken-pipe path — a false green that passes even without the fix. + let stdout = child.stdout.take().expect("capture stdout"); + let mut reader = BufReader::new(stdout); + let mut first_line = String::new(); + reader + .read_line(&mut first_line) + .expect("read first help line"); + drop(reader); + + let output = child.wait_with_output().expect("wait for openhuman-core"); + let stderr = String::from_utf8_lossy(&output.stderr); + + assert!( + !stderr.contains("Broken pipe"), + "stderr must not include a broken-pipe panic: {stderr}" + ); + assert!( + !stderr.contains("panicked"), + "stderr must not include a panic report: {stderr}" + ); + + // Acceptance criterion #1: a closed downstream reader must yield a clean + // exit — either the child finished before the pipe closed, or the restored + // default disposition let SIGPIPE terminate it. A normal-code crash (e.g. + // the panic exit code 101) is precisely the regression this guards against. + assert!( + output.status.success() || output.status.signal() == Some(libc::SIGPIPE), + "process must exit cleanly or via SIGPIPE, got {:?}", + output.status + ); +}