fix: restore default SIGPIPE disposition so piped output does not abort - #1128
Open
aidenhiggs wants to merge 1 commit into
Open
fix: restore default SIGPIPE disposition so piped output does not abort#1128aidenhiggs wants to merge 1 commit into
aidenhiggs wants to merge 1 commit into
Conversation
Rust's standard library sets SIGPIPE to SIG_IGN before main runs, so writing to a closed pipe returns EPIPE rather than terminating the process. The print!/println! macros panic on write errors, and because the release profile sets panic = "abort", that panic becomes a SIGABRT with a core dump. The user-visible effect is that ordinary shell usage such as `railway logs | head -n 5` crashes the CLI once the reader exits. On systems running systemd-coredump this writes a ~220 KB core dump per occurrence, and because a core is a verbatim copy of process memory those files can contain the user's Railway API token. Restore SIG_DFL for SIGPIPE at the top of main so the process terminates silently on a broken pipe, which is the conventional behaviour for a Unix command line tool. This uses nix, already a dependency with the signal feature enabled, and is cfg(unix)-gated so it is a no-op on Windows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Author
|
Flagging two things this PR needs from a maintainer, since neither is available
In the meantime I ran the CI steps locally against
Happy to adjust the approach if you'd rather handle |
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.
Problem
The CLI dies with
SIGABRTand leaves a core dump behind whenever its stdout pipeis closed before it finishes writing — ordinary shell usage like
railway logs | head -n 5or piping into a pager the user then quits.
On Linux distributions that collect core dumps (anything running
systemd-coredump,which is the default on Arch, Fedora, and others) each occurrence writes a ~220 KB
core to
/var/lib/systemd/coredump/. Because a core is a verbatim copy of processmemory, those files can contain the user's Railway API token. On the machine where
I hit this, six cores had accumulated before I noticed.
Reproduction
Close the read end of the pipe before the CLI writes:
Note that
railway --help | head -1does not reproduce it — the help text fitsin the 64 KB pipe buffer, so the write never fails. The reader has to go away
before the write lands.
Cause
Two things combine:
SIGPIPEtoSIG_IGNbeforemainruns, so awrite to a closed pipe returns
EPIPEinstead of terminating the process.Nothing in this crate restores the default disposition —
grep -rn "SIGPIPE" src/returns no hits.print!/println!panic on write errors, andCargo.tomlsetspanic = "abort"for the release profile, so that panic becomes aSIGABRTwith a core dump rather than a quiet exit.
Fix
Restore
SIG_DFLforSIGPIPEat the top ofmain, so the process terminatessilently on a broken pipe — the conventional behaviour for a Unix CLI, and what
users' shells already expect from
| head.This uses
nix, which is already a dependency with thesignalfeature enabled,so it adds no new crates. The change is
#[cfg(unix)]-gated and is a no-op onWindows, which has no
SIGPIPE.Verification
cargo checkpasses; no new warningscargo fmt --all -- --checkpassesBuilt
--releaseand ran the reproduction above against both binaries underidentical conditions:
SIGABRT(6), core dumpedSIGPIPE(13), no corerailway --help | head -3now exits 141 (128+13), matching the conventionalbehaviour of
yes | head. Normal non-piped invocations are unaffected:railway --versionandrailway statusbehave exactly as before.Tested on Linux x86_64 with rustc 1.98.0, against
masterat 5.44.0.Note for maintainers
CI's label check requires a release label —
release/patchseems right for this.