fix(cli): restore the default SIGPIPE disposition - #377
Merged
Conversation
Rust's runtime ignores SIGPIPE, so a closed output pipe surfaces as an
EPIPE write error rather than terminating the process. `secretspec` then
fails where every other Unix tool exits quietly:
$ secretspec export --format dotenv | head
Error: x Failed to export secrets
|-> IO error: Broken pipe (os error 32)
`-> Broken pipe (os error 32) # exit 1
$ secretspec check --json | head
thread 'main' panicked at library/std/src/io/stdio.rs:1166:9:
failed printing to stdout: Broken pipe (os error 32) # exit 101
Resetting the disposition to SIG_DFL in the binary entry point fixes
every stdout-writing command at once, rather than teaching each call
site to special-case EPIPE. Both commands above now terminate on signal
13 with empty stderr; unpiped output is unchanged.
This is the follow-up promised in cachix#373, where the same broken-pipe
behavior came up on the `check` path.
libc is a new direct dependency, declared only under cfg(unix). It was
already in the lock file transitively, so nothing new is vendored.
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.
Follow-up to point 3 of your review on #373.
Rust's runtime ignores
SIGPIPE, so a closed output pipe comes back as anEPIPEwrite error instead of terminating the process.secretspecthenfails where every other Unix tool exits quietly:
check --jsonis the worse of the two: it panics out ofprintln!ratherthan returning an error.
The fix
Reset the disposition to
SIG_DFLat the top of the binary entry point. Doingit there fixes every stdout-writing command at once, instead of teaching each
call site to special-case
EPIPE— which is why this is one small commitrather than three.
After the change, both commands terminate on signal 13 with empty stderr,
and unpiped output is byte-for-byte unchanged.
Tests
secretspec/tests/sigpipe.rscovers both paths: it spawns the real binary,reads a few bytes, drops the read end, and asserts termination by
SIGPIPEwith no stderr. The secret count is sized so output exceeds the ~64KiB pipe
buffer — below that the write fits in the buffer and the pipe never breaks.
I checked both tests fail without the fix, with exactly the errors quoted
above, so they're genuinely pinning the behavior rather than passing
vacuously.
Note on
libcNew direct dependency, declared only under
[target.'cfg(unix)'.dependencies].It was already in
Cargo.locktransitively, so nothing new is vendored. Happyto drop it for a hand-rolled
extern "C"declaration if you'd rather not addthe direct dep.