Skip to content

Commit 54315a7

Browse files
committed
feat(pass): forward signals from pass run to the child
Replace `child.Run()` with explicit `Start`/`Wait` bracketed by a signal forwarder. SIGINT, SIGTERM, and SIGHUP delivered to `pass run` are now relayed to the child instead of killing both processes through the foreground process group. The wrapper waits for the child to exit on its own terms before returning, so trapped cleanup handlers in the child run to completion. When the child is killed by a signal, the exit code reported by `pass run` is now `128 + signum` instead of `-1`, matching shell convention. Platform split for the signal set and exit-code translation: SIGINT/SIGTERM/SIGHUP on Unix, SIGINT only on Windows.
1 parent 3be0e98 commit 54315a7

4 files changed

Lines changed: 164 additions & 5 deletions

File tree

plugins/pass/commands/run.go

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"fmt"
2121
"os"
2222
"os/exec"
23+
"os/signal"
2324
"strings"
2425

2526
"github.com/spf13/cobra"
@@ -67,18 +68,43 @@ started and exits non-zero.`,
6768
return err
6869
}
6970

70-
child := exec.CommandContext(cmd.Context(), args[0], args[1:]...)
71+
// No CommandContext: the signal forwarder owns the child's
72+
// lifecycle. Tying the child to cmd.Context() would let cobra's
73+
// ctx cancellation SIGKILL the child out from under the forwarder.
74+
child := exec.Command(args[0], args[1:]...)
7175
child.Env = env
7276
child.Stdin = os.Stdin
7377
child.Stdout = os.Stdout
7478
child.Stderr = os.Stderr
7579

76-
if err := child.Run(); err != nil {
80+
if err := child.Start(); err != nil {
81+
return err
82+
}
83+
84+
sigCh := make(chan os.Signal, 1)
85+
signal.Notify(sigCh, forwardableSignals()...)
86+
done := make(chan struct{})
87+
go func() {
88+
for {
89+
select {
90+
case sig := <-sigCh:
91+
_ = child.Process.Signal(sig)
92+
case <-done:
93+
return
94+
}
95+
}
96+
}()
97+
98+
waitErr := child.Wait()
99+
signal.Stop(sigCh)
100+
close(done)
101+
102+
if waitErr != nil {
77103
var exitErr *exec.ExitError
78-
if errors.As(err, &exitErr) {
79-
os.Exit(exitErr.ExitCode())
104+
if errors.As(waitErr, &exitErr) {
105+
os.Exit(childExitCode(exitErr.ProcessState))
80106
}
81-
return err
107+
return waitErr
82108
}
83109
return nil
84110
},

plugins/pass/commands/run_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,18 @@
1515
package commands
1616

1717
import (
18+
"bufio"
1819
"context"
1920
"errors"
21+
"fmt"
22+
"io"
2023
"os"
2124
"os/exec"
25+
"runtime"
2226
"strconv"
27+
"syscall"
2328
"testing"
29+
"time"
2430

2531
"github.com/stretchr/testify/assert"
2632
"github.com/stretchr/testify/require"
@@ -38,6 +44,7 @@ const (
3844
helperWrapperEnv = "GO_PASS_RUN_WRAPPER"
3945
helperActiveEnv = "GO_PASS_RUN_HELPER_ACTIVE"
4046
helperExitEnv = "GO_PASS_RUN_HELPER_EXIT"
47+
helperSleepEnv = "GO_PASS_RUN_HELPER_SLEEP"
4148
)
4249

4350
func TestMain(m *testing.M) {
@@ -48,6 +55,13 @@ func TestMain(m *testing.M) {
4855
return // unreachable; runAsWrapper exits
4956
}
5057
if os.Getenv(helperActiveEnv) != "" {
58+
if os.Getenv(helperSleepEnv) != "" {
59+
// Signal-handling test: announce readiness, then block until a
60+
// signal kills us with its default disposition (so the wrapper
61+
// observes a signaled exit, not a normal one).
62+
_, _ = fmt.Fprintln(os.Stderr, "READY")
63+
select {}
64+
}
5165
code := 0
5266
if v := os.Getenv(helperExitEnv); v != "" {
5367
if n, err := strconv.Atoi(v); err == nil {
@@ -192,6 +206,54 @@ func TestRunCommand(t *testing.T) {
192206
require.True(t, errors.As(err, &exitErr), "expected ExitError, got %v", err)
193207
assert.Equal(t, 42, exitErr.ExitCode())
194208
})
209+
210+
t.Run("forwards SIGINT and exits 130", func(t *testing.T) {
211+
if runtime.GOOS == "windows" {
212+
t.Skip("SIGINT cross-process semantics differ on Windows")
213+
}
214+
215+
sub := exec.CommandContext(t.Context(), exe)
216+
sub.Env = append(os.Environ(),
217+
helperWrapperEnv+"=1",
218+
helperActiveEnv+"=1",
219+
helperSleepEnv+"=1",
220+
)
221+
stderr, err := sub.StderrPipe()
222+
require.NoError(t, err)
223+
require.NoError(t, sub.Start())
224+
225+
waitForReady(t, stderr)
226+
227+
require.NoError(t, sub.Process.Signal(syscall.SIGINT))
228+
229+
err = sub.Wait()
230+
var exitErr *exec.ExitError
231+
require.True(t, errors.As(err, &exitErr), "expected ExitError, got %v", err)
232+
assert.Equal(t, 130, exitErr.ExitCode())
233+
})
234+
}
235+
236+
func waitForReady(t *testing.T, r io.Reader) {
237+
t.Helper()
238+
scanner := bufio.NewScanner(r)
239+
done := make(chan bool, 1)
240+
go func() {
241+
for scanner.Scan() {
242+
if scanner.Text() == "READY" {
243+
done <- true
244+
return
245+
}
246+
}
247+
done <- false
248+
}()
249+
select {
250+
case ok := <-done:
251+
require.True(t, ok, "subprocess closed stderr before printing READY")
252+
case <-time.After(5 * time.Second):
253+
t.Fatal("timed out waiting for READY from subprocess")
254+
}
255+
// Drain remaining stderr in the background so the pipe never blocks.
256+
go func() { _, _ = io.Copy(io.Discard, r) }()
195257
}
196258

197259
// testWriter forwards cobra output to t.Log so it does not leak onto stderr.

plugins/pass/commands/run_unix.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright 2025-2026 Docker, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
//go:build !windows
16+
17+
package commands
18+
19+
import (
20+
"os"
21+
"syscall"
22+
)
23+
24+
func forwardableSignals() []os.Signal {
25+
return []os.Signal{syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP}
26+
}
27+
28+
func childExitCode(state *os.ProcessState) int {
29+
if state.Exited() {
30+
return state.ExitCode()
31+
}
32+
if ws, ok := state.Sys().(syscall.WaitStatus); ok && ws.Signaled() {
33+
return 128 + int(ws.Signal())
34+
}
35+
return state.ExitCode()
36+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright 2025-2026 Docker, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
//go:build windows
16+
17+
package commands
18+
19+
import (
20+
"os"
21+
"syscall"
22+
)
23+
24+
// On Windows only SIGINT can be sent to another process via os.Process.Signal;
25+
// SIGTERM and SIGHUP are accepted by signal.Notify but cannot be forwarded to
26+
// a child process.
27+
func forwardableSignals() []os.Signal {
28+
return []os.Signal{syscall.SIGINT}
29+
}
30+
31+
// Windows does not surface a signaled-process state through ProcessState;
32+
// ExitCode() is authoritative for both normal and abnormal termination.
33+
func childExitCode(state *os.ProcessState) int {
34+
return state.ExitCode()
35+
}

0 commit comments

Comments
 (0)