[release/0.25] fix: reset inherited signal dispositions for forked processes on macOS

internal/dcp/commands/root.go

@@ -93,6 +93,12 @@ func NewRootCmd(log *logger.Logger) (*cobra.Command, error) {
 		rootCmd.AddCommand(cmd)
 	}
 
+	if cmd, err = dcpproc_cmds.NewForkProcessExecCommand(log.Logger); err != nil {
+		return nil, fmt.Errorf("could not set up '%s' command: %w", dcpproc_cmds.ForkProcessExecCmdName, err)
+	} else {
+		rootCmd.AddCommand(cmd)
+	}
+
 	// Add dcptun sub-commands
 	rootCmd.AddCommand(dcptun_cmds.NewRunServerCommand(log.Logger))
 

internal/dcpproc/commands/fork_process.go

@@ -8,8 +8,12 @@ package commands
 import (
 	"context"
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
+	"strconv"
+	"strings"
+	"syscall"
 
 	"github.com/go-logr/logr"
 	"github.com/spf13/cobra"
@@ -57,6 +61,14 @@ func forkProcess(log logr.Logger) func(cmd *cobra.Command, args []string) error
 		logger.WithSessionId(childCmd)
 		process.ForkFromParent(childCmd)
 
+		execShim, shimErr := useExecShim(childCmd)
+		if shimErr != nil {
+			return shimErr
+		}
+		if execShim != nil {
+			defer execShim.close()
+		}
+
 		monitorEnabled := cmd.Flags().Changed("monitor")
 		var monitorCtx context.Context
 		var monitorCtxCancel context.CancelFunc
@@ -71,7 +83,7 @@ func forkProcess(log logr.Logger) func(cmd *cobra.Command, args []string) error
 			}
 		}
 
-		pid, childExitInfoCh, disposeChildExecutor, startErr := startForkedProcess(cmd, childCmd, monitorEnabled, log)
+		pid, childExitInfoCh, disposeChildExecutor, startErr := startForkedProcess(cmd, childCmd, execShim, monitorEnabled, log)
 		if startErr != nil {
 			return startErr
 		}
