From b1a3e052f201ef18f47d11ecb712039f38c5f310 Mon Sep 17 00:00:00 2001 From: contra Date: Sat, 5 Sep 2026 13:09:38 +0300 Subject: [PATCH 01/17] fix: handle SIGINT/SIGTERM/SIGHUP in shellroute run Forward signal to child process group, escalate to SIGKILL after 5 seconds if child doesn't exit, then always call sess.Stop() for clean API session teardown. Ensures systemd stop, Ctrl+C, and terminal close all end the session cleanly instead of relying on the orphan reaper. Signed-off-by: contra --- internal/cli/run.go | 30 ++++++++++- internal/cli/run_signal_test.go | 88 +++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 internal/cli/run_signal_test.go diff --git a/internal/cli/run.go b/internal/cli/run.go index 382f0eb..2e7e701 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "os/exec" + "os/signal" "strings" "sync/atomic" "syscall" @@ -169,11 +170,38 @@ func runRun(cmd *cobra.Command, args []string) error { fmt.Fprintln(os.Stderr, "\n Connection lost during execution. Command killed.") } + // Handle SIGINT/SIGTERM/SIGHUP: forward to child, wait for it to exit, + // then clean up the session. This ensures systemd stop, Ctrl+C, and + // terminal close all end the API session cleanly. + sigCh := make(chan os.Signal, 1) + signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) + go func() { + sig, ok := <-sigCh + if !ok || !childRunning.Load() { + return + } + // Forward signal to child process group + if childCmd.Process != nil { + syscall.Kill(-childCmd.Process.Pid, sig.(syscall.Signal)) + } + // Escalate after 5 seconds if child doesn't exit + go func() { + select { + case <-time.After(5 * time.Second): + if childRunning.Load() && childCmd.Process != nil { + syscall.Kill(-childCmd.Process.Pid, syscall.SIGKILL) + } + case <-killCancel: + } + }() + }() + childErr := childCmd.Wait() childRunning.Store(false) close(killCancel) + signal.Stop(sigCh) - // Tear down session + // Tear down session — always called, even after signal resp, stopErr := sess.Stop() if !runNoStat { if childErr != nil { diff --git a/internal/cli/run_signal_test.go b/internal/cli/run_signal_test.go new file mode 100644 index 0000000..14598ab --- /dev/null +++ b/internal/cli/run_signal_test.go @@ -0,0 +1,88 @@ +//go:build !windows + +package cli + +import ( + "os" + "os/exec" + "strings" + "sync/atomic" + "testing" + "time" +) + +// Tests for signal handling in shellroute run. +// These test the signal forwarding behavior by running a subprocess +// and verifying it receives the signal. + +func TestRunSignal_ChildReceivesSIGTERM(t *testing.T) { + if os.Getenv("SHELLROUTE_TEST_SIGNAL_CHILD") == "1" { + // Child process: sleep until killed, print what signal we got + cmd := exec.Command("sh", "-c", `trap 'echo GOT_SIGTERM; exit 0' TERM; while true; do sleep 0.1; done`) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + cmd.Run() + return + } + + // Parent: verify child receives SIGTERM when parent is killed + // This is a behavioral test — we verify the signal forwarding code compiles + // and the handler is registered. Full integration would need shellroute run + // with a real session. + t.Log("Signal handler compiles and registers correctly") +} + +func TestRunSignal_EscalateToSIGKILL(t *testing.T) { + var killed atomic.Bool + killCancel := make(chan struct{}) + + go func() { + select { + case <-time.After(100 * time.Millisecond): + killed.Store(true) + case <-killCancel: + } + }() + + time.Sleep(200 * time.Millisecond) + close(killCancel) + + if !killed.Load() { + t.Error("escalation timer should have fired") + } +} + +func TestRunSignal_NoEscalateIfChildExits(t *testing.T) { + var killed atomic.Bool + killCancel := make(chan struct{}) + + go func() { + select { + case <-time.After(5 * time.Second): + killed.Store(true) + case <-killCancel: + } + }() + + close(killCancel) + time.Sleep(50 * time.Millisecond) + + if killed.Load() { + t.Error("escalation should not fire when child exits promptly") + } +} + +func TestRunSignal_HandlerRegistersAllSignals(t *testing.T) { + // Verify the signal handler code references all three signals + // by checking the source. This is a compile-time + source check. + src, err := os.ReadFile("run.go") + if err != nil { + t.Skip("cannot read run.go") + } + content := string(src) + for _, sig := range []string{"SIGINT", "SIGTERM", "SIGHUP"} { + if !strings.Contains(content, sig) { + t.Errorf("run.go should handle %s", sig) + } + } +} From 36723a73718a455e85e7d189c261b01fa65e98c1 Mon Sep 17 00:00:00 2001 From: contra Date: Sat, 5 Sep 2026 13:17:23 +0300 Subject: [PATCH 02/17] test: real subprocess signal tests for shellroute run 8 tests using real bash subprocesses: - SIGTERM forwarded, child exits via trap - SIGHUP forwarded, child exits via trap - SIGKILL escalation when child ignores SIGTERM - No escalation when child exits promptly - Signal to dead process doesn't panic - Second signal not swallowed during escalation wait - All three signals registered in run.go - sess.Stop() always called after Wait() Signed-off-by: contra --- internal/cli/run_signal_test.go | 185 +++++++++++++++++++++++++------- 1 file changed, 145 insertions(+), 40 deletions(-) diff --git a/internal/cli/run_signal_test.go b/internal/cli/run_signal_test.go index 14598ab..3e859c6 100644 --- a/internal/cli/run_signal_test.go +++ b/internal/cli/run_signal_test.go @@ -7,82 +7,187 @@ import ( "os/exec" "strings" "sync/atomic" + "syscall" "testing" "time" ) -// Tests for signal handling in shellroute run. -// These test the signal forwarding behavior by running a subprocess -// and verifying it receives the signal. - -func TestRunSignal_ChildReceivesSIGTERM(t *testing.T) { - if os.Getenv("SHELLROUTE_TEST_SIGNAL_CHILD") == "1" { - // Child process: sleep until killed, print what signal we got - cmd := exec.Command("sh", "-c", `trap 'echo GOT_SIGTERM; exit 0' TERM; while true; do sleep 0.1; done`) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - cmd.Run() - return - } - - // Parent: verify child receives SIGTERM when parent is killed - // This is a behavioral test — we verify the signal forwarding code compiles - // and the handler is registered. Full integration would need shellroute run - // with a real session. - t.Log("Signal handler compiles and registers correctly") +// TestSignal_ForwardSIGTERM: child receives SIGTERM and exits with trap code. +func TestSignal_ForwardSIGTERM(t *testing.T) { + cmd := exec.Command("bash", "-c", `trap 'exit 42' TERM; while true; do sleep 0.1; done`) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + time.Sleep(100 * time.Millisecond) + + syscall.Kill(cmd.Process.Pid, syscall.SIGTERM) + + err := cmd.Wait() + if exitErr, ok := err.(*exec.ExitError); ok { + if exitErr.ExitCode() != 42 { + t.Errorf("exit code = %d, want 42", exitErr.ExitCode()) + } + } else if err == nil { + t.Fatal("child should have exited non-zero from trap") + } } -func TestRunSignal_EscalateToSIGKILL(t *testing.T) { - var killed atomic.Bool - killCancel := make(chan struct{}) +// TestSignal_ForwardSIGHUP: child receives SIGHUP and exits with trap code. +func TestSignal_ForwardSIGHUP(t *testing.T) { + cmd := exec.Command("bash", "-c", `trap 'exit 43' HUP; while true; do sleep 0.1; done`) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + time.Sleep(100 * time.Millisecond) + + syscall.Kill(cmd.Process.Pid, syscall.SIGHUP) + + err := cmd.Wait() + if exitErr, ok := err.(*exec.ExitError); ok { + if exitErr.ExitCode() != 43 { + t.Errorf("exit code = %d, want 43", exitErr.ExitCode()) + } + } else if err == nil { + t.Fatal("child should have exited from SIGHUP trap") + } +} + +// TestSignal_EscalateToKILL: child ignores SIGTERM, gets SIGKILL after timeout. +func TestSignal_EscalateToKILL(t *testing.T) { + cmd := exec.Command("bash", "-c", `trap '' TERM; while true; do sleep 0.1; done`) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + time.Sleep(100 * time.Millisecond) + + var childRunning atomic.Bool + childRunning.Store(true) + + syscall.Kill(cmd.Process.Pid, syscall.SIGTERM) go func() { - select { - case <-time.After(100 * time.Millisecond): - killed.Store(true) - case <-killCancel: + time.Sleep(500 * time.Millisecond) // shortened for test + if childRunning.Load() { + syscall.Kill(cmd.Process.Pid, syscall.SIGKILL) } }() - time.Sleep(200 * time.Millisecond) - close(killCancel) + err := cmd.Wait() + childRunning.Store(false) - if !killed.Load() { - t.Error("escalation timer should have fired") + if err == nil { + t.Fatal("child should have been killed") } } -func TestRunSignal_NoEscalateIfChildExits(t *testing.T) { - var killed atomic.Bool +// TestSignal_NoEscalateIfPromptExit: child exits on SIGTERM, escalation cancelled. +func TestSignal_NoEscalateIfPromptExit(t *testing.T) { + cmd := exec.Command("bash", "-c", `trap 'exit 0' TERM; while true; do sleep 0.1; done`) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + time.Sleep(100 * time.Millisecond) + + var escalated atomic.Bool killCancel := make(chan struct{}) + syscall.Kill(cmd.Process.Pid, syscall.SIGTERM) + go func() { select { case <-time.After(5 * time.Second): - killed.Store(true) + escalated.Store(true) case <-killCancel: } }() + cmd.Wait() close(killCancel) time.Sleep(50 * time.Millisecond) - if killed.Load() { - t.Error("escalation should not fire when child exits promptly") + if escalated.Load() { + t.Error("should not escalate when child exits promptly") } } -func TestRunSignal_HandlerRegistersAllSignals(t *testing.T) { - // Verify the signal handler code references all three signals - // by checking the source. This is a compile-time + source check. +// TestSignal_IgnoredAfterChildExit: signalling dead process doesn't panic. +func TestSignal_IgnoredAfterChildExit(t *testing.T) { + cmd := exec.Command("bash", "-c", `exit 0`) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + cmd.Wait() + + // Signal after exit — should return error, not panic + err := syscall.Kill(cmd.Process.Pid, syscall.SIGTERM) + if err == nil { + t.Log("signal to dead process succeeded (PID reuse possible)") + } +} + +// TestSignal_SecondSignalReachesChild: second SIGTERM not swallowed. +func TestSignal_SecondSignalReachesChild(t *testing.T) { + cmd := exec.Command("bash", "-c", ` + count=0 + trap 'count=$((count+1)); if [ $count -ge 2 ]; then exit 44; fi' TERM + while true; do sleep 0.1; done + `) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + time.Sleep(100 * time.Millisecond) + + syscall.Kill(cmd.Process.Pid, syscall.SIGTERM) + time.Sleep(200 * time.Millisecond) + syscall.Kill(cmd.Process.Pid, syscall.SIGTERM) + + err := cmd.Wait() + if exitErr, ok := err.(*exec.ExitError); ok { + if exitErr.ExitCode() != 44 { + t.Errorf("exit code = %d, want 44", exitErr.ExitCode()) + } + } else if err == nil { + t.Fatal("child should have exited from second SIGTERM") + } +} + +// TestSignal_HandlerRegistersAllSignals: verify run.go registers all three. +func TestSignal_HandlerRegistersAllSignals(t *testing.T) { src, err := os.ReadFile("run.go") if err != nil { t.Skip("cannot read run.go") } content := string(src) - for _, sig := range []string{"SIGINT", "SIGTERM", "SIGHUP"} { + for _, sig := range []string{"syscall.SIGINT", "syscall.SIGTERM", "syscall.SIGHUP"} { if !strings.Contains(content, sig) { - t.Errorf("run.go should handle %s", sig) + t.Errorf("run.go should register %s", sig) } } } + +// TestSignal_CleanupAlwaysCalled: verify sess.Stop() path always runs after Wait. +// This is a structural check — the production code calls sess.Stop() unconditionally +// after childCmd.Wait(), which runs whether the child exits normally, from a signal, +// or from SIGKILL escalation. +func TestSignal_CleanupAlwaysCalled(t *testing.T) { + src, err := os.ReadFile("run.go") + if err != nil { + t.Skip("cannot read run.go") + } + content := string(src) + // sess.Stop() must appear after childCmd.Wait() + waitIdx := strings.Index(content, "childErr := childCmd.Wait()") + stopIdx := strings.Index(content, "resp, stopErr := sess.Stop()") + if waitIdx < 0 || stopIdx < 0 { + t.Fatal("cannot find Wait/Stop in run.go") + } + if stopIdx < waitIdx { + t.Error("sess.Stop() must be called after childCmd.Wait()") + } +} From 20b644dd163115714b86a603f9b5fc040ebfb273 Mon Sep 17 00:00:00 2001 From: contra Date: Sat, 5 Sep 2026 13:20:19 +0300 Subject: [PATCH 03/17] refactor: extract RunSignalHandler, fix second-signal swallow Production signal handler extracted to run_signal.go as RunSignalHandler. Fixes: - Second signal during escalation wait triggers immediate SIGKILL (was swallowed by single-read channel) - Buffer channel size 2 so second signal isn't lost - EscalateAfter injectable for tests run.go calls RunSignalHandler with process group PID. Tests call production RunSignalHandler directly: - SIGTERM forwarding via syscall.Kill(Getpid()) - SIGHUP forwarding - Timeout escalation to SIGKILL - Second signal immediate SIGKILL - No escalation when child exits promptly - No panic after child exit Signed-off-by: contra --- internal/cli/run.go | 34 +---- internal/cli/run_signal.go | 62 ++++++++ internal/cli/run_signal_test.go | 256 ++++++++++++++++---------------- 3 files changed, 200 insertions(+), 152 deletions(-) create mode 100644 internal/cli/run_signal.go diff --git a/internal/cli/run.go b/internal/cli/run.go index 2e7e701..b5c3234 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -7,7 +7,6 @@ import ( "fmt" "os" "os/exec" - "os/signal" "strings" "sync/atomic" "syscall" @@ -170,36 +169,19 @@ func runRun(cmd *cobra.Command, args []string) error { fmt.Fprintln(os.Stderr, "\n Connection lost during execution. Command killed.") } - // Handle SIGINT/SIGTERM/SIGHUP: forward to child, wait for it to exit, - // then clean up the session. This ensures systemd stop, Ctrl+C, and + // Handle SIGINT/SIGTERM/SIGHUP: forward to child, escalate if needed, + // then clean up the session. Ensures systemd stop, Ctrl+C, and // terminal close all end the API session cleanly. - sigCh := make(chan os.Signal, 1) - signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) - go func() { - sig, ok := <-sigCh - if !ok || !childRunning.Load() { - return - } - // Forward signal to child process group - if childCmd.Process != nil { - syscall.Kill(-childCmd.Process.Pid, sig.(syscall.Signal)) - } - // Escalate after 5 seconds if child doesn't exit - go func() { - select { - case <-time.After(5 * time.Second): - if childRunning.Load() && childCmd.Process != nil { - syscall.Kill(-childCmd.Process.Pid, syscall.SIGKILL) - } - case <-killCancel: - } - }() - }() + cleanupSig := RunSignalHandler(SignalHandlerConfig{ + Pid: -childCmd.Process.Pid, // negative = process group + ChildRunning: &childRunning, + KillCancel: killCancel, + }) childErr := childCmd.Wait() childRunning.Store(false) close(killCancel) - signal.Stop(sigCh) + cleanupSig() // Tear down session — always called, even after signal resp, stopErr := sess.Stop() diff --git a/internal/cli/run_signal.go b/internal/cli/run_signal.go new file mode 100644 index 0000000..1f49fb4 --- /dev/null +++ b/internal/cli/run_signal.go @@ -0,0 +1,62 @@ +//go:build !windows + +package cli + +import ( + "os" + "os/signal" + "sync/atomic" + "syscall" + "time" +) + +// SignalHandlerConfig holds the dependencies for the child signal handler. +type SignalHandlerConfig struct { + Pid int // child process PID (positive = process, negative = group) + ChildRunning *atomic.Bool // set to false when child exits + KillCancel chan struct{} // closed when child exits (cancels escalation) + EscalateAfter time.Duration // duration before SIGKILL escalation (default 5s) +} + +// RunSignalHandler listens for SIGINT/SIGTERM/SIGHUP, forwards to the child +// process group, and escalates to SIGKILL if the child doesn't exit in time. +// A second signal during the escalation wait triggers immediate SIGKILL. +// Returns a cleanup function that must be called after the child exits. +func RunSignalHandler(cfg SignalHandlerConfig) func() { + if cfg.EscalateAfter == 0 { + cfg.EscalateAfter = 5 * time.Second + } + + sigCh := make(chan os.Signal, 2) // buffer 2 so second signal isn't lost + signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) + + go func() { + sig, ok := <-sigCh + if !ok || !cfg.ChildRunning.Load() { + return + } + + // Forward first signal to child process group + syscall.Kill(cfg.Pid, sig.(syscall.Signal)) + + // Wait for child exit, escalation timeout, or second signal + select { + case <-time.After(cfg.EscalateAfter): + if cfg.ChildRunning.Load() { + syscall.Kill(cfg.Pid, syscall.SIGKILL) + } + case <-sigCh: + // Second signal — immediate SIGKILL + if cfg.ChildRunning.Load() { + syscall.Kill(cfg.Pid, syscall.SIGKILL) + } + case <-cfg.KillCancel: + // Child exited normally + } + }() + + return func() { + signal.Stop(sigCh) + close(sigCh) + } +} diff --git a/internal/cli/run_signal_test.go b/internal/cli/run_signal_test.go index 3e859c6..7b9561f 100644 --- a/internal/cli/run_signal_test.go +++ b/internal/cli/run_signal_test.go @@ -3,48 +3,80 @@ package cli import ( - "os" "os/exec" - "strings" "sync/atomic" "syscall" "testing" "time" ) -// TestSignal_ForwardSIGTERM: child receives SIGTERM and exits with trap code. -func TestSignal_ForwardSIGTERM(t *testing.T) { - cmd := exec.Command("bash", "-c", `trap 'exit 42' TERM; while true; do sleep 0.1; done`) +// All tests call the production RunSignalHandler from run_signal.go. + +func startChild(t *testing.T, script string) *exec.Cmd { + t.Helper() + cmd := exec.Command("bash", "-c", script) cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} if err := cmd.Start(); err != nil { t.Fatal(err) } - time.Sleep(100 * time.Millisecond) + time.Sleep(100 * time.Millisecond) // let trap register + return cmd +} + +// TestHandler_ForwardSIGTERM: production handler forwards SIGTERM to child group. +func TestHandler_ForwardSIGTERM(t *testing.T) { + cmd := startChild(t, `trap 'exit 42' TERM; while true; do sleep 0.1; done`) + + var running atomic.Bool + running.Store(true) + cancel := make(chan struct{}) - syscall.Kill(cmd.Process.Pid, syscall.SIGTERM) + cleanup := RunSignalHandler(SignalHandlerConfig{ + Pid: -cmd.Process.Pid, + ChildRunning: &running, + KillCancel: cancel, + EscalateAfter: 5 * time.Second, + }) + + // Simulate: OS sends SIGTERM to our process (via the handler's signal channel) + syscall.Kill(syscall.Getpid(), syscall.SIGTERM) err := cmd.Wait() + running.Store(false) + close(cancel) + cleanup() + if exitErr, ok := err.(*exec.ExitError); ok { if exitErr.ExitCode() != 42 { t.Errorf("exit code = %d, want 42", exitErr.ExitCode()) } } else if err == nil { - t.Fatal("child should have exited non-zero from trap") + t.Fatal("child should have exited from SIGTERM trap") } } -// TestSignal_ForwardSIGHUP: child receives SIGHUP and exits with trap code. -func TestSignal_ForwardSIGHUP(t *testing.T) { - cmd := exec.Command("bash", "-c", `trap 'exit 43' HUP; while true; do sleep 0.1; done`) - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - if err := cmd.Start(); err != nil { - t.Fatal(err) - } - time.Sleep(100 * time.Millisecond) +// TestHandler_ForwardSIGHUP: production handler forwards SIGHUP. +func TestHandler_ForwardSIGHUP(t *testing.T) { + cmd := startChild(t, `trap 'exit 43' HUP; while true; do sleep 0.1; done`) + + var running atomic.Bool + running.Store(true) + cancel := make(chan struct{}) + + cleanup := RunSignalHandler(SignalHandlerConfig{ + Pid: -cmd.Process.Pid, + ChildRunning: &running, + KillCancel: cancel, + EscalateAfter: 5 * time.Second, + }) - syscall.Kill(cmd.Process.Pid, syscall.SIGHUP) + syscall.Kill(syscall.Getpid(), syscall.SIGHUP) err := cmd.Wait() + running.Store(false) + close(cancel) + cleanup() + if exitErr, ok := err.(*exec.ExitError); ok { if exitErr.ExitCode() != 43 { t.Errorf("exit code = %d, want 43", exitErr.ExitCode()) @@ -54,140 +86,112 @@ func TestSignal_ForwardSIGHUP(t *testing.T) { } } -// TestSignal_EscalateToKILL: child ignores SIGTERM, gets SIGKILL after timeout. -func TestSignal_EscalateToKILL(t *testing.T) { - cmd := exec.Command("bash", "-c", `trap '' TERM; while true; do sleep 0.1; done`) - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - if err := cmd.Start(); err != nil { - t.Fatal(err) - } - time.Sleep(100 * time.Millisecond) +// TestHandler_EscalateToKILL: child ignores SIGTERM, handler escalates to SIGKILL. +func TestHandler_EscalateToKILL(t *testing.T) { + cmd := startChild(t, `trap '' TERM; while true; do sleep 0.1; done`) - var childRunning atomic.Bool - childRunning.Store(true) + var running atomic.Bool + running.Store(true) + cancel := make(chan struct{}) - syscall.Kill(cmd.Process.Pid, syscall.SIGTERM) + cleanup := RunSignalHandler(SignalHandlerConfig{ + Pid: -cmd.Process.Pid, + ChildRunning: &running, + KillCancel: cancel, + EscalateAfter: 500 * time.Millisecond, // shortened for test + }) - go func() { - time.Sleep(500 * time.Millisecond) // shortened for test - if childRunning.Load() { - syscall.Kill(cmd.Process.Pid, syscall.SIGKILL) - } - }() + syscall.Kill(syscall.Getpid(), syscall.SIGTERM) err := cmd.Wait() - childRunning.Store(false) + running.Store(false) + close(cancel) + cleanup() if err == nil { t.Fatal("child should have been killed") } } -// TestSignal_NoEscalateIfPromptExit: child exits on SIGTERM, escalation cancelled. -func TestSignal_NoEscalateIfPromptExit(t *testing.T) { - cmd := exec.Command("bash", "-c", `trap 'exit 0' TERM; while true; do sleep 0.1; done`) - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - if err := cmd.Start(); err != nil { - t.Fatal(err) - } - time.Sleep(100 * time.Millisecond) +// TestHandler_SecondSignalImmediateKILL: second signal during escalation wait +// triggers immediate SIGKILL instead of waiting for the timer. +func TestHandler_SecondSignalImmediateKILL(t *testing.T) { + cmd := startChild(t, `trap '' TERM INT; while true; do sleep 0.1; done`) - var escalated atomic.Bool - killCancel := make(chan struct{}) + var running atomic.Bool + running.Store(true) + cancel := make(chan struct{}) - syscall.Kill(cmd.Process.Pid, syscall.SIGTERM) + cleanup := RunSignalHandler(SignalHandlerConfig{ + Pid: -cmd.Process.Pid, + ChildRunning: &running, + KillCancel: cancel, + EscalateAfter: 30 * time.Second, // long timer — second signal should beat it + }) - go func() { - select { - case <-time.After(5 * time.Second): - escalated.Store(true) - case <-killCancel: - } - }() + start := time.Now() + syscall.Kill(syscall.Getpid(), syscall.SIGTERM) + time.Sleep(200 * time.Millisecond) + syscall.Kill(syscall.Getpid(), syscall.SIGINT) // second signal - cmd.Wait() - close(killCancel) - time.Sleep(50 * time.Millisecond) + err := cmd.Wait() + elapsed := time.Since(start) + running.Store(false) + close(cancel) + cleanup() - if escalated.Load() { - t.Error("should not escalate when child exits promptly") + if err == nil { + t.Fatal("child should have been killed") + } + if elapsed > 5*time.Second { + t.Errorf("took %v — second signal should have triggered immediate SIGKILL", elapsed) } } -// TestSignal_IgnoredAfterChildExit: signalling dead process doesn't panic. -func TestSignal_IgnoredAfterChildExit(t *testing.T) { - cmd := exec.Command("bash", "-c", `exit 0`) - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - if err := cmd.Start(); err != nil { - t.Fatal(err) - } +// TestHandler_NoEscalateIfChildExits: child exits promptly, no SIGKILL. +func TestHandler_NoEscalateIfChildExits(t *testing.T) { + cmd := startChild(t, `trap 'exit 0' TERM; while true; do sleep 0.1; done`) + + var running atomic.Bool + running.Store(true) + cancel := make(chan struct{}) + + cleanup := RunSignalHandler(SignalHandlerConfig{ + Pid: -cmd.Process.Pid, + ChildRunning: &running, + KillCancel: cancel, + EscalateAfter: 5 * time.Second, + }) + + syscall.Kill(syscall.Getpid(), syscall.SIGTERM) + cmd.Wait() + running.Store(false) + close(cancel) + cleanup() - // Signal after exit — should return error, not panic - err := syscall.Kill(cmd.Process.Pid, syscall.SIGTERM) - if err == nil { - t.Log("signal to dead process succeeded (PID reuse possible)") - } + // If we get here without hanging, escalation was cancelled } -// TestSignal_SecondSignalReachesChild: second SIGTERM not swallowed. -func TestSignal_SecondSignalReachesChild(t *testing.T) { - cmd := exec.Command("bash", "-c", ` - count=0 - trap 'count=$((count+1)); if [ $count -ge 2 ]; then exit 44; fi' TERM - while true; do sleep 0.1; done - `) - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - if err := cmd.Start(); err != nil { - t.Fatal(err) - } - time.Sleep(100 * time.Millisecond) +// TestHandler_IgnoredAfterChildExit: handler doesn't panic on signal after child exits. +func TestHandler_IgnoredAfterChildExit(t *testing.T) { + cmd := startChild(t, `exit 0`) - syscall.Kill(cmd.Process.Pid, syscall.SIGTERM) - time.Sleep(200 * time.Millisecond) - syscall.Kill(cmd.Process.Pid, syscall.SIGTERM) + var running atomic.Bool + running.Store(true) + cancel := make(chan struct{}) - err := cmd.Wait() - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() != 44 { - t.Errorf("exit code = %d, want 44", exitErr.ExitCode()) - } - } else if err == nil { - t.Fatal("child should have exited from second SIGTERM") - } -} + cleanup := RunSignalHandler(SignalHandlerConfig{ + Pid: -cmd.Process.Pid, + ChildRunning: &running, + KillCancel: cancel, + EscalateAfter: 5 * time.Second, + }) -// TestSignal_HandlerRegistersAllSignals: verify run.go registers all three. -func TestSignal_HandlerRegistersAllSignals(t *testing.T) { - src, err := os.ReadFile("run.go") - if err != nil { - t.Skip("cannot read run.go") - } - content := string(src) - for _, sig := range []string{"syscall.SIGINT", "syscall.SIGTERM", "syscall.SIGHUP"} { - if !strings.Contains(content, sig) { - t.Errorf("run.go should register %s", sig) - } - } -} + cmd.Wait() + running.Store(false) + close(cancel) + cleanup() -// TestSignal_CleanupAlwaysCalled: verify sess.Stop() path always runs after Wait. -// This is a structural check — the production code calls sess.Stop() unconditionally -// after childCmd.Wait(), which runs whether the child exits normally, from a signal, -// or from SIGKILL escalation. -func TestSignal_CleanupAlwaysCalled(t *testing.T) { - src, err := os.ReadFile("run.go") - if err != nil { - t.Skip("cannot read run.go") - } - content := string(src) - // sess.Stop() must appear after childCmd.Wait() - waitIdx := strings.Index(content, "childErr := childCmd.Wait()") - stopIdx := strings.Index(content, "resp, stopErr := sess.Stop()") - if waitIdx < 0 || stopIdx < 0 { - t.Fatal("cannot find Wait/Stop in run.go") - } - if stopIdx < waitIdx { - t.Error("sess.Stop() must be called after childCmd.Wait()") - } + // No panic = pass } From 4868b27dbebeea2402df52e0990c1ae3dbd94544 Mon Sep 17 00:00:00 2001 From: contra Date: Sat, 5 Sep 2026 13:42:33 +0300 Subject: [PATCH 04/17] test: add live signal-handling test for shellroute run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scripts/test-run-signal-live.sh --live [COUNTRY] Requires --live opt-in (creates one paid session). Builds from current checkout, launches child with SIGTERM trap, sends SIGTERM to shellroute, verifies: child received signal, session ended cleanly, no orphan processes. Not in CI — manual verification for release gating. Signed-off-by: contra --- scripts/test-run-signal-live.sh | 187 ++++++++++++++++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100755 scripts/test-run-signal-live.sh diff --git a/scripts/test-run-signal-live.sh b/scripts/test-run-signal-live.sh new file mode 100755 index 0000000..ec43450 --- /dev/null +++ b/scripts/test-run-signal-live.sh @@ -0,0 +1,187 @@ +#!/usr/bin/env bash +set -uo pipefail + +# Live signal-handling test for shellroute run. +# Creates one real (paid) session to verify SIGTERM forwarding and clean shutdown. +# +# Usage: +# ./scripts/test-run-signal-live.sh --live [COUNTRY] +# +# Requirements: +# - Authenticated shellroute (shellroute login or SHELLROUTE_API_KEY) +# - Current checkout builds successfully +# +# Default country: US + +usage() { + echo "Usage: $0 --live [COUNTRY]" + echo "" + echo "Live signal-handling test for shellroute run." + echo "Creates one real minimally-used paid session." + echo "" + echo "Options:" + echo " --live Required. Confirms you accept one paid session." + echo " --help Show this help." + echo "" + echo "Arguments:" + echo " COUNTRY ISO country code (default: US)" + exit 0 +} + +# --- Parse args --- +LIVE=false +COUNTRY=US +for arg in "$@"; do + case "$arg" in + --live) LIVE=true ;; + --help|-h) usage ;; + *) COUNTRY="$arg" ;; + esac +done + +if [ "$LIVE" != "true" ]; then + echo "Error: this test creates a real paid session." + echo "Run with --live to confirm: $0 --live [$COUNTRY]" + exit 1 +fi + +# --- Build from current checkout --- +REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +BUILD_DIR=$(mktemp -d) +trap 'rm -rf "$BUILD_DIR" "$READY_FILE" 2>/dev/null; [ -n "${SR_PID:-}" ] && kill "$SR_PID" 2>/dev/null; wait "$SR_PID" 2>/dev/null' EXIT + +echo "=== Building shellroute from current checkout ===" +go build -o "$BUILD_DIR/shellroute" "$REPO_ROOT/cmd/shellroute" 2>&1 +if [ $? -ne 0 ]; then + echo "FAIL: build failed" + exit 1 +fi +SR="$BUILD_DIR/shellroute" +echo "Built: $SR" + +# --- Verify auth --- +if ! "$SR" balance >/dev/null 2>&1; then + echo "FAIL: not authenticated. Run shellroute login first." + exit 1 +fi +echo "Auth: ok" + +# --- Create readiness signal file --- +READY_FILE=$(mktemp) +rm -f "$READY_FILE" + +# --- Child script: signals readiness, traps SIGTERM, exits with code 42 --- +CHILD_SCRIPT=' +ready_file="$1" +trap '"'"'echo CHILD_GOT_SIGTERM; exit 42'"'"' TERM +touch "$ready_file" +while true; do sleep 0.1; done +' + +echo "" +echo "=== Running shellroute run $COUNTRY with signal test ===" + +# Launch shellroute run in background with a child that traps SIGTERM +"$SR" run "$COUNTRY" -- bash -c "$CHILD_SCRIPT" -- "$READY_FILE" >"$BUILD_DIR/stdout" 2>"$BUILD_DIR/stderr" & +SR_PID=$! + +# --- Wait for child readiness (max 60s) --- +echo "Waiting for child readiness..." +WAITED=0 +while [ ! -f "$READY_FILE" ] && [ $WAITED -lt 60 ]; do + sleep 1 + WAITED=$((WAITED + 1)) + # Check if shellroute already exited (connection failure) + if ! kill -0 "$SR_PID" 2>/dev/null; then + echo "FAIL: shellroute exited before child was ready." + echo "--- stdout ---" + cat "$BUILD_DIR/stdout" + echo "--- stderr ---" + cat "$BUILD_DIR/stderr" + exit 1 + fi +done + +if [ ! -f "$READY_FILE" ]; then + echo "FAIL: child did not signal readiness within 60s." + kill "$SR_PID" 2>/dev/null + exit 1 +fi +echo "Child ready after ${WAITED}s." + +# --- Send SIGTERM to shellroute process --- +echo "Sending SIGTERM to shellroute (PID $SR_PID)..." +kill -TERM "$SR_PID" + +# --- Wait for shellroute to exit (max 15s) --- +WAITED=0 +while kill -0 "$SR_PID" 2>/dev/null && [ $WAITED -lt 15 ]; do + sleep 1 + WAITED=$((WAITED + 1)) +done + +if kill -0 "$SR_PID" 2>/dev/null; then + echo "FAIL: shellroute did not exit within 15s after SIGTERM." + kill -9 "$SR_PID" 2>/dev/null + exit 1 +fi + +wait "$SR_PID" 2>/dev/null +SR_EXIT=$? +SR_PID="" + +echo "Shellroute exited (code $SR_EXIT) after ${WAITED}s." + +# --- Verify results --- +PASS=0 +FAIL=0 + +echo "" +echo "=== Verification ===" + +# 1. Child received SIGTERM (printed CHILD_GOT_SIGTERM) +if grep -q "CHILD_GOT_SIGTERM" "$BUILD_DIR/stdout"; then + echo " PASS: child received SIGTERM" + PASS=$((PASS + 1)) +else + echo " FAIL: child did not receive SIGTERM" + FAIL=$((FAIL + 1)) +fi + +# 2. Session ended cleanly (stderr contains "session ended") +if grep -q "session ended" "$BUILD_DIR/stderr"; then + echo " PASS: session ended cleanly" + PASS=$((PASS + 1)) +else + echo " FAIL: no 'session ended' in stderr" + FAIL=$((FAIL + 1)) +fi + +# 3. No child process remains +if pgrep -f "CHILD_GOT_SIGTERM" >/dev/null 2>&1; then + echo " FAIL: child process still running" + FAIL=$((FAIL + 1)) +else + echo " PASS: no child process remains" + PASS=$((PASS + 1)) +fi + +# 4. Shellroute exited (already verified above, but confirm non-hang) +echo " PASS: shellroute exited within timeout" +PASS=$((PASS + 1)) + +echo "" +echo "=== Results ===" +echo "Passed: $PASS Failed: $FAIL" + +if [ $FAIL -gt 0 ]; then + echo "" + echo "--- stdout ---" + cat "$BUILD_DIR/stdout" + echo "--- stderr ---" + cat "$BUILD_DIR/stderr" + exit 1 +fi + +echo "" +echo "Signal handling verified: SIGTERM → child forwarded → session ended cleanly." From 5a361a5bdbe82d39e5da57a8213c859b6d88ec81 Mon Sep 17 00:00:00 2001 From: contra Date: Sat, 5 Sep 2026 13:45:21 +0300 Subject: [PATCH 05/17] =?UTF-8?q?fix:=20live=20signal=20test=20=E2=80=94?= =?UTF-8?q?=20child=20exits=200,=20exact=20PGID=20check,=20strict=20args?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes all blockers: - Child trap exits 0 (not 42), assert shellroute exits 0 - Orphan check uses exact child PGID from temp file - All temp files in single WORK_DIR initialized before trap - Build from REPO_ROOT with ./cmd/shellroute - Strict arg parsing rejects unknown flags and extra args - Process group cleanup in EXIT trap - Exact 'shellroute session ended' marker - Output shown on every failure - CONTRIBUTING.md documents optional live tests Signed-off-by: contra --- CONTRIBUTING.md | 10 ++ scripts/test-run-signal-live.sh | 180 +++++++++++++++++++++----------- 2 files changed, 128 insertions(+), 62 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3e6ff7f..e8b25e3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,6 +25,16 @@ This runs: go vet, gofmt, build, unit tests (with race detector), public audit, 4. Run `./scripts/run-tests.sh` 5. Open a pull request +## Optional live tests + +Some tests create real paid sessions. They are not part of `run-tests.sh` and require explicit opt-in: + +```bash +./scripts/test-run-signal-live.sh --live +``` + +Run `--help` on each script for details. + ## DCO All commits must be signed off (`git commit -s`). This certifies you wrote the code or have the right to submit it under the Apache 2.0 license. diff --git a/scripts/test-run-signal-live.sh b/scripts/test-run-signal-live.sh index ec43450..6ae27dd 100755 --- a/scripts/test-run-signal-live.sh +++ b/scripts/test-run-signal-live.sh @@ -9,54 +9,94 @@ set -uo pipefail # # Requirements: # - Authenticated shellroute (shellroute login or SHELLROUTE_API_KEY) -# - Current checkout builds successfully -# -# Default country: US +# - Go toolchain (builds from current checkout) + +SCRIPT_NAME="$(basename "$0")" +REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" usage() { - echo "Usage: $0 --live [COUNTRY]" - echo "" - echo "Live signal-handling test for shellroute run." - echo "Creates one real minimally-used paid session." - echo "" - echo "Options:" - echo " --live Required. Confirms you accept one paid session." - echo " --help Show this help." - echo "" - echo "Arguments:" - echo " COUNTRY ISO country code (default: US)" + cat </dev/null; [ -n "${SR_PID:-}" ] && kill "$SR_PID" 2>/dev/null; wait "$SR_PID" 2>/dev/null' EXIT +# --- Create temp dir first (before any cleanup references) --- +WORK_DIR=$(mktemp -d) +READY_FILE="$WORK_DIR/child-ready" +PGID_FILE="$WORK_DIR/child-pgid" +SR_PID="" +cleanup() { + # Kill shellroute process group if still running + if [ -n "$SR_PID" ] && kill -0 "$SR_PID" 2>/dev/null; then + kill -TERM "$SR_PID" 2>/dev/null + sleep 1 + kill -9 "$SR_PID" 2>/dev/null + fi + # Kill child process group if recorded and still exists + if [ -f "$PGID_FILE" ]; then + local pgid + pgid=$(cat "$PGID_FILE" 2>/dev/null) + if [ -n "$pgid" ] && kill -0 "-$pgid" 2>/dev/null; then + kill -9 "-$pgid" 2>/dev/null + fi + fi + rm -rf "$WORK_DIR" +} +trap cleanup EXIT + +# --- Build from current checkout --- echo "=== Building shellroute from current checkout ===" -go build -o "$BUILD_DIR/shellroute" "$REPO_ROOT/cmd/shellroute" 2>&1 +(cd "$REPO_ROOT" && go build -o "$WORK_DIR/shellroute" ./cmd/shellroute) 2>&1 if [ $? -ne 0 ]; then echo "FAIL: build failed" exit 1 fi -SR="$BUILD_DIR/shellroute" +SR="$WORK_DIR/shellroute" echo "Built: $SR" # --- Verify auth --- @@ -66,23 +106,22 @@ if ! "$SR" balance >/dev/null 2>&1; then fi echo "Auth: ok" -# --- Create readiness signal file --- -READY_FILE=$(mktemp) -rm -f "$READY_FILE" +echo "" +echo "=== Running shellroute run $COUNTRY with signal test ===" -# --- Child script: signals readiness, traps SIGTERM, exits with code 42 --- +# --- Child script: writes PGID, signals readiness, traps SIGTERM, exits 0 --- CHILD_SCRIPT=' -ready_file="$1" -trap '"'"'echo CHILD_GOT_SIGTERM; exit 42'"'"' TERM +pgid_file="$1" +ready_file="$2" +echo $$ > "$pgid_file" +trap '"'"'echo CHILD_GOT_SIGTERM >&2; exit 0'"'"' TERM touch "$ready_file" while true; do sleep 0.1; done ' -echo "" -echo "=== Running shellroute run $COUNTRY with signal test ===" - -# Launch shellroute run in background with a child that traps SIGTERM -"$SR" run "$COUNTRY" -- bash -c "$CHILD_SCRIPT" -- "$READY_FILE" >"$BUILD_DIR/stdout" 2>"$BUILD_DIR/stderr" & +# Launch shellroute run in background +"$SR" run "$COUNTRY" -- bash -c "$CHILD_SCRIPT" -- "$PGID_FILE" "$READY_FILE" \ + >"$WORK_DIR/stdout" 2>"$WORK_DIR/stderr" & SR_PID=$! # --- Wait for child readiness (max 60s) --- @@ -91,20 +130,18 @@ WAITED=0 while [ ! -f "$READY_FILE" ] && [ $WAITED -lt 60 ]; do sleep 1 WAITED=$((WAITED + 1)) - # Check if shellroute already exited (connection failure) if ! kill -0 "$SR_PID" 2>/dev/null; then echo "FAIL: shellroute exited before child was ready." echo "--- stdout ---" - cat "$BUILD_DIR/stdout" + cat "$WORK_DIR/stdout" echo "--- stderr ---" - cat "$BUILD_DIR/stderr" + cat "$WORK_DIR/stderr" exit 1 fi done if [ ! -f "$READY_FILE" ]; then echo "FAIL: child did not signal readiness within 60s." - kill "$SR_PID" 2>/dev/null exit 1 fi echo "Child ready after ${WAITED}s." @@ -122,7 +159,10 @@ done if kill -0 "$SR_PID" 2>/dev/null; then echo "FAIL: shellroute did not exit within 15s after SIGTERM." - kill -9 "$SR_PID" 2>/dev/null + echo "--- stdout ---" + cat "$WORK_DIR/stdout" + echo "--- stderr ---" + cat "$WORK_DIR/stderr" exit 1 fi @@ -136,50 +176,66 @@ echo "Shellroute exited (code $SR_EXIT) after ${WAITED}s." PASS=0 FAIL=0 +show_output() { + echo "--- stdout ---" + cat "$WORK_DIR/stdout" + echo "--- stderr ---" + cat "$WORK_DIR/stderr" +} + echo "" echo "=== Verification ===" -# 1. Child received SIGTERM (printed CHILD_GOT_SIGTERM) -if grep -q "CHILD_GOT_SIGTERM" "$BUILD_DIR/stdout"; then +# 1. Shellroute exited 0 (child exited 0 from trap) +if [ "$SR_EXIT" -eq 0 ]; then + echo " PASS: shellroute exited 0" + PASS=$((PASS + 1)) +else + echo " FAIL: shellroute exited $SR_EXIT, want 0" + FAIL=$((FAIL + 1)) + show_output +fi + +# 2. Child received SIGTERM +if grep -q "CHILD_GOT_SIGTERM" "$WORK_DIR/stderr"; then echo " PASS: child received SIGTERM" PASS=$((PASS + 1)) else - echo " FAIL: child did not receive SIGTERM" + echo " FAIL: child did not print CHILD_GOT_SIGTERM" FAIL=$((FAIL + 1)) + show_output fi -# 2. Session ended cleanly (stderr contains "session ended") -if grep -q "session ended" "$BUILD_DIR/stderr"; then +# 3. Session ended cleanly +if grep -q "shellroute session ended" "$WORK_DIR/stderr"; then echo " PASS: session ended cleanly" PASS=$((PASS + 1)) else - echo " FAIL: no 'session ended' in stderr" + echo " FAIL: no 'shellroute session ended' in stderr" FAIL=$((FAIL + 1)) + show_output fi -# 3. No child process remains -if pgrep -f "CHILD_GOT_SIGTERM" >/dev/null 2>&1; then - echo " FAIL: child process still running" - FAIL=$((FAIL + 1)) +# 4. No child process remains (check exact PGID) +if [ -f "$PGID_FILE" ]; then + CHILD_PGID=$(cat "$PGID_FILE") + if [ -n "$CHILD_PGID" ] && kill -0 "$CHILD_PGID" 2>/dev/null; then + echo " FAIL: child process $CHILD_PGID still running" + FAIL=$((FAIL + 1)) + else + echo " PASS: child process $CHILD_PGID no longer exists" + PASS=$((PASS + 1)) + fi else - echo " PASS: no child process remains" - PASS=$((PASS + 1)) + echo " FAIL: child did not write PGID file" + FAIL=$((FAIL + 1)) fi -# 4. Shellroute exited (already verified above, but confirm non-hang) -echo " PASS: shellroute exited within timeout" -PASS=$((PASS + 1)) - echo "" echo "=== Results ===" echo "Passed: $PASS Failed: $FAIL" if [ $FAIL -gt 0 ]; then - echo "" - echo "--- stdout ---" - cat "$BUILD_DIR/stdout" - echo "--- stderr ---" - cat "$BUILD_DIR/stderr" exit 1 fi From 7f6e0725428f469c8efb9361f445840ee0a2af46 Mon Sep 17 00:00:00 2001 From: contra Date: Sat, 5 Sep 2026 13:46:43 +0300 Subject: [PATCH 06/17] fix: live test uses process group check, 7s graceful cleanup, guarded mktemp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Orphan verification: kill -0 -- -PGID (process group, not just PID) - Cleanup: SIGTERM → wait up to 7s for shellroute graceful path → SIGKILL - Child group cleanup: numeric PGID validation, kill -9 -- -PGID - mktemp failure guarded before any work - Exact marker: 'shellroute session ended.' with period Signed-off-by: contra --- scripts/test-run-signal-live.sh | 40 ++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/scripts/test-run-signal-live.sh b/scripts/test-run-signal-live.sh index 6ae27dd..548472e 100755 --- a/scripts/test-run-signal-live.sh +++ b/scripts/test-run-signal-live.sh @@ -65,27 +65,32 @@ if [ "$LIVE" != "true" ]; then fi # --- Create temp dir first (before any cleanup references) --- -WORK_DIR=$(mktemp -d) +WORK_DIR=$(mktemp -d) || { echo "FAIL: mktemp failed"; exit 1; } +[ -d "$WORK_DIR" ] || { echo "FAIL: temp dir does not exist"; exit 1; } READY_FILE="$WORK_DIR/child-ready" PGID_FILE="$WORK_DIR/child-pgid" SR_PID="" cleanup() { - # Kill shellroute process group if still running + # Give shellroute time for its graceful 5s escalation + sess.Stop() if [ -n "$SR_PID" ] && kill -0 "$SR_PID" 2>/dev/null; then kill -TERM "$SR_PID" 2>/dev/null - sleep 1 + local w=0 + while kill -0 "$SR_PID" 2>/dev/null && [ $w -lt 7 ]; do + sleep 1; w=$((w + 1)) + done kill -9 "$SR_PID" 2>/dev/null + wait "$SR_PID" 2>/dev/null fi - # Kill child process group if recorded and still exists + # Kill child process group by exact PGID if [ -f "$PGID_FILE" ]; then local pgid pgid=$(cat "$PGID_FILE" 2>/dev/null) - if [ -n "$pgid" ] && kill -0 "-$pgid" 2>/dev/null; then - kill -9 "-$pgid" 2>/dev/null + if [ -n "$pgid" ] && [ "$pgid" -gt 0 ] 2>/dev/null && kill -0 -- "-$pgid" 2>/dev/null; then + kill -9 -- "-$pgid" 2>/dev/null fi fi - rm -rf "$WORK_DIR" + [ -d "$WORK_DIR" ] && rm -rf "$WORK_DIR" } trap cleanup EXIT @@ -207,24 +212,29 @@ else fi # 3. Session ended cleanly -if grep -q "shellroute session ended" "$WORK_DIR/stderr"; then +if grep -qF "shellroute session ended." "$WORK_DIR/stderr"; then echo " PASS: session ended cleanly" PASS=$((PASS + 1)) else - echo " FAIL: no 'shellroute session ended' in stderr" + echo " FAIL: no 'shellroute session ended.' in stderr" FAIL=$((FAIL + 1)) show_output fi -# 4. No child process remains (check exact PGID) +# 4. No child process group remains (check exact PGID) if [ -f "$PGID_FILE" ]; then CHILD_PGID=$(cat "$PGID_FILE") - if [ -n "$CHILD_PGID" ] && kill -0 "$CHILD_PGID" 2>/dev/null; then - echo " FAIL: child process $CHILD_PGID still running" - FAIL=$((FAIL + 1)) + if [ -n "$CHILD_PGID" ] && [ "$CHILD_PGID" -gt 0 ] 2>/dev/null; then + if kill -0 -- "-$CHILD_PGID" 2>/dev/null; then + echo " FAIL: child process group $CHILD_PGID still running" + FAIL=$((FAIL + 1)) + else + echo " PASS: child process group $CHILD_PGID no longer exists" + PASS=$((PASS + 1)) + fi else - echo " PASS: child process $CHILD_PGID no longer exists" - PASS=$((PASS + 1)) + echo " FAIL: invalid PGID in file: '$CHILD_PGID'" + FAIL=$((FAIL + 1)) fi else echo " FAIL: child did not write PGID file" From e4e9332c0f0e80f1c8b392dfb73d94eac9d7b578 Mon Sep 17 00:00:00 2001 From: contra Date: Sat, 5 Sep 2026 13:48:05 +0300 Subject: [PATCH 07/17] fix: guard SIGKILL against PID reuse, 10s cleanup timeout, specific CONTRIBUTING entry Cleanup recheck kill -0 before SIGKILL to avoid hitting a reused PID. Timeout increased to 10s (5s child escalation + API teardown). CONTRIBUTING describes the specific test, not generic 'some tests'. Signed-off-by: contra --- CONTRIBUTING.md | 8 ++++---- scripts/test-run-signal-live.sh | 9 ++++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e8b25e3..23b12a5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,15 +25,15 @@ This runs: go vet, gofmt, build, unit tests (with race detector), public audit, 4. Run `./scripts/run-tests.sh` 5. Open a pull request -## Optional live tests +## Optional live test -Some tests create real paid sessions. They are not part of `run-tests.sh` and require explicit opt-in: +`test-run-signal-live.sh` verifies that `shellroute run` forwards SIGTERM to the child process and ends the API session cleanly. It creates one real paid session and requires `--live`: ```bash -./scripts/test-run-signal-live.sh --live +./scripts/test-run-signal-live.sh --live [COUNTRY] ``` -Run `--help` on each script for details. +Not part of `run-tests.sh` or CI. Run manually before releasing signal-handling changes. ## DCO diff --git a/scripts/test-run-signal-live.sh b/scripts/test-run-signal-live.sh index 548472e..1ad20c7 100755 --- a/scripts/test-run-signal-live.sh +++ b/scripts/test-run-signal-live.sh @@ -72,14 +72,17 @@ PGID_FILE="$WORK_DIR/child-pgid" SR_PID="" cleanup() { - # Give shellroute time for its graceful 5s escalation + sess.Stop() + # Give shellroute time for 5s child escalation + API session teardown (~10s total) if [ -n "$SR_PID" ] && kill -0 "$SR_PID" 2>/dev/null; then kill -TERM "$SR_PID" 2>/dev/null local w=0 - while kill -0 "$SR_PID" 2>/dev/null && [ $w -lt 7 ]; do + while kill -0 "$SR_PID" 2>/dev/null && [ $w -lt 10 ]; do sleep 1; w=$((w + 1)) done - kill -9 "$SR_PID" 2>/dev/null + # Only SIGKILL if still alive (avoid PID reuse) + if kill -0 "$SR_PID" 2>/dev/null; then + kill -9 "$SR_PID" 2>/dev/null + fi wait "$SR_PID" 2>/dev/null fi # Kill child process group by exact PGID From 532575e7e326dcdffab0c544956a81a9d928cd43 Mon Sep 17 00:00:00 2001 From: contra Date: Sat, 5 Sep 2026 13:48:38 +0300 Subject: [PATCH 08/17] fix: cleanup timeout 25s (5s escalation + 15s API timeout + margin) Signed-off-by: contra --- scripts/test-run-signal-live.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/test-run-signal-live.sh b/scripts/test-run-signal-live.sh index 1ad20c7..8d9a8f3 100755 --- a/scripts/test-run-signal-live.sh +++ b/scripts/test-run-signal-live.sh @@ -72,11 +72,11 @@ PGID_FILE="$WORK_DIR/child-pgid" SR_PID="" cleanup() { - # Give shellroute time for 5s child escalation + API session teardown (~10s total) + # Give shellroute time for 5s child escalation + 15s API client timeout + margin if [ -n "$SR_PID" ] && kill -0 "$SR_PID" 2>/dev/null; then kill -TERM "$SR_PID" 2>/dev/null local w=0 - while kill -0 "$SR_PID" 2>/dev/null && [ $w -lt 10 ]; do + while kill -0 "$SR_PID" 2>/dev/null && [ $w -lt 25 ]; do sleep 1; w=$((w + 1)) done # Only SIGKILL if still alive (avoid PID reuse) From 2335bc49d3d1a38173fd454fe3f3fa646043c56c Mon Sep 17 00:00:00 2001 From: contra Date: Sat, 5 Sep 2026 13:49:36 +0300 Subject: [PATCH 09/17] fix: normal test wait 20s to match API client timeout Signed-off-by: contra --- scripts/test-run-signal-live.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/test-run-signal-live.sh b/scripts/test-run-signal-live.sh index 8d9a8f3..95931f2 100755 --- a/scripts/test-run-signal-live.sh +++ b/scripts/test-run-signal-live.sh @@ -158,15 +158,15 @@ echo "Child ready after ${WAITED}s." echo "Sending SIGTERM to shellroute (PID $SR_PID)..." kill -TERM "$SR_PID" -# --- Wait for shellroute to exit (max 15s) --- +# --- Wait for shellroute to exit (max 20s: 5s escalation + 15s API timeout) --- WAITED=0 -while kill -0 "$SR_PID" 2>/dev/null && [ $WAITED -lt 15 ]; do +while kill -0 "$SR_PID" 2>/dev/null && [ $WAITED -lt 20 ]; do sleep 1 WAITED=$((WAITED + 1)) done if kill -0 "$SR_PID" 2>/dev/null; then - echo "FAIL: shellroute did not exit within 15s after SIGTERM." + echo "FAIL: shellroute did not exit within 20s after SIGTERM." echo "--- stdout ---" cat "$WORK_DIR/stdout" echo "--- stderr ---" From e15530660e4b9af888aa1d4b273bd446b9f43445 Mon Sep 17 00:00:00 2001 From: contra Date: Sat, 5 Sep 2026 13:51:02 +0300 Subject: [PATCH 10/17] style: fix gofmt alignment in run_signal.go Signed-off-by: contra --- internal/cli/run_signal.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/cli/run_signal.go b/internal/cli/run_signal.go index 1f49fb4..258e418 100644 --- a/internal/cli/run_signal.go +++ b/internal/cli/run_signal.go @@ -12,9 +12,9 @@ import ( // SignalHandlerConfig holds the dependencies for the child signal handler. type SignalHandlerConfig struct { - Pid int // child process PID (positive = process, negative = group) - ChildRunning *atomic.Bool // set to false when child exits - KillCancel chan struct{} // closed when child exits (cancels escalation) + Pid int // child process PID (positive = process, negative = group) + ChildRunning *atomic.Bool // set to false when child exits + KillCancel chan struct{} // closed when child exits (cancels escalation) EscalateAfter time.Duration // duration before SIGKILL escalation (default 5s) } From d2b5c4de596eb3331b4f3954a2776cd0e4dc5c0a Mon Sep 17 00:00:00 2001 From: contra Date: Mon, 7 Sep 2026 19:46:42 +0300 Subject: [PATCH 11/17] =?UTF-8?q?fix(run):=20exit=20the=20way=20the=20chil?= =?UTF-8?q?d=20did=20=E2=80=94=20re-raise=20its=20signal,=20keep=20its=20c?= =?UTF-8?q?ode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A child killed by a signal made shellroute exit 255 (Go reports -1 for signal deaths) and print "Command failed". Under systemd stop with the default KillMode, most programs die from SIGTERM, so a clean stop left the unit in "failed (status=255)"; Ctrl+C gave 255 instead of 130. Now shellroute mirrors the child: exit codes pass through, and SIGINT/ SIGTERM/SIGHUP/SIGKILL deaths are re-raised on shellroute itself so systemd sees a clean stop and shells get the 128+n convention. Other signals use 128+n directly because the Go runtime would turn them into a crash dump. "Command failed" is only printed for non-zero exit codes. Signed-off-by: contra --- internal/cli/run.go | 13 ++-- internal/cli/run_exit_test.go | 140 ++++++++++++++++++++++++++++++++++ internal/cli/run_signal.go | 30 ++++++++ 3 files changed, 176 insertions(+), 7 deletions(-) create mode 100644 internal/cli/run_exit_test.go diff --git a/internal/cli/run.go b/internal/cli/run.go index b5c3234..34beba7 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -185,8 +185,10 @@ func runRun(cmd *cobra.Command, args []string) error { // Tear down session — always called, even after signal resp, stopErr := sess.Stop() + exitErr, _ := childErr.(*exec.ExitError) if !runNoStat { - if childErr != nil { + // A signal death is reported by exiting the same way, not as a failure. + if childErr != nil && (exitErr == nil || childSignal(exitErr) == 0) { display.Error("Command failed: %s", args[0]) } if stopErr != nil { @@ -199,13 +201,10 @@ func runRun(cmd *cobra.Command, args []string) error { } } - if childErr != nil { - if exitErr, ok := childErr.(*exec.ExitError); ok { - os.Exit(exitErr.ExitCode()) - } - return childErr + if exitErr != nil { + exitAsChild(exitErr) } - return nil + return childErr } const defaultNoProxy = "localhost,127.0.0.1,::1" diff --git a/internal/cli/run_exit_test.go b/internal/cli/run_exit_test.go new file mode 100644 index 0000000..cea120f --- /dev/null +++ b/internal/cli/run_exit_test.go @@ -0,0 +1,140 @@ +//go:build !windows + +package cli + +import ( + "errors" + "fmt" + "os" + "os/exec" + "syscall" + "testing" + "time" +) + +// exitAsChild and exitFromSignal end the process, so each case runs in a +// re-executed test binary driven by TestHelperProcess. + +func TestHelperProcess(t *testing.T) { + switch os.Getenv("SR_TEST_HELPER") { + case "": + return + case "exit-as-child": + var cmd *exec.Cmd + if code := os.Getenv("SR_TEST_CHILD_EXIT"); code != "" { + cmd = exec.Command("bash", "-c", "exit "+code) + } else { + cmd = exec.Command("sleep", "30") + } + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + if err := cmd.Start(); err != nil { + fmt.Println(err) + os.Exit(99) + } + if name := os.Getenv("SR_TEST_CHILD_SIG"); name != "" { + time.Sleep(200 * time.Millisecond) + syscall.Kill(-cmd.Process.Pid, signalByName(name)) + } + err := cmd.Wait() + var ee *exec.ExitError + if errors.As(err, &ee) { + exitAsChild(ee) + } + if err != nil { + fmt.Println(err) + os.Exit(99) + } + os.Exit(0) + case "exit-from-signal": + exitFromSignal(signalByName(os.Getenv("SR_TEST_SIG"))) + } +} + +func signalByName(name string) syscall.Signal { + switch name { + case "INT": + return syscall.SIGINT + case "TERM": + return syscall.SIGTERM + case "HUP": + return syscall.SIGHUP + case "KILL": + return syscall.SIGKILL + case "USR1": + return syscall.SIGUSR1 + } + panic("unknown signal " + name) +} + +type helperResult struct { + code int // exit status, or -1 when killed by a signal + sig syscall.Signal + out string +} + +// runHelper re-executes the test binary running only TestHelperProcess. +// With hupIgnored, SIGHUP is ignored on entry, as under nohup. +func runHelper(t *testing.T, hupIgnored bool, env ...string) helperResult { + t.Helper() + var cmd *exec.Cmd + if hupIgnored { + cmd = exec.Command("bash", "-c", `trap '' HUP; exec "$0" -test.run='^TestHelperProcess$'`, os.Args[0]) + } else { + cmd = exec.Command(os.Args[0], "-test.run=^TestHelperProcess$") + } + cmd.Env = append(os.Environ(), env...) + out, err := cmd.CombinedOutput() + res := helperResult{out: string(out)} + if err == nil { + return res + } + var ee *exec.ExitError + if !errors.As(err, &ee) { + t.Fatalf("helper: %v", err) + } + ws := ee.Sys().(syscall.WaitStatus) + if ws.Signaled() { + res.code, res.sig = -1, ws.Signal() + } else { + res.code = ws.ExitStatus() + } + return res +} + +func TestExitAsChild(t *testing.T) { + cases := []struct { + name string + env []string + wantCode int + wantSig syscall.Signal + }{ + {"exit code kept", []string{"SR_TEST_CHILD_EXIT=7"}, 7, 0}, + {"SIGTERM re-raised", []string{"SR_TEST_CHILD_SIG=TERM"}, -1, syscall.SIGTERM}, + {"SIGINT re-raised", []string{"SR_TEST_CHILD_SIG=INT"}, -1, syscall.SIGINT}, + {"SIGHUP re-raised", []string{"SR_TEST_CHILD_SIG=HUP"}, -1, syscall.SIGHUP}, + {"SIGKILL re-raised", []string{"SR_TEST_CHILD_SIG=KILL"}, -1, syscall.SIGKILL}, + {"other signal uses 128+n", []string{"SR_TEST_CHILD_SIG=USR1"}, 128 + int(syscall.SIGUSR1), 0}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + res := runHelper(t, false, append(c.env, "SR_TEST_HELPER=exit-as-child")...) + if res.code != c.wantCode || res.sig != c.wantSig { + t.Errorf("got code=%d sig=%v, want code=%d sig=%v\n%s", res.code, res.sig, c.wantCode, c.wantSig, res.out) + } + }) + } +} + +// exitFromSignal must not re-enable a signal that is ignored on entry: with +// SIGHUP ignored (nohup), it falls back to exit status 129. +func TestExitFromSignal_IgnoredFallsBack(t *testing.T) { + res := runHelper(t, true, "SR_TEST_HELPER=exit-from-signal", "SR_TEST_SIG=HUP") + if res.code != 129 || res.sig != 0 { + t.Errorf("got code=%d sig=%v, want code=129\n%s", res.code, res.sig, res.out) + } + + res = runHelper(t, false, "SR_TEST_HELPER=exit-from-signal", "SR_TEST_SIG=HUP") + if res.sig != syscall.SIGHUP { + t.Errorf("got code=%d sig=%v, want killed by SIGHUP\n%s", res.code, res.sig, res.out) + } +} diff --git a/internal/cli/run_signal.go b/internal/cli/run_signal.go index 258e418..fbe2658 100644 --- a/internal/cli/run_signal.go +++ b/internal/cli/run_signal.go @@ -4,6 +4,7 @@ package cli import ( "os" + "os/exec" "os/signal" "sync/atomic" "syscall" @@ -60,3 +61,32 @@ func RunSignalHandler(cfg SignalHandlerConfig) func() { close(sigCh) } } + +// childSignal returns the signal that terminated the child, or 0. +func childSignal(err *exec.ExitError) syscall.Signal { + if ws, ok := err.Sys().(syscall.WaitStatus); ok && ws.Signaled() { + return ws.Signal() + } + return 0 +} + +// exitAsChild ends the process the way the child ended: same exit code, or +// the same signal, so shells and systemd see the real cause. +func exitAsChild(err *exec.ExitError) { + if sig := childSignal(err); sig != 0 { + exitFromSignal(sig) + } + os.Exit(err.ExitCode()) +} + +// exitFromSignal re-raises sig on this process. Only stop signals and SIGKILL +// are re-raised; the Go runtime turns others (SIGSEGV, SIGQUIT, ...) into a +// crash dump. Falls back to the 128+n shell convention when sig is ignored. +func exitFromSignal(sig syscall.Signal) { + switch sig { + case syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP, syscall.SIGKILL: + syscall.Kill(os.Getpid(), sig) + time.Sleep(250 * time.Millisecond) + } + os.Exit(128 + int(sig)) +} From d6b654f0c5f3ec97b6988037e6874519642f831d Mon Sep 17 00:00:00 2001 From: contra Date: Mon, 7 Sep 2026 19:50:34 +0300 Subject: [PATCH 12/17] fix(run): a stop signal during session startup ends the session Signal handling started only after the child was running. A SIGTERM or Ctrl+C while the session was being created (up to ~60s: API call plus exit-IP detection) killed shellroute with the default action and left the API session open until the server reaper closed it. RunSignalHandler becomes SignalHandler, registered before session.Start: before Attach a signal cancels the session context and is remembered; Attach switches to forwarding, delivering a signal that arrived earlier to the child. Exit-IP detection now observes the context so the abort is prompt, and the interrupted startup ends the session, prints the summary, and exits by re-raising the signal. Signed-off-by: contra --- internal/cli/run.go | 63 +++--- internal/cli/run_signal.go | 131 +++++++---- internal/cli/run_signal_test.go | 276 ++++++++++++------------ internal/session/control.go | 6 +- internal/session/control_monitor.go | 2 +- internal/session/session.go | 23 +- internal/session/session_detect_test.go | 38 ++++ 7 files changed, 329 insertions(+), 210 deletions(-) create mode 100644 internal/session/session_detect_test.go diff --git a/internal/cli/run.go b/internal/cli/run.go index 34beba7..a7bff04 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -104,9 +104,13 @@ func runRun(cmd *cobra.Command, args []string) error { client := api.New(cfg.APIURL, cfg.APIKey) - // Start session + // Stop signals are handled from here on: during startup they abort the + // session, once the child runs they are forwarded to it. + ctx, cancel := context.WithCancel(context.Background()) + sigs := NewSignalHandler(cancel, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) + sess, err := session.Start( - context.Background(), + ctx, client, &api.SessionCreateRequest{ Country: country, @@ -118,10 +122,18 @@ func runRun(cmd *cobra.Command, args []string) error { session.StartOpts{TrackRelays: true, Mode: "run"}, ) if err != nil { + sigs.Stop() return handleSessionError(err) } + if sig := sigs.StartupSignal(); sig != 0 { + sigs.Stop() + endRunSession(sess) + exitFromSignal(sig) + } + if sess.GetExitIP() == "" { + sigs.Stop() sess.Stop() display.Error("Connection failed — no working upstream. Try again.") return fmt.Errorf("no exit IP") @@ -140,6 +152,7 @@ func runRun(cmd *cobra.Command, args []string) error { childCmd.Env = buildProxyEnv(os.Environ(), sess.ProxyURL()) if err := childCmd.Start(); err != nil { + sigs.Stop() sess.Stop() return fmt.Errorf("failed to start command: %w", err) } @@ -169,37 +182,19 @@ func runRun(cmd *cobra.Command, args []string) error { fmt.Fprintln(os.Stderr, "\n Connection lost during execution. Command killed.") } - // Handle SIGINT/SIGTERM/SIGHUP: forward to child, escalate if needed, - // then clean up the session. Ensures systemd stop, Ctrl+C, and - // terminal close all end the API session cleanly. - cleanupSig := RunSignalHandler(SignalHandlerConfig{ - Pid: -childCmd.Process.Pid, // negative = process group - ChildRunning: &childRunning, - KillCancel: killCancel, - }) + sigs.Attach(-childCmd.Process.Pid, &childRunning, killCancel) childErr := childCmd.Wait() childRunning.Store(false) close(killCancel) - cleanupSig() + sigs.Stop() - // Tear down session — always called, even after signal - resp, stopErr := sess.Stop() + // A signal death is reported by exiting the same way, not as a failure. exitErr, _ := childErr.(*exec.ExitError) - if !runNoStat { - // A signal death is reported by exiting the same way, not as a failure. - if childErr != nil && (exitErr == nil || childSignal(exitErr) == 0) { - display.Error("Command failed: %s", args[0]) - } - if stopErr != nil { - display.Error("Failed to end session. Try again.") - } else { - display.SessionSummary("shellroute session ended.", resp.DurationSec, resp.BytesTotal, resp.CostUSD, resp.BalanceUSD) - if resp.BalanceUSD <= 0.001 { - display.Warn("Balance depleted. Top up at https://console.shellroute.com, then reconnect.") - } - } + if childErr != nil && !runNoStat && (exitErr == nil || childSignal(exitErr) == 0) { + display.Error("Command failed: %s", args[0]) } + endRunSession(sess) if exitErr != nil { exitAsChild(exitErr) @@ -207,6 +202,22 @@ func runRun(cmd *cobra.Command, args []string) error { return childErr } +// endRunSession ends the session and prints the summary unless --no-stat. +func endRunSession(sess *session.Session) { + resp, err := sess.Stop() + if runNoStat { + return + } + if err != nil { + display.Error("Failed to end session. Try again.") + return + } + display.SessionSummary("shellroute session ended.", resp.DurationSec, resp.BytesTotal, resp.CostUSD, resp.BalanceUSD) + if resp.BalanceUSD <= 0.001 { + display.Warn("Balance depleted. Top up at https://console.shellroute.com, then reconnect.") + } +} + const defaultNoProxy = "localhost,127.0.0.1,::1" // buildProxyEnv creates the child environment with proxy vars, NO_PROXY bypass, diff --git a/internal/cli/run_signal.go b/internal/cli/run_signal.go index fbe2658..c3c4df5 100644 --- a/internal/cli/run_signal.go +++ b/internal/cli/run_signal.go @@ -3,65 +3,122 @@ package cli import ( + "context" "os" "os/exec" "os/signal" + "sync" "sync/atomic" "syscall" "time" ) -// SignalHandlerConfig holds the dependencies for the child signal handler. -type SignalHandlerConfig struct { - Pid int // child process PID (positive = process, negative = group) - ChildRunning *atomic.Bool // set to false when child exits - KillCancel chan struct{} // closed when child exits (cancels escalation) - EscalateAfter time.Duration // duration before SIGKILL escalation (default 5s) +// SignalHandler owns stop-signal handling for the life of one session. +// +// Before Attach, a stop signal cancels session startup. After Attach, the +// first signal is forwarded to the child process group and escalated to +// SIGKILL if the child does not exit in time; a second signal during that +// wait kills immediately. +type SignalHandler struct { + ch chan os.Signal + cancelStartup context.CancelFunc + escalateAfter time.Duration + + mu sync.Mutex + startupSig syscall.Signal // first signal before Attach, 0 if none + fired chan struct{} // closed with startupSig + attached bool + pid int + running *atomic.Bool + killCancel chan struct{} + stopOnce sync.Once } -// RunSignalHandler listens for SIGINT/SIGTERM/SIGHUP, forwards to the child -// process group, and escalates to SIGKILL if the child doesn't exit in time. -// A second signal during the escalation wait triggers immediate SIGKILL. -// Returns a cleanup function that must be called after the child exits. -func RunSignalHandler(cfg SignalHandlerConfig) func() { - if cfg.EscalateAfter == 0 { - cfg.EscalateAfter = 5 * time.Second +// NewSignalHandler registers for sigs immediately, so no window exists +// between creating the session and starting the child. +func NewSignalHandler(cancelStartup context.CancelFunc, sigs ...os.Signal) *SignalHandler { + h := &SignalHandler{ + ch: make(chan os.Signal, 2), // room for a second signal during escalation + cancelStartup: cancelStartup, + escalateAfter: 5 * time.Second, + fired: make(chan struct{}), } + signal.Notify(h.ch, sigs...) + go h.loop() + return h +} - sigCh := make(chan os.Signal, 2) // buffer 2 so second signal isn't lost - signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) - - go func() { - sig, ok := <-sigCh - if !ok || !cfg.ChildRunning.Load() { - return +func (h *SignalHandler) loop() { + for s := range h.ch { + sig := s.(syscall.Signal) + h.mu.Lock() + if !h.attached { + if h.startupSig == 0 { + h.startupSig = sig + close(h.fired) + h.cancelStartup() + } + h.mu.Unlock() + continue } + pid, running, killCancel := h.pid, h.running, h.killCancel + h.mu.Unlock() - // Forward first signal to child process group - syscall.Kill(cfg.Pid, sig.(syscall.Signal)) - - // Wait for child exit, escalation timeout, or second signal + if !running.Load() { + continue + } + syscall.Kill(pid, sig) select { - case <-time.After(cfg.EscalateAfter): - if cfg.ChildRunning.Load() { - syscall.Kill(cfg.Pid, syscall.SIGKILL) + case <-time.After(h.escalateAfter): + if running.Load() { + syscall.Kill(pid, syscall.SIGKILL) } - case <-sigCh: - // Second signal — immediate SIGKILL - if cfg.ChildRunning.Load() { - syscall.Kill(cfg.Pid, syscall.SIGKILL) + case <-h.ch: // second signal + if running.Load() { + syscall.Kill(pid, syscall.SIGKILL) } - case <-cfg.KillCancel: - // Child exited normally + case <-killCancel: } - }() + } +} - return func() { - signal.Stop(sigCh) - close(sigCh) +// StartupSignal returns the signal received before Attach, or 0. +func (h *SignalHandler) StartupSignal() syscall.Signal { + h.mu.Lock() + defer h.mu.Unlock() + return h.startupSig +} + +// Wait blocks until a signal arrives; for modes that run no child. +func (h *SignalHandler) Wait() syscall.Signal { + <-h.fired + return h.StartupSignal() +} + +// Attach starts forwarding signals to pid (negative = process group). +// A signal that arrived before Attach is forwarded now. +func (h *SignalHandler) Attach(pid int, running *atomic.Bool, killCancel chan struct{}) { + h.mu.Lock() + h.attached = true + h.pid, h.running, h.killCancel = pid, running, killCancel + sig := h.startupSig + h.mu.Unlock() + if sig != 0 { + select { + case h.ch <- sig: + default: // buffer full: the pending signals get forwarded instead + } } } +// Stop unregisters the handler; later signals get default handling. +func (h *SignalHandler) Stop() { + h.stopOnce.Do(func() { + signal.Stop(h.ch) + close(h.ch) + }) +} + // childSignal returns the signal that terminated the child, or 0. func childSignal(err *exec.ExitError) syscall.Signal { if ws, ok := err.Sys().(syscall.WaitStatus); ok && ws.Signaled() { diff --git a/internal/cli/run_signal_test.go b/internal/cli/run_signal_test.go index 7b9561f..fa47d2f 100644 --- a/internal/cli/run_signal_test.go +++ b/internal/cli/run_signal_test.go @@ -3,6 +3,9 @@ package cli import ( + "context" + "errors" + "os" "os/exec" "sync/atomic" "syscall" @@ -10,7 +13,8 @@ import ( "time" ) -// All tests call the production RunSignalHandler from run_signal.go. +// All tests drive the production SignalHandler. Signals are sent to the test +// process itself and reach the handler through signal.Notify, as in production. func startChild(t *testing.T, script string) *exec.Cmd { t.Helper() @@ -23,175 +27,175 @@ func startChild(t *testing.T, script string) *exec.Cmd { return cmd } -// TestHandler_ForwardSIGTERM: production handler forwards SIGTERM to child group. -func TestHandler_ForwardSIGTERM(t *testing.T) { - cmd := startChild(t, `trap 'exit 42' TERM; while true; do sleep 0.1; done`) +// childHandler is a running child with a handler attached to its process group. +type childHandler struct { + cmd *exec.Cmd + h *SignalHandler + running atomic.Bool + cancel chan struct{} + done chan error // child's Wait result +} - var running atomic.Bool - running.Store(true) - cancel := make(chan struct{}) +func newHandler(sigs ...syscall.Signal) (*SignalHandler, context.Context) { + ctx, cancel := context.WithCancel(context.Background()) + var osSigs []os.Signal + for _, s := range sigs { + osSigs = append(osSigs, s) + } + return NewSignalHandler(cancel, osSigs...), ctx +} - cleanup := RunSignalHandler(SignalHandlerConfig{ - Pid: -cmd.Process.Pid, - ChildRunning: &running, - KillCancel: cancel, - EscalateAfter: 5 * time.Second, - }) +func attachHandler(t *testing.T, script string, escalateAfter time.Duration) *childHandler { + t.Helper() + c := &childHandler{cmd: startChild(t, script), cancel: make(chan struct{}), done: make(chan error, 1)} + c.running.Store(true) + c.h, _ = newHandler(syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) + c.h.escalateAfter = escalateAfter + c.h.Attach(-c.cmd.Process.Pid, &c.running, c.cancel) + go func() { c.done <- c.cmd.Wait() }() + return c +} - // Simulate: OS sends SIGTERM to our process (via the handler's signal channel) - syscall.Kill(syscall.Getpid(), syscall.SIGTERM) +// wait returns the child's exit error once it is gone and the handler is stopped. +func (c *childHandler) wait(t *testing.T) error { + t.Helper() + var err error + select { + case err = <-c.done: + case <-time.After(5 * time.Second): + syscall.Kill(-c.cmd.Process.Pid, syscall.SIGKILL) + <-c.done + t.Error("child still running after 5s") + } + c.running.Store(false) + close(c.cancel) + c.h.Stop() + return err +} - err := cmd.Wait() - running.Store(false) - close(cancel) - cleanup() +func exitStatus(t *testing.T, err error) (code int, sig syscall.Signal) { + t.Helper() + if err == nil { + return 0, 0 + } + var ee *exec.ExitError + if !errors.As(err, &ee) { + t.Fatalf("unexpected error: %v", err) + } + ws := ee.Sys().(syscall.WaitStatus) + if ws.Signaled() { + return -1, ws.Signal() + } + return ws.ExitStatus(), 0 +} - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() != 42 { - t.Errorf("exit code = %d, want 42", exitErr.ExitCode()) - } - } else if err == nil { - t.Fatal("child should have exited from SIGTERM trap") +func TestHandler_ForwardSIGTERM(t *testing.T) { + c := attachHandler(t, `trap 'exit 42' TERM; while true; do sleep 0.1; done`, 5*time.Second) + syscall.Kill(syscall.Getpid(), syscall.SIGTERM) + if code, _ := exitStatus(t, c.wait(t)); code != 42 { + t.Errorf("exit code = %d, want 42", code) } } -// TestHandler_ForwardSIGHUP: production handler forwards SIGHUP. func TestHandler_ForwardSIGHUP(t *testing.T) { - cmd := startChild(t, `trap 'exit 43' HUP; while true; do sleep 0.1; done`) - - var running atomic.Bool - running.Store(true) - cancel := make(chan struct{}) - - cleanup := RunSignalHandler(SignalHandlerConfig{ - Pid: -cmd.Process.Pid, - ChildRunning: &running, - KillCancel: cancel, - EscalateAfter: 5 * time.Second, - }) - + c := attachHandler(t, `trap 'exit 43' HUP; while true; do sleep 0.1; done`, 5*time.Second) syscall.Kill(syscall.Getpid(), syscall.SIGHUP) - - err := cmd.Wait() - running.Store(false) - close(cancel) - cleanup() - - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() != 43 { - t.Errorf("exit code = %d, want 43", exitErr.ExitCode()) - } - } else if err == nil { - t.Fatal("child should have exited from SIGHUP trap") + if code, _ := exitStatus(t, c.wait(t)); code != 43 { + t.Errorf("exit code = %d, want 43", code) } } -// TestHandler_EscalateToKILL: child ignores SIGTERM, handler escalates to SIGKILL. +// Child ignores SIGTERM: handler escalates to SIGKILL after escalateAfter. func TestHandler_EscalateToKILL(t *testing.T) { - cmd := startChild(t, `trap '' TERM; while true; do sleep 0.1; done`) - - var running atomic.Bool - running.Store(true) - cancel := make(chan struct{}) - - cleanup := RunSignalHandler(SignalHandlerConfig{ - Pid: -cmd.Process.Pid, - ChildRunning: &running, - KillCancel: cancel, - EscalateAfter: 500 * time.Millisecond, // shortened for test - }) - + c := attachHandler(t, `trap '' TERM; while true; do sleep 0.1; done`, 500*time.Millisecond) syscall.Kill(syscall.Getpid(), syscall.SIGTERM) - - err := cmd.Wait() - running.Store(false) - close(cancel) - cleanup() - - if err == nil { - t.Fatal("child should have been killed") + if _, sig := exitStatus(t, c.wait(t)); sig != syscall.SIGKILL { + t.Errorf("signal = %v, want SIGKILL", sig) } } -// TestHandler_SecondSignalImmediateKILL: second signal during escalation wait -// triggers immediate SIGKILL instead of waiting for the timer. +// A second signal during the escalation wait kills immediately. func TestHandler_SecondSignalImmediateKILL(t *testing.T) { - cmd := startChild(t, `trap '' TERM INT; while true; do sleep 0.1; done`) - - var running atomic.Bool - running.Store(true) - cancel := make(chan struct{}) - - cleanup := RunSignalHandler(SignalHandlerConfig{ - Pid: -cmd.Process.Pid, - ChildRunning: &running, - KillCancel: cancel, - EscalateAfter: 30 * time.Second, // long timer — second signal should beat it - }) - + c := attachHandler(t, `trap '' TERM INT; while true; do sleep 0.1; done`, 30*time.Second) start := time.Now() syscall.Kill(syscall.Getpid(), syscall.SIGTERM) time.Sleep(200 * time.Millisecond) - syscall.Kill(syscall.Getpid(), syscall.SIGINT) // second signal - - err := cmd.Wait() - elapsed := time.Since(start) - running.Store(false) - close(cancel) - cleanup() - - if err == nil { - t.Fatal("child should have been killed") + syscall.Kill(syscall.Getpid(), syscall.SIGINT) + _, sig := exitStatus(t, c.wait(t)) + if sig != syscall.SIGKILL { + t.Errorf("signal = %v, want SIGKILL", sig) } - if elapsed > 5*time.Second { - t.Errorf("took %v — second signal should have triggered immediate SIGKILL", elapsed) + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Errorf("took %v — second signal should have killed immediately", elapsed) } } -// TestHandler_NoEscalateIfChildExits: child exits promptly, no SIGKILL. +// Child exits promptly: no SIGKILL, exit code preserved. func TestHandler_NoEscalateIfChildExits(t *testing.T) { - cmd := startChild(t, `trap 'exit 0' TERM; while true; do sleep 0.1; done`) - - var running atomic.Bool - running.Store(true) - cancel := make(chan struct{}) - - cleanup := RunSignalHandler(SignalHandlerConfig{ - Pid: -cmd.Process.Pid, - ChildRunning: &running, - KillCancel: cancel, - EscalateAfter: 5 * time.Second, - }) - + c := attachHandler(t, `trap 'exit 0' TERM; while true; do sleep 0.1; done`, 300*time.Millisecond) syscall.Kill(syscall.Getpid(), syscall.SIGTERM) + if code, sig := exitStatus(t, c.wait(t)); code != 0 || sig != 0 { + t.Errorf("exit = (%d, %v), want clean exit 0", code, sig) + } +} - cmd.Wait() - running.Store(false) - close(cancel) - cleanup() +// A signal after the child is gone must not touch its (possibly reused) pgid. +func TestHandler_SignalAfterChildExitIgnored(t *testing.T) { + c := attachHandler(t, `exit 0`, 5*time.Second) + <-c.done + c.running.Store(false) + syscall.Kill(syscall.Getpid(), syscall.SIGTERM) + time.Sleep(100 * time.Millisecond) // still registered: the handler must swallow it + close(c.cancel) + c.h.Stop() +} - // If we get here without hanging, escalation was cancelled +func TestHandler_StopWithoutSignal(t *testing.T) { + h, ctx := newHandler(syscall.SIGTERM) + h.Stop() + if ctx.Err() != nil { + t.Error("startup cancelled without a signal") + } } -// TestHandler_IgnoredAfterChildExit: handler doesn't panic on signal after child exits. -func TestHandler_IgnoredAfterChildExit(t *testing.T) { - cmd := startChild(t, `exit 0`) +// Before Attach, the first signal cancels startup and is remembered. +func TestHandler_StartupSignalCancels(t *testing.T) { + h, ctx := newHandler(syscall.SIGINT, syscall.SIGTERM) + defer h.Stop() - var running atomic.Bool - running.Store(true) - cancel := make(chan struct{}) + syscall.Kill(syscall.Getpid(), syscall.SIGTERM) + select { + case <-ctx.Done(): + case <-time.After(2 * time.Second): + t.Fatal("startup not cancelled after SIGTERM") + } + if sig := h.Wait(); sig != syscall.SIGTERM { + t.Errorf("Wait() = %v, want SIGTERM", sig) + } - cleanup := RunSignalHandler(SignalHandlerConfig{ - Pid: -cmd.Process.Pid, - ChildRunning: &running, - KillCancel: cancel, - EscalateAfter: 5 * time.Second, - }) + syscall.Kill(syscall.Getpid(), syscall.SIGINT) + time.Sleep(100 * time.Millisecond) + if sig := h.StartupSignal(); sig != syscall.SIGTERM { + t.Errorf("StartupSignal() = %v after second signal, want SIGTERM", sig) + } +} + +// A signal that arrived before Attach is forwarded to the child on Attach. +func TestHandler_AttachForwardsStartupSignal(t *testing.T) { + h, ctx := newHandler(syscall.SIGTERM) + syscall.Kill(syscall.Getpid(), syscall.SIGTERM) + select { + case <-ctx.Done(): + case <-time.After(2 * time.Second): + t.Fatal("startup not cancelled after SIGTERM") + } - cmd.Wait() - running.Store(false) - close(cancel) - cleanup() + c := &childHandler{h: h, cmd: startChild(t, `trap 'exit 42' TERM; while true; do sleep 0.1; done`), cancel: make(chan struct{}), done: make(chan error, 1)} + c.running.Store(true) + go func() { c.done <- c.cmd.Wait() }() + h.Attach(-c.cmd.Process.Pid, &c.running, c.cancel) - // No panic = pass + if code, _ := exitStatus(t, c.wait(t)); code != 42 { + t.Errorf("exit code = %d, want 42 (startup signal forwarded)", code) + } } diff --git a/internal/session/control.go b/internal/session/control.go index fd5fe61..1ef8ec1 100644 --- a/internal/session/control.go +++ b/internal/session/control.go @@ -162,7 +162,7 @@ func (c *Controller) autoRotateWithContext(ctx context.Context) { return } if sess.GetExitIP() == "" { - if ip := detectExitIPRetry(sess.Port, 15*time.Second); ip != "" { + if ip := detectExitIPRetry(ctx, sess.Port, 15*time.Second); ip != "" { sess.SetExitIP(ip) } } @@ -344,7 +344,7 @@ func (c *Controller) httpConnect(w http.ResponseWriter, r *http.Request) { if c.connectTimeout > 0 { retryTimeout = c.connectTimeout } - if ip := detectExitIPRetry(sess.Port, retryTimeout); ip != "" { + if ip := detectExitIPRetry(context.Background(), sess.Port, retryTimeout); ip != "" { sess.SetExitIP(ip) } } @@ -509,7 +509,7 @@ func (c *Controller) httpRotate(w http.ResponseWriter, r *http.Request) { sess := c.sess c.mu.Unlock() if sess != nil && sess.GetExitIP() == "" { - if ip := detectExitIPRetry(sess.Port, 20*time.Second); ip != "" { + if ip := detectExitIPRetry(context.Background(), sess.Port, 20*time.Second); ip != "" { sess.SetExitIP(ip) } } diff --git a/internal/session/control_monitor.go b/internal/session/control_monitor.go index 0d8d236..6d473ba 100644 --- a/internal/session/control_monitor.go +++ b/internal/session/control_monitor.go @@ -69,7 +69,7 @@ func (c *Controller) monitor(ctx context.Context) { c.mu.Unlock() // Re-detect exit IP — it may have changed after recovery - if ip := detectExitIP(sessPort); ip != "" { + if ip := detectExitIP(ctx, sessPort); ip != "" { c.mu.Lock() if c.sess != nil { c.sess.SetExitIP(ip) diff --git a/internal/session/session.go b/internal/session/session.go index 0bbb231..8abebe0 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -201,7 +201,7 @@ func startWithResponse(ctx context.Context, client *api.Client, resp *api.Sessio } connectStart := time.Now() if s.GetExitIP() == "" { - if ip := detectExitIPRetry(port, connectTimeout); ip != "" { + if ip := detectExitIPRetry(ctx, port, connectTimeout); ip != "" { s.SetExitIP(ip) } } @@ -354,14 +354,19 @@ func gatewayNeedsTLS(endpoint string) bool { return host != "localhost" && host != "127.0.0.1" && host != "::1" } -// detectExitIPRetry tries to detect exit IP with retries up to the given timeout. -func detectExitIPRetry(port int, timeout time.Duration) string { +// detectExitIPRetry tries to detect exit IP with retries up to the given +// timeout, or until ctx is cancelled. +func detectExitIPRetry(ctx context.Context, port int, timeout time.Duration) string { deadline := time.Now().Add(timeout) for time.Now().Before(deadline) { - if ip := detectExitIP(port); ip != "" { + if ip := detectExitIP(ctx, port); ip != "" { return ip } - time.Sleep(2 * time.Second) + select { + case <-ctx.Done(): + return "" + case <-time.After(2 * time.Second): + } } return "" } @@ -371,7 +376,7 @@ var ipDetectEndpoints = []string{ "https://api.ipify.org", } -func detectExitIP(port int) string { +func detectExitIP(ctx context.Context, port int) string { proxyURL := fmt.Sprintf("http://127.0.0.1:%d", port) transport := &http.Transport{ Proxy: func(*http.Request) (*neturl.URL, error) { @@ -384,7 +389,11 @@ func detectExitIP(port int) string { } for _, endpoint := range ipDetectEndpoints { - resp, err := client.Get(endpoint) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + if err != nil { + continue + } + resp, err := client.Do(req) if err != nil { continue } diff --git a/internal/session/session_detect_test.go b/internal/session/session_detect_test.go new file mode 100644 index 0000000..28df1f8 --- /dev/null +++ b/internal/session/session_detect_test.go @@ -0,0 +1,38 @@ +package session + +import ( + "context" + "testing" + "time" +) + +// Nothing listens on port 1, so detection can only end by timeout or cancel. + +func TestDetectExitIPRetry_CancelledBeforeStart(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + start := time.Now() + if ip := detectExitIPRetry(ctx, 1, 30*time.Second); ip != "" { + t.Fatalf("ip = %q, want empty", ip) + } + if d := time.Since(start); d > 2*time.Second { + t.Fatalf("took %v, want immediate return on cancelled context", d) + } +} + +func TestDetectExitIPRetry_CancelledWhileWaiting(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(300 * time.Millisecond) + cancel() + }() + + start := time.Now() + if ip := detectExitIPRetry(ctx, 1, 30*time.Second); ip != "" { + t.Fatalf("ip = %q, want empty", ip) + } + if d := time.Since(start); d > 3*time.Second { + t.Fatalf("took %v, want return shortly after cancel", d) + } +} From c6dda90befb27697ec82fd01bff6c8cd001d040a Mon Sep 17 00:00:00 2001 From: contra Date: Mon, 7 Sep 2026 19:52:44 +0300 Subject: [PATCH 13/17] fix(run): leave stop signals that are ignored on entry alone signal.Notify re-enables a signal that was ignored when the process started. Under nohup that defeated the hangup immunity: shellroute caught the SIGHUP, forwarded it to a child that had inherited the ignore, and escalated to SIGKILL five seconds later. On v0.1.3 both survived. The handler now skips any requested signal that signal.Ignored reports, so nohup keeps working and other stop signals stay handled. Signed-off-by: contra --- internal/cli/run_exit_test.go | 2 ++ internal/cli/run_signal.go | 14 ++++++-- internal/cli/run_signal_test.go | 58 +++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/internal/cli/run_exit_test.go b/internal/cli/run_exit_test.go index cea120f..b3b03da 100644 --- a/internal/cli/run_exit_test.go +++ b/internal/cli/run_exit_test.go @@ -47,6 +47,8 @@ func TestHelperProcess(t *testing.T) { os.Exit(0) case "exit-from-signal": exitFromSignal(signalByName(os.Getenv("SR_TEST_SIG"))) + case "ignored-hup": + helperIgnoredHUP() } } diff --git a/internal/cli/run_signal.go b/internal/cli/run_signal.go index c3c4df5..71a7bc8 100644 --- a/internal/cli/run_signal.go +++ b/internal/cli/run_signal.go @@ -35,7 +35,9 @@ type SignalHandler struct { } // NewSignalHandler registers for sigs immediately, so no window exists -// between creating the session and starting the child. +// between creating the session and starting the child. A signal ignored on +// entry (nohup, trap '' HUP) stays ignored: Notify would re-enable it and the +// child, which inherited the ignore, would end up SIGKILLed after the escalation. func NewSignalHandler(cancelStartup context.CancelFunc, sigs ...os.Signal) *SignalHandler { h := &SignalHandler{ ch: make(chan os.Signal, 2), // room for a second signal during escalation @@ -43,7 +45,15 @@ func NewSignalHandler(cancelStartup context.CancelFunc, sigs ...os.Signal) *Sign escalateAfter: 5 * time.Second, fired: make(chan struct{}), } - signal.Notify(h.ch, sigs...) + var wanted []os.Signal + for _, s := range sigs { + if !signal.Ignored(s) { + wanted = append(wanted, s) + } + } + if len(wanted) > 0 { // Notify with no signals would relay every signal + signal.Notify(h.ch, wanted...) + } go h.loop() return h } diff --git a/internal/cli/run_signal_test.go b/internal/cli/run_signal_test.go index fa47d2f..09802ff 100644 --- a/internal/cli/run_signal_test.go +++ b/internal/cli/run_signal_test.go @@ -5,8 +5,10 @@ package cli import ( "context" "errors" + "fmt" "os" "os/exec" + "os/signal" "sync/atomic" "syscall" "testing" @@ -199,3 +201,59 @@ func TestHandler_AttachForwardsStartupSignal(t *testing.T) { t.Errorf("exit code = %d, want 42 (startup signal forwarded)", code) } } + +// Under nohup (SIGHUP ignored on entry) the handler must leave SIGHUP alone: +// the child inherited the ignore, so a forwarded SIGHUP would only lead to +// SIGKILL. Other stop signals keep working. Runs via TestHelperProcess. +func TestHandler_LeavesIgnoredSignalsAlone(t *testing.T) { + res := runHelper(t, true, "SR_TEST_HELPER=ignored-hup") + if res.code != 0 || res.sig != 0 { + t.Errorf("helper: code=%d sig=%v\n%s", res.code, res.sig, res.out) + } +} + +func helperIgnoredHUP() { + fail := func(msg string) { + fmt.Println(msg) + os.Exit(1) + } + if !signal.Ignored(syscall.SIGHUP) { + fail("precondition: SIGHUP not ignored on entry") + } + cmd := exec.Command("bash", "-c", `while true; do sleep 0.1; done`) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + if err := cmd.Start(); err != nil { + fail(err.Error()) + } + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + + var running atomic.Bool + running.Store(true) + killCancel := make(chan struct{}) + _, cancel := context.WithCancel(context.Background()) + h := NewSignalHandler(cancel, syscall.SIGHUP, syscall.SIGTERM) + h.escalateAfter = 300 * time.Millisecond + h.Attach(-cmd.Process.Pid, &running, killCancel) + + if !signal.Ignored(syscall.SIGHUP) { + fail("handler re-enabled an ignored SIGHUP") + } + syscall.Kill(syscall.Getpid(), syscall.SIGHUP) // ignored: nothing may reach the child + select { + case err := <-done: + fail("child died after ignored SIGHUP: " + err.Error()) + case <-time.After(time.Second): + } + + syscall.Kill(syscall.Getpid(), syscall.SIGTERM) // still handled + select { + case <-done: + case <-time.After(3 * time.Second): + fail("child still running after SIGTERM") + } + running.Store(false) + close(killCancel) + h.Stop() + os.Exit(0) +} From 9ae0d12f86ee9a26ea47db86534f1d8d747ad81b Mon Sep 17 00:00:00 2001 From: contra Date: Mon, 7 Sep 2026 19:52:44 +0300 Subject: [PATCH 14/17] fix(proxy): a stop signal during session startup ends the session Same gap as in run mode: signals were only handled once the proxy was up, so a SIGTERM or Ctrl+C during session creation killed the process and left the API session open. The handler is now registered before session.Start; an interrupted startup ends the session and prints the summary like a normal disconnect. Signed-off-by: contra --- internal/cli/connect.go | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/internal/cli/connect.go b/internal/cli/connect.go index 9d6d90b..5abd740 100644 --- a/internal/cli/connect.go +++ b/internal/cli/connect.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "os" - "os/signal" "syscall" "time" @@ -127,8 +126,13 @@ func runConnectHeadless(cfg *config.Config, country string) error { client := api.New(cfg.APIURL, cfg.APIKey) + // Stop signals are handled from here on: during startup they abort the + // session, afterwards they end it. + ctx, cancel := context.WithCancel(context.Background()) + sigs := NewSignalHandler(cancel, syscall.SIGINT, syscall.SIGTERM) + sess, err := session.Start( - context.Background(), + ctx, client, &api.SessionCreateRequest{ Country: country, @@ -140,10 +144,16 @@ func runConnectHeadless(cfg *config.Config, country string) error { session.StartOpts{Mode: "proxy"}, ) if err != nil { + sigs.Stop() return handleSessionError(err) } + if sigs.StartupSignal() != 0 { + return waitAndDisconnect(sess, sigs) + } + if sess.GetExitIP() == "" { + sigs.Stop() sess.Stop() display.Error("Connection failed — no working upstream. Try again.") return fmt.Errorf("no exit IP") @@ -156,13 +166,13 @@ func runConnectHeadless(cfg *config.Config, country string) error { outputEnv(sess) } - return waitAndDisconnect(sess) + return waitAndDisconnect(sess, sigs) } -func waitAndDisconnect(sess *session.Session) error { - sigCh := make(chan os.Signal, 1) - signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) - <-sigCh +// waitAndDisconnect blocks until a stop signal, then ends the session. +func waitAndDisconnect(sess *session.Session, sigs *SignalHandler) error { + sigs.Wait() + sigs.Stop() fmt.Fprintln(os.Stderr) From f37908485f414a7cc7bfbd06c7a0d719574417ff Mon Sep 17 00:00:00 2001 From: contra Date: Mon, 7 Sep 2026 19:52:44 +0300 Subject: [PATCH 15/17] test: live signal test expects shellroute to die by SIGTERM like its child The child now dies from the forwarded SIGTERM instead of exiting 0, which is what real programs under systemd do, and shellroute must exit by SIGTERM (status 143 in bash) rather than 0 or 255. Signed-off-by: contra --- CONTRIBUTING.md | 2 +- scripts/test-run-signal-live.sh | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 23b12a5..e1cf5d7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -27,7 +27,7 @@ This runs: go vet, gofmt, build, unit tests (with race detector), public audit, ## Optional live test -`test-run-signal-live.sh` verifies that `shellroute run` forwards SIGTERM to the child process and ends the API session cleanly. It creates one real paid session and requires `--live`: +`test-run-signal-live.sh` verifies that `shellroute run` forwards SIGTERM to the child process, ends the API session cleanly, and then exits by SIGTERM the way the child did. It creates one real paid session and requires `--live`: ```bash ./scripts/test-run-signal-live.sh --live [COUNTRY] diff --git a/scripts/test-run-signal-live.sh b/scripts/test-run-signal-live.sh index 95931f2..e955940 100755 --- a/scripts/test-run-signal-live.sh +++ b/scripts/test-run-signal-live.sh @@ -2,7 +2,9 @@ set -uo pipefail # Live signal-handling test for shellroute run. -# Creates one real (paid) session to verify SIGTERM forwarding and clean shutdown. +# Creates one real (paid) session to verify that SIGTERM is forwarded to the +# child, that shellroute then exits by SIGTERM like the child (what systemd +# stop and shells expect), and that the session ends cleanly. # # Usage: # ./scripts/test-run-signal-live.sh --live [COUNTRY] @@ -117,12 +119,12 @@ echo "Auth: ok" echo "" echo "=== Running shellroute run $COUNTRY with signal test ===" -# --- Child script: writes PGID, signals readiness, traps SIGTERM, exits 0 --- +# --- Child script: writes PGID, signals readiness, reports SIGTERM, then dies by it --- CHILD_SCRIPT=' pgid_file="$1" ready_file="$2" echo $$ > "$pgid_file" -trap '"'"'echo CHILD_GOT_SIGTERM >&2; exit 0'"'"' TERM +trap '"'"'echo CHILD_GOT_SIGTERM >&2; trap - TERM; kill -TERM $$'"'"' TERM touch "$ready_file" while true; do sleep 0.1; done ' @@ -194,12 +196,12 @@ show_output() { echo "" echo "=== Verification ===" -# 1. Shellroute exited 0 (child exited 0 from trap) -if [ "$SR_EXIT" -eq 0 ]; then - echo " PASS: shellroute exited 0" +# 1. Shellroute died by SIGTERM like its child (bash reports 128+15) +if [ "$SR_EXIT" -eq 143 ]; then + echo " PASS: shellroute exited by SIGTERM (143), mirroring the child" PASS=$((PASS + 1)) else - echo " FAIL: shellroute exited $SR_EXIT, want 0" + echo " FAIL: shellroute exit status $SR_EXIT, want 143 (killed by SIGTERM)" FAIL=$((FAIL + 1)) show_output fi @@ -253,4 +255,4 @@ if [ $FAIL -gt 0 ]; then fi echo "" -echo "Signal handling verified: SIGTERM → child forwarded → session ended cleanly." +echo "Signal handling verified: SIGTERM → forwarded to child → session ended → shellroute exited by SIGTERM." From 0cf632906f360d0d29935a18a40995c3f741b18e Mon Sep 17 00:00:00 2001 From: contra Date: Mon, 7 Sep 2026 19:53:07 +0300 Subject: [PATCH 16/17] style: gofmt run_signal.go Signed-off-by: contra --- internal/cli/run_signal.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cli/run_signal.go b/internal/cli/run_signal.go index 71a7bc8..4f15335 100644 --- a/internal/cli/run_signal.go +++ b/internal/cli/run_signal.go @@ -36,7 +36,7 @@ type SignalHandler struct { // NewSignalHandler registers for sigs immediately, so no window exists // between creating the session and starting the child. A signal ignored on -// entry (nohup, trap '' HUP) stays ignored: Notify would re-enable it and the +// entry (nohup, trap ” HUP) stays ignored: Notify would re-enable it and the // child, which inherited the ignore, would end up SIGKILLed after the escalation. func NewSignalHandler(cancelStartup context.CancelFunc, sigs ...os.Signal) *SignalHandler { h := &SignalHandler{ From ca33212b1f54cfac065737b671278fed8117ab68 Mon Sep 17 00:00:00 2001 From: contra Date: Mon, 7 Sep 2026 19:53:47 +0300 Subject: [PATCH 17/17] style: reword ignored-signal comment so gofmt keeps it literal Signed-off-by: contra --- internal/cli/run_signal.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/cli/run_signal.go b/internal/cli/run_signal.go index 4f15335..d05f300 100644 --- a/internal/cli/run_signal.go +++ b/internal/cli/run_signal.go @@ -36,8 +36,9 @@ type SignalHandler struct { // NewSignalHandler registers for sigs immediately, so no window exists // between creating the session and starting the child. A signal ignored on -// entry (nohup, trap ” HUP) stays ignored: Notify would re-enable it and the -// child, which inherited the ignore, would end up SIGKILLed after the escalation. +// entry (nohup, a shell trap with an empty action) stays ignored: Notify would +// re-enable it, and the child, which inherited the ignore, would end up +// SIGKILLed after the escalation. func NewSignalHandler(cancelStartup context.CancelFunc, sigs ...os.Signal) *SignalHandler { h := &SignalHandler{ ch: make(chan os.Signal, 2), // room for a second signal during escalation