Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,10 @@ which = { version = "8.0", features = ["regex"] }
assert_cmd = "2.2.0"
assert_fs = "1.1.3"
escargot = "0.5.15"
nix = { version = "0.29", features = ["signal", "process"] }
predicates = "3.1.4"
tempfile = "3.27"
wait-timeout = "0.2"

[build-dependencies]
anyhow = "1.0.102"
Expand Down
29 changes: 29 additions & 0 deletions src/bin/scope-intercept.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ use human_panic::setup_panic;
use std::env;
use std::io::Cursor;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use tokio::io::BufReader;
use tokio::signal::unix::{SignalKind, signal};
use tracing::{Level, enabled, error, info, warn};

/// A wrapper CLI that can be used to capture output from a program, check if there are known errors
Expand Down Expand Up @@ -76,6 +78,26 @@ async fn run_command(opts: Cli) -> anyhow::Result<i32> {
let current_dir = std::env::current_dir()?;
let path = env::var("PATH").unwrap_or_default();

// SIGINT/SIGTERM/SIGHUP are delivered to every member of our foreground
// process group, including the child, so its own cleanup traps run. We
// just need to keep ourselves alive long enough for the child to finish
// shutting down — otherwise our captured stdout/stderr pipes close and
// the child dies with SIGPIPE before its trap can complete.
let interrupted = Arc::new(AtomicBool::new(false));
for kind in [
SignalKind::interrupt(),
SignalKind::terminate(),
SignalKind::hangup(),
] {
let mut stream = signal(kind)?;
let interrupted = interrupted.clone();
tokio::spawn(async move {
while stream.recv().await.is_some() {
interrupted.store(true, Ordering::SeqCst);
}
});
}

let capture = OutputCapture::capture_output(CaptureOpts {
working_dir: &current_dir,
args: &command,
Expand All @@ -93,6 +115,13 @@ async fn run_command(opts: Cli) -> anyhow::Result<i32> {
return Ok(exit_code);
}

// The user explicitly asked to quit — don't surprise them with
// known-error prompts, retry, or bug-report offers. Just propagate the
// child's exit code (typically 130 for SIGINT, 143 for SIGTERM).
if interrupted.load(Ordering::SeqCst) {
return Ok(exit_code);
}

error!(target: "user", "Command failed, checking for a known error");
let found_config = opts.config_options.load_config().await.unwrap_or_else(|e| {
error!(target: "user", "Unable to load configs from disk: {:?}", e);
Expand Down
100 changes: 100 additions & 0 deletions tests/scope_intercept.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
use assert_fs::fixture::{FileWriteStr, PathChild};
use nix::sys::signal::{Signal, killpg};
use nix::unistd::{Pid, setsid};
use predicates::boolean::PredicateBooleanExt;
use predicates::prelude::predicate;
use std::os::unix::process::CommandExt;
use std::process::Command as StdCommand;
use std::time::{Duration, Instant};
use wait_timeout::ChildExt;

#[allow(dead_code)]
mod common;
Expand Down Expand Up @@ -158,3 +164,97 @@ fn test_intercept_no_known_errors_match() {

helper.clean_work_dir();
}

// When scope-intercept wraps a long-running interactive process (e.g.
// used as a shebang on a server start script), Ctrl+C must let the
// child run its own SIGINT trap and exit cleanly. Before this fix,
// scope-intercept terminated immediately on SIGINT, closing the
// captured stdout/stderr pipes and killing the child with SIGPIPE
// before its trap could complete.
#[test]
fn test_intercept_forwards_sigint_and_waits_for_child_cleanup() {
let helper = ScopeTestHelper::new(
"test_intercept_forwards_sigint_and_waits_for_child_cleanup",
"empty",
);

let work_dir = helper.work_dir.path().to_path_buf();
let cleanup_marker = work_dir.join("cleanup.done");
let ready_marker = work_dir.join("ready");

helper
.work_dir
.child("trap.sh")
.write_str(
"#!/bin/bash\n\
trap 'echo trapped > cleanup.done; exit 130' INT\n\
touch ready\n\
# sleep in short increments so the trap fires promptly\n\
for _ in $(seq 1 30); do sleep 1; done\n",
)
.unwrap();

let intercept_bin = assert_cmd::cargo::cargo_bin("scope-intercept");
let mut command = StdCommand::new(intercept_bin);
command
.current_dir(&work_dir)
.env("NO_COLOR", "1")
.args(["--", "bash", "trap.sh"]);
// Put scope-intercept in its own session/process group so killpg below
// hits both it and the wrapped bash, mirroring how a terminal delivers
// Ctrl+C to the foreground process group.
//
// SAFETY: pre_exec runs the closure between fork and exec, so it must
// call only async-signal-safe functions. setsid(2) is on the POSIX
// async-signal-safe list and touches no shared state in the parent.
unsafe {
Comment thread
rubberduck203 marked this conversation as resolved.
command.pre_exec(|| setsid().map(|_| ()).map_err(std::io::Error::from));
}
let mut child = command.spawn().expect("failed to spawn scope-intercept");

// Poll for the ready marker. There's no good cross-process "file
// appeared" primitive without bringing in inotify/kqueue, so we
// sleep between checks to avoid burning a core.
let deadline = Instant::now() + Duration::from_secs(5);
while !ready_marker.exists() {
if Instant::now() > deadline {
child.kill().ok();
panic!(
"trap.sh never wrote ready marker; scope-intercept may not be running the script"
);
}
std::thread::sleep(Duration::from_millis(50));
}

let pgid = Pid::from_raw(child.id() as i32);
killpg(pgid, Signal::SIGINT).expect("killpg(pgid, SIGINT) failed");

// If scope-intercept doesn't shut down within 10s, the bug is back:
// it's hanging instead of letting the child run its trap and
// propagating the exit.
let status = match child
.wait_timeout(Duration::from_secs(10))
.expect("wait_timeout failed")
{
Some(status) => status,
None => {
child.kill().ok();
panic!("scope-intercept did not exit within 10s after SIGINT");
}
};

assert!(
cleanup_marker.exists(),
"child SIGINT trap did not run — cleanup.done was never created"
);
let body = std::fs::read_to_string(&cleanup_marker).unwrap();
assert_eq!(body.trim(), "trapped");

assert_eq!(
status.code(),
Some(130),
"scope-intercept should propagate the child's exit code (130 for SIGINT), got {status:?}"
);

helper.clean_work_dir();
}
Loading