@@ -115,6 +127,7 @@ func forkProcess(log logr.Logger) func(cmd *cobra.Command, args []string) error
 func startForkedProcess(
 	cmd *cobra.Command,
 	childCmd *exec.Cmd,
+	execShim *execShimHandshake,
 	observeExit bool,
 	log logr.Logger,
 ) (process.Pid_t, <-chan process.ProcessExitInfo, func(), error) {
@@ -141,6 +154,18 @@ func startForkedProcess(
 		return process.UnknownPID, nil, nil, fmt.Errorf("could not start forked process: %w", startErr)
 	}
 
+	// Starting the shim only means dcp itself started. The PID must not be reported before the
+	// requested program is known to be running, so that a program which cannot be executed is
+	// still reported as a start failure.
+	if execShim != nil {
+		if execErr := execShim.wait(); execErr != nil {
+			// The logger already carries the command and arguments.
+			log.Error(execErr, "Failed to execute forked process")
+			executor.Dispose()
+			return process.UnknownPID, nil, nil, fmt.Errorf("could not start forked process: %w", execErr)
+		}
+	}
+
 	pid := handle.Pid
 	if _, writeErr := fmt.Fprintln(cmd.OutOrStdout(), pid); writeErr != nil {
 		log.Error(writeErr, "Failed to write forked process PID", "PID", pid)
@@ -163,3 +188,97 @@ func trimForkProcessArgSeparator(args []string) []string {
 
 	return args
 }
+
+// Redirects the child through the 'fork-process-exec' command on platforms where the child would
+// otherwise inherit the Go runtime's signal handler flags. The shim clears those flags and then
+// execs the original program, which keeps the process ID, session, standard streams, and exit
+// code that the caller of 'fork-process' expects.
+//
+// The reset cannot be done here: the Go runtime restores its own signal dispositions in the
+// forked child before it reaches execve, so it has to happen in the process that calls exec.
+//
+// Returns the handshake that reports whether the shim reached the requested program, or nil when
+// the child is started directly. The caller owns the returned handshake and must close it.
+func useExecShim(childCmd *exec.Cmd) (*execShimHandshake, error) {
+	if !process.SignalDispositionsLeakToChildren() {
+		return nil, nil
+	}
+
+	if childCmd.Err != nil {
+		// The program could not be located. Leave the command untouched so that starting it
+		// reports that original failure rather than one from the shim.
+		return nil, nil
+	}
+
+	dcpPath, dcpPathErr := os.Executable()
+	if dcpPathErr != nil {
+		return nil, fmt.Errorf("could not determine the path of the current executable: %w", dcpPathErr)
+	}
+
+	statusR, statusW, pipeErr := os.Pipe()
+	if pipeErr != nil {
+		return nil, fmt.Errorf("could not create the exec status pipe: %w", pipeErr)
+	}
+
+	shimArgs := []string{dcpPath, ForkProcessExecCmdName, "--" + execPathFlagName, childCmd.Path, "--"}
+	childCmd.Args = append(shimArgs, childCmd.Args...)
+	childCmd.Path = dcpPath
+
+	// The shim reports the outcome of the exec on this descriptor. It is the only extra file, so
+	// the shim sees it as execStatusFd.
+	childCmd.ExtraFiles = append(childCmd.ExtraFiles, statusW)
+
+	return &execShimHandshake{statusR: statusR, statusW: statusW}, nil
+}
+
+// execShimHandshake reports whether the shim managed to exec the requested program. Starting the
+// shim only proves that dcp itself could be started, so without this the caller would be told
+// that a program which never ran had started successfully.
+//
+// The shim inherits the write end. A successful execve closes it and the read end reports EOF,
+// while a failure sends the errno before the shim exits.
+type execShimHandshake struct {
+	statusR *os.File
+	statusW *os.File
+}
+
+// wait blocks until the shim either replaces itself with the requested program or reports why it
+// could not. It returns the failure that a direct start would have reported.
+func (h *execShimHandshake) wait() error {
+	// The write end is now owned by the shim. The parent's copy has to go, because the read below
+	// only reports EOF once every writer is closed.
+	h.closeWriteEnd()
+
+	status, readErr := io.ReadAll(h.statusR)
+	if readErr != nil {
+		return fmt.Errorf("could not read the exec status: %w", readErr)
+	}
+
+	if len(status) == 0 {
+		// EOF with nothing written: the descriptor was closed by a successful execve.
+		return nil
+	}
+
+	errnoValue, parseErr := strconv.Atoi(strings.TrimSpace(string(status)))
+	if parseErr != nil {
+		return fmt.Errorf("the exec status %q could not be parsed: %w", status, parseErr)
+	}
+
+	return syscall.Errno(errnoValue)
+}
+
+func (h *execShimHandshake) closeWriteEnd() {
+	if h.statusW != nil {
+		_ = h.statusW.Close()
+		h.statusW = nil
+	}
+}
+
+func (h *execShimHandshake) close() {
+	h.closeWriteEnd()
+
+	if h.statusR != nil {
+		_ = h.statusR.Close()
+		h.statusR = nil
+	}
+}

internal/dcpproc/commands/fork_process_exec.go

@@ -0,0 +1,109 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See LICENSE in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+package commands
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"syscall"
+
+	"github.com/go-logr/logr"
+	"github.com/spf13/cobra"
+
+	cmds "github.com/microsoft/dcp/internal/commands"
+	"github.com/microsoft/dcp/pkg/process"
+)
+
+const (
+	// The name of the command, also used when 'fork-process' builds an invocation of it.
+	ForkProcessExecCmdName = "fork-process-exec"
+
+	// The flag carrying the resolved path of the image to execute. It is passed separately from
+	// the arguments so that the child keeps the argv[0] the caller asked for.
+	execPathFlagName = "exec-path"
+
+	// The descriptor 'fork-process' passes as the only extra file, on which this command reports
+	// whether the exec succeeded. It is the first descriptor after the standard streams.
+	execStatusFd = 3
+
+	// Reported when the image cannot be executed, matching the shell convention for a command
+	// that could not be run. 'fork-process' reports the underlying errno itself, so this is only
+	// a fallback for anything that inspects the shim's own exit code.
+	execFailedExitCode = 127
+)
+
+var execPath string
+
+// NewForkProcessExecCommand creates the 'fork-process-exec' command, which replaces itself with
+// the requested image after clearing the signal dispositions inherited from the Go runtime.
+// It is an implementation detail of 'fork-process' and is not meant to be invoked directly.
+func NewForkProcessExecCommand(log logr.Logger) (*cobra.Command, error) {
+	forkProcessExecCmd := &cobra.Command{
+		Use:   ForkProcessExecCmdName + " --" + execPathFlagName + " path -- command [args...]",
+		Short: "Replaces this process with another program.",
+		Long:  "Clears the signal dispositions this process inherited from the Go runtime and then replaces it with the requested program, keeping the same process ID. Used internally by 'fork-process' so that children do not inherit signal handler flags that confuse other language runtimes.",
+		RunE:  forkProcessExec(log),
+		Args:  validateForkProcessExecArgs,
+
+		Hidden:       true,
+		SilenceUsage: true,
+	}
+
+	forkProcessExecCmd.Flags().StringVar(&execPath, execPathFlagName, "", "Resolved path of the program to execute")
+
+	return forkProcessExecCmd, nil
+}
+
+func validateForkProcessExecArgs(_ *cobra.Command, args []string) error {
+	if len(trimForkProcessArgSeparator(args)) == 0 {
+		return fmt.Errorf("command is required")
+	}
+
+	return nil
+}
+
+func forkProcessExec(log logr.Logger) func(cmd *cobra.Command, args []string) error {
+	return func(_ *cobra.Command, args []string) error {
+		args = trimForkProcessArgSeparator(args)
+
+		if execPath == "" {
+			return fmt.Errorf("--%s is required", execPathFlagName)
+		}
+
+		log = log.WithName("ForkProcessExec").WithValues(
+			"Path", execPath,
+			"Args", args[1:],
+		)
+
+		// 'fork-process' waits for this descriptor to close, which is how a successful execve is
+		// reported, so it must not survive into the new program. It is always supplied, because
+		// this command is only ever started by 'fork-process'.
+		statusFile := os.NewFile(execStatusFd, "exec-status")
+		syscall.CloseOnExec(execStatusFd)
+
+		// From this point on the process must not rely on the Go runtime's signal handling,
+		// which the reset disables. The only remaining step is the exec.
+		process.ResetSignalDispositions()
+
+		// Exec only returns when it fails; on success this process becomes the requested program.
+		execErr := syscall.Exec(execPath, args, os.Environ())
+
+		var execErrno syscall.Errno
+		if !errors.As(execErr, &execErrno) {
+			// Report something the parent can still parse; the message below stays accurate.
+			execErrno = syscall.EINVAL
+		}
+
+		_, _ = fmt.Fprintf(statusFile, "%d", int(execErrno))
+		_ = statusFile.Close()
+
+		exitCode := execFailedExitCode
+
+		log.Error(execErr, "Could not execute the requested program")
+		return cmds.NewExitCodeError(fmt.Errorf("could not execute %q: %w", execPath, execErr), exitCode)
+	}
+}