fix: Fixing signal propagation issues (#5326)

* fix: Inproper signal handling

* fix: Test

* fix: bot recommendations

* include contribution from sylr

* Switch to cmd.Cancel func override

* Fix the mighty coderabbit comments

* Fix sonar

* Fix coderabbit

* fix: Ensuring that signals properly propagate from users

---------

Co-authored-by: Moritz Wirth <mw@flanga.io>

Author: yhakbar

cli/commands/hcl/format/format.go

@@ -18,6 +18,7 @@ import (
 	"github.com/gruntwork-io/terragrunt/config"
 	"github.com/gruntwork-io/terragrunt/internal/component"
 	"github.com/gruntwork-io/terragrunt/internal/filter"
+	"github.com/gruntwork-io/terragrunt/internal/os/signal"
 	"github.com/gruntwork-io/terragrunt/pkg/log"
 	"github.com/gruntwork-io/terragrunt/pkg/log/writer"
 	"golang.org/x/exp/slices"
@@ -289,7 +290,33 @@ func bytesDiff(ctx context.Context, l log.Logger, b1, b2 []byte, path string) ([
 		return nil, err
 	}
 
-	data, err := exec.CommandContext(ctx, "diff", "--label="+filepath.Join("old", path), "--label="+filepath.Join("new/", path), "-u", f1.Name(), f2.Name()).CombinedOutput()
+	diffPath, err := exec.LookPath("diff")
+	if err != nil {
+		return nil, fmt.Errorf("failed to find diff command in PATH: %w", err)
+	}
+
+	cmd := exec.CommandContext(
+		ctx,
+		diffPath,
+		"--label="+filepath.Join("old", path),
+		"--label="+filepath.Join("new/", path),
+		"-u",
+		f1.Name(),
+		f2.Name(),
+	)
+	cmd.Cancel = func() error {
+		if cmd.Process == nil {
+			return nil
+		}
+
+		if sig := signal.SignalFromContext(ctx); sig != nil {
+			return cmd.Process.Signal(sig)
+		}
+
+		return cmd.Process.Signal(os.Kill)
+	}
+
+	data, err := cmd.CombinedOutput()
 	if len(data) > 0 {
 		// diff exits with a non-zero status when the files don't match.
 		// Ignore that failure as long as we get output.

engine/engine.go

@@ -20,6 +20,7 @@ import (
 	"github.com/gruntwork-io/terragrunt/internal/cache"
 	"github.com/gruntwork-io/terragrunt/internal/experiment"
 	"github.com/gruntwork-io/terragrunt/internal/github"
+	"github.com/gruntwork-io/terragrunt/internal/os/signal"
 
 	"github.com/hashicorp/go-hclog"
 
@@ -515,7 +516,17 @@ func createEngine(ctx context.Context, l log.Logger, terragruntOptions *options.
 		context.WithoutCancel(ctx),
 		localEnginePath,
 	)
+	cmd.Cancel = func() error {
+		if cmd.Process == nil {
+			return nil
+		}
+
+		if sig := signal.SignalFromContext(ctx); sig != nil {
+			return cmd.Process.Signal(sig)
+		}
 
+		return cmd.Process.Signal(os.Kill)
+	}
 	// pass log level to engine
 	cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", engineLogLevelEnv, engineLogLevel))
 	client := plugin.NewClient(&plugin.ClientConfig{
@@ -758,7 +769,6 @@ func ReadEngineOutput(runOptions *ExecutionOptions, forceStdErr bool, output out
 
 	for {
 		response, err := output()
-
 		if err != nil && (errors.Is(err, ErrEngineInitFailed) || errors.Is(err, ErrEngineShutdownFailed)) {
 			return err
 		}

internal/git/git.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/go-git/go-git/v6"
 	"github.com/go-git/go-git/v6/storage/filesystem"
+	"github.com/gruntwork-io/terragrunt/internal/os/signal"
 	"github.com/gruntwork-io/terragrunt/pkg/log"
 )
 
@@ -646,6 +647,17 @@ func (g *GitRunner) SetRemoteHeadAuto(ctx context.Context) error {
 
 func (g *GitRunner) prepareCommand(ctx context.Context, name string, args ...string) *exec.Cmd {
 	cmd := exec.CommandContext(ctx, g.GitPath, append([]string{name}, args...)...)
+	cmd.Cancel = func() error {
+		if cmd.Process == nil {
+			return nil
+		}
+
+		if sig := signal.SignalFromContext(ctx); sig != nil {
+			return cmd.Process.Signal(sig)
+		}
+
+		return cmd.Process.Signal(os.Kill)
+	}
 
 	if g.WorkDir != "" {
 		cmd.Dir = g.WorkDir

internal/os/exec/cmd.go

@@ -39,6 +39,17 @@ func Command(ctx context.Context, name string, args ...string) *Cmd {
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
+	cmd.Cancel = func() error {
+		if cmd.Process == nil {
+			return nil
+		}
+
+		if sig := signal.SignalFromContext(ctx); sig != nil {
+			return cmd.Process.Signal(sig)
+		}
+
+		return cmd.Process.Signal(os.Kill)
+	}
 
 	return cmd
 }

internal/os/signal/context_canceled.go

@@ -2,6 +2,7 @@ package signal
 
 import (
 	"context"
+	"errors"
 	"os"
 )
 
@@ -10,6 +11,22 @@ type ContextCanceledError struct {
 	Signal os.Signal
 }
 
+// SignalFromContext extracts the signal that caused the context cancellation, if any.
+// Returns nil if the context was not cancelled due to a signal.
+func SignalFromContext(ctx context.Context) os.Signal {
+	cause := context.Cause(ctx)
+	if cause == nil {
+		return nil
+	}
+
+	var canceledErr *ContextCanceledError
+	if errors.As(cause, &canceledErr) && canceledErr.Signal != nil {
+		return canceledErr.Signal
+	}
+
+	return nil
+}
+
 // NewContextCanceledError returns a new `ContextCanceledError` instance.
 func NewContextCanceledError(sig os.Signal) *ContextCanceledError {
 	return &ContextCanceledError{Signal: sig}

shell/run_cmd_unix_test.go

@@ -51,8 +51,10 @@ func TestRunCommandWithOutputInterrupt(t *testing.T) {
 	expectedExitStatusErr := fmt.Sprintf("Failed to execute \"%s 5\" in .\n\nexit status %d", cmdPath, expectedWait)
 	expectedKilledErr := fmt.Sprintf("Failed to execute \"%s 5\" in .\n\nsignal: killed", cmdPath)
 
-	if actualErr.Error() != expectedExitStatusErr && actualErr.Error() != expectedKilledErr {
-		t.Errorf("Expected error to be either:\n  %s\nor:\n  %s\nbut got:\n  %s",
-			expectedExitStatusErr, expectedKilledErr, actualErr.Error())
+	if actualErr.Error() == expectedKilledErr {
+		t.Errorf("Expected process to gracefully terminate but got\n: %s", actualErr.Error())
+	} else if actualErr.Error() != expectedExitStatusErr {
+		t.Errorf("Expected error to be:\n  %s\nbut got:\n  %s",
+			expectedExitStatusErr, actualErr.Error())
 	}
 }

shell/run_cmd_windows_test.go

@@ -58,9 +58,13 @@ func TestWindowsRunCommandWithOutputInterrupt(t *testing.T) {
 	containsKilled := strings.Contains(actualErrStr, "signal: killed")
 	containsFailedExecute := strings.Contains(actualErrStr, fmt.Sprintf("Failed to execute \"%s", cmdPath))
 
+	if containsKilled {
+		t.Errorf("Expected process to gracefully terminate but got\n: %s", actualErrStr)
+	}
+
 	// On Windows, the batch file might exit with status 1 when interrupted, or be killed by signal
-	if !containsFailedExecute || (!containsExitStatus5 && !containsExitStatus1 && !containsKilled) {
-		t.Errorf("Expected error to contain 'Failed to execute \"%s' and either 'exit status 5', 'exit status 1', or 'signal: killed', but got:\n  %s",
+	if !containsFailedExecute || (!containsExitStatus5 && !containsExitStatus1) {
+		t.Errorf("Expected error to contain 'Failed to execute \"%s' and either 'exit status 5', or 'exit status 1', but got:\n  %s",
 			cmdPath, actualErrStr)
 	}
 }

test/helpers/test_helpers.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 
 	"github.com/gruntwork-io/terragrunt/internal/component"
+	"github.com/gruntwork-io/terragrunt/internal/os/signal"
 	"github.com/gruntwork-io/terragrunt/options"
 	"github.com/stretchr/testify/require"
 )
@@ -148,6 +149,17 @@ func ExecWithTestLogger(t *testing.T, dir, command string, args ...string) {
 	ctx := t.Context()
 	cmd := exec.CommandContext(ctx, command, args...)
 	cmd.Dir = dir
+	cmd.Cancel = func() error {
+		if cmd.Process == nil {
+			return nil
+		}
+
+		if sig := signal.SignalFromContext(ctx); sig != nil {
+			return cmd.Process.Signal(sig)
+		}
+
+		return cmd.Process.Signal(os.Kill)
+	}
 
 	var stdout, stderr bytes.Buffer
 

util/shell.go

@@ -4,11 +4,10 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"os/exec"
 	"strings"
 	"syscall"
 
-	"os/exec"
-
 	"github.com/gruntwork-io/terragrunt/internal/cli"
 	"github.com/gruntwork-io/terragrunt/internal/errors"
 )
@@ -44,13 +43,11 @@ func GetExitCode(err error) (int, error) {
 	var exitStatus interface {
 		ExitStatus() (int, error)
 	}
-
 	if errors.As(err, &exitStatus) {
 		return exitStatus.ExitStatus()
 	}
 
 	var exitCoder cli.ExitCoder
-
 	if errors.As(err, &exitCoder) {
 		return exitCoder.ExitCode(), nil
 	}