Child stderr fidelity: default SIGPIPE on exec, opt-out for the txn stderr tee - #173
Open
congwang-mk wants to merge 2 commits into
Open
Child stderr fidelity: default SIGPIPE on exec, opt-out for the txn stderr tee#173congwang-mk wants to merge 2 commits into
congwang-mk wants to merge 2 commits into
Conversation
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 <cwang@multikernel.io>
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 <cwang@multikernel.io>
Contributor
Author
|
@dzerik This is probably introduced by your recent pipline transaction changes. Please take a look. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two child/stage stderr behavior fixes, split out of #172.
1. Sandboxed programs inherited an ignored SIGPIPE
The Rust runtime ignores SIGPIPE process-wide and an ignored disposition survives execve, so every program exec'd by sandlock's own fork/exec path started 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. Beyond the noise, exit statuses changed shape (EPIPE error exit instead of death by signal 13), sandlock's pipeline stages saw EPIPE instead of clean SIGPIPE death when a downstream stage exited early, and child behavior silently depended on which language the embedding process was written in (CPython also ignores SIGPIPE; a C embedder does not).
Fix: reset SIGPIPE to SIG_DFL in the child right before execvp, matching what std::process::Command, posix_spawn, and shells do. The in-process entry arm is untouched: it runs Rust code that expects the Rust runtime's disposition.
2. Transaction stages tee stderr to the parent's fd 2 with no way to opt out
The live tee is right for the interactive CLI but wrong for embedders whose fd 2 is not the place for workload output, and for test harnesses (raw fd writes bypass libtest's capture, so the stderr-cap test sprayed its 128 KiB flood payload onto the terminal).
Fix: add
Transaction::tee_stderr(bool), default true so the CLI keeps its behavior, 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.Test plan
🤖 Generated with Claude Code