From 01ae05bab29cae1071e074872f820827fe634bba Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Tue, 28 Jul 2026 15:54:36 -0700 Subject: [PATCH 1/2] fix(core): restore default SIGPIPE in the child before execve The Rust runtime ignores SIGPIPE process-wide and an ignored disposition survives execve, so every sandboxed program started life with SIGPIPE ignored: writes to a closed pipe returned EPIPE and utilities printed 'Broken pipe' errors instead of dying silently the way they do under a shell. The popen fd-leak test made this visible by spamming 'echo: Broken pipe' forty times per run, but the fidelity gap applied to every exec'd workload. Reset SIGPIPE to SIG_DFL right before execvp, matching what std::process::Command and POSIX shells do. The in-process entry arm is untouched: it runs Rust code that expects the Rust runtime's disposition. Signed-off-by: Cong Wang --- crates/sandlock-core/src/context.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/sandlock-core/src/context.rs b/crates/sandlock-core/src/context.rs index 2b980f1e..d347d46c 100644 --- a/crates/sandlock-core/src/context.rs +++ b/crates/sandlock-core/src/context.rs @@ -563,6 +563,12 @@ pub(crate) fn confine_child(args: ChildSpawnArgs<'_>) -> ! { }; // 14. exec + // + // Restore SIGPIPE first: the Rust runtime ignores it process-wide, and an + // ignored disposition survives execve, so without this every sandboxed + // program sees write() fail with EPIPE instead of dying silently the way + // it would under a shell (std::process::Command does the same reset). + unsafe { libc::signal(libc::SIGPIPE, libc::SIG_DFL) }; debug_assert!(!cmd.is_empty(), "cmd must not be empty"); let argv_ptrs: Vec<*const libc::c_char> = cmd .iter() From cdcb37667b1d8f79d02a08b44ae5393821250830 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Tue, 28 Jul 2026 16:14:38 -0700 Subject: [PATCH 2/2] feat(txn): let callers opt out of the live stderr tee A transaction tees each stage's stderr to the parent's fd 2 as a live courtesy alongside the bounded capture. That is right for the interactive CLI but wrong for embedders whose fd 2 is not the place for workload output, and for test harnesses, whose capture raw fd writes bypass: the stderr-cap test flooded the terminal with its 128 KiB flood payload. Add Transaction::tee_stderr(bool), default true, threaded to the tee open in the stage driver; stages become capture-only when disabled. The stderr-emitting transaction tests opt out, keeping the suite's terminal output clean. Signed-off-by: Cong Wang --- crates/sandlock-core/src/transaction.rs | 29 +++++++++++++++---- .../tests/integration/test_transaction.rs | 10 +++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/crates/sandlock-core/src/transaction.rs b/crates/sandlock-core/src/transaction.rs index 8fbaaceb..b8c0009d 100644 --- a/crates/sandlock-core/src/transaction.rs +++ b/crates/sandlock-core/src/transaction.rs @@ -61,6 +61,7 @@ use crate::result::{ExitStatus, RunResult}; pub struct Transaction { stages: Vec, commit_lock_wait: Duration, + tee_stderr: bool, } impl Transaction { @@ -78,7 +79,21 @@ impl Transaction { /// `|`-built chain means "connect these by pipes", which a transaction does /// not do. pub fn new(stages: impl IntoIterator) -> Self { - Self { stages: stages.into_iter().collect(), commit_lock_wait: COMMIT_LOCK_WAIT } + Self { + stages: stages.into_iter().collect(), + commit_lock_wait: COMMIT_LOCK_WAIT, + tee_stderr: true, + } + } + + /// Whether each stage's stderr, besides being captured in the outcome, is + /// also streamed live to this process's fd 2. Defaults to true — the right + /// behavior for an interactive CLI. Turn it off when the embedding + /// process's stderr is not the place for workload output (a daemon or GUI + /// embedder, a test harness whose capture raw fd writes bypass). + pub fn tee_stderr(mut self, tee: bool) -> Self { + self.tee_stderr = tee; + self } /// How long the commit may wait for another transaction to release the @@ -120,7 +135,7 @@ impl Transaction { /// [`list_preserved`](crate::recovery::list_preserved). pub async fn run(self, timeout: Option) -> Result { validate_txn_stages(&self.stages)?; - run_txn(self.stages, timeout, Disposition::Commit, self.commit_lock_wait).await + run_txn(self.stages, timeout, Disposition::Commit, self.commit_lock_wait, self.tee_stderr).await } /// Run every stage and report what the transaction *would* change, then @@ -134,7 +149,7 @@ impl Transaction { /// out first. pub async fn dry_run(self, timeout: Option) -> Result { validate_txn_stages(&self.stages)?; - run_txn(self.stages, timeout, Disposition::DryRun, self.commit_lock_wait).await + run_txn(self.stages, timeout, Disposition::DryRun, self.commit_lock_wait, self.tee_stderr).await } } @@ -496,6 +511,7 @@ async fn run_txn( timeout: Option, disposition: Disposition, commit_lock_wait: Duration, + tee_stderr: bool, ) -> Result { // All stages share the validated workdir; take COW storage/quota from the // first stage (they overlay the same lower). @@ -517,7 +533,7 @@ async fn run_txn( // so a timeout that cancels the driver future does not take the completed // stages' results down with it. let results = std::sync::Arc::new(std::sync::Mutex::new(Vec::::new())); - let drive = drive_txn_stages(stages, shared, std::sync::Arc::clone(&results)); + let drive = drive_txn_stages(stages, shared, std::sync::Arc::clone(&results), tee_stderr); let driven: Result, TxnError> = match timeout { Some(dur) => match tokio::time::timeout(dur, drive).await { Ok(r) => r, @@ -665,13 +681,16 @@ async fn drive_txn_stages( stages: Vec, shared: crate::sandbox::SharedCow, results: std::sync::Arc>>, + tee_stderr: bool, ) -> Result, TxnError> { use std::os::fd::{AsRawFd, OwnedFd}; // An INDEPENDENT, non-blocking open file description onto the parent's fd 2, // cloned per stage to tee each stage's captured stderr back to fd 2 // (best-effort). See `open_stderr_tee` for why it must not be a dup of fd 2. - let tee_fd: Option = open_stderr_tee(); + // `None` when the caller opted out via Transaction::tee_stderr(false): + // stages are then capture-only. + let tee_fd: Option = if tee_stderr { open_stderr_tee() } else { None }; // Stage names share one unique run id so concurrent transactions in the // same UID never collide on their runtime dirs. diff --git a/crates/sandlock-core/tests/integration/test_transaction.rs b/crates/sandlock-core/tests/integration/test_transaction.rs index 5dade20a..9b4b13ec 100644 --- a/crates/sandlock-core/tests/integration/test_transaction.rs +++ b/crates/sandlock-core/tests/integration/test_transaction.rs @@ -563,6 +563,8 @@ async fn test_txn_stage_failure_is_not_masked_by_a_backgrounded_descendant() { &["sh", "-c", "(while :; do :; done) & echo boom 1>&2; exit 1"], ), ]) + // Capture-only, so 'boom' does not leak to the test terminal. + .tee_stderr(false) .run(Some(Duration::from_secs(8))), ) .await @@ -1170,6 +1172,8 @@ async fn test_txn_stages_capture_stderr() { Stage::new(&policy, &["sh", "-c", "echo plan > a.txt"]), Stage::new(&policy, &["sh", "-c", "echo boom 1>&2; exit 1"]), ]) + // Capture-only, so 'boom' does not leak to the test terminal. + .tee_stderr(false) .run(None) .await .expect("transaction should run"); @@ -1231,6 +1235,9 @@ async fn test_txn_stage_stderr_is_capped_and_tail_biased() { &["sh", "-c", "yes X | head -c 131072 1>&2; printf ZZZEND 1>&2; exit 1"], ), ]) + // Capture-only: the live tee would flood the test terminal with the X's + // (raw fd 2 writes bypass libtest's capture). + .tee_stderr(false) .run(None) .await .expect("transaction should run"); @@ -1293,6 +1300,9 @@ async fn test_txn_stages_inherit_stdout_but_capture_stderr() { Stage::new(&policy, &["sh", "-c", &s0]), Stage::new(&policy, &["sh", "-c", &s1]), ]) + // Capture-only: the tee is parent-side, so disabling it does not change + // the stage fd wiring this test asserts. + .tee_stderr(false) .run(None) .await .expect("transaction should run");