From cb56ca15959af26c28dec9f1c3fdf64df77622ca Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 19 May 2026 10:46:57 +0000 Subject: [PATCH 1/2] fix: propagate child process exit code Both LandJail.Run() and NSJailManager.Run() always returned nil, discarding the child process exit code. The landjail child also wrapped exit codes in fmt.Errorf() instead of calling os.Exit(). Changes: - Add exitcode.Error type to carry exit codes through the error chain - Fix landjail child to call os.Exit(exitCode), matching nsjail behavior - Fix both managers to capture child errors via a channel and return exitcode.Error from Run() - Fix main.go to extract exitcode.Error before defaulting to os.Exit(1) - Change NSJailManager.RunChildProcess to return error (was void) Fixes coder/boundary#190 --- cli/cli.go | 16 +++++++++++++++- landjail/child.go | 11 ++++------- landjail/manager.go | 19 ++++++++++++++----- nsjail_manager/child.go | 13 +++---------- nsjail_manager/manager.go | 25 +++++++++++++++++++------ 5 files changed, 55 insertions(+), 29 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index f43eeca2..0042b384 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -2,8 +2,10 @@ package cli import ( "encoding/json" + "errors" "fmt" "os" + "os/exec" "path/filepath" "github.com/coder/boundary/config" @@ -241,7 +243,19 @@ func BaseCommand(version string) *serpent.Command { } logger.Debug("Application config", "config", appConfigInJSON) - return run.Run(inv.Context(), logger, appConfig) + err = run.Run(inv.Context(), logger, appConfig) + + // If the child process exited with a non-zero code, exit + // with the same code directly. All cleanup (proxy, etc.) + // has already happened inside Run(). Exiting here ensures + // the correct code is propagated regardless of how the + // calling framework handles errors (standalone binary or + // embedded as a coder subcommand). + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + os.Exit(exitErr.ExitCode()) + } + return err }, } } diff --git a/landjail/child.go b/landjail/child.go index a88abf92..4ecc3285 100644 --- a/landjail/child.go +++ b/landjail/child.go @@ -61,15 +61,12 @@ func RunChild(logger *slog.Logger, config config.AppConfig) error { // Run the command - this will block until it completes err = cmd.Run() if err != nil { - // Check if this is a normal exit with non-zero status code if exitError, ok := err.(*exec.ExitError); ok { - exitCode := exitError.ExitCode() - logger.Debug("Command exited with non-zero status", "exit_code", exitCode) - return fmt.Errorf("command exited with code %d", exitCode) + logger.Debug("Command exited with non-zero status", "exit_code", exitError.ExitCode()) + } else { + logger.Error("Command execution failed", "error", err) } - // This is an unexpected error - logger.Error("Command execution failed", "error", err) - return fmt.Errorf("command execution failed: %v", err) + return err } logger.Debug("Command completed successfully") diff --git a/landjail/manager.go b/landjail/manager.go index b38f54f5..41671033 100644 --- a/landjail/manager.go +++ b/landjail/manager.go @@ -67,12 +67,12 @@ func (b *LandJail) Run(ctx context.Context) error { ctx, cancel := context.WithCancel(ctx) defer cancel() + // childErr receives the result of RunChildProcess so we can + // propagate the child's exit code to our caller. + childErr := make(chan error, 1) go func() { defer cancel() - err := b.RunChildProcess(os.Args) - if err != nil { - b.logger.Error("Failed to run child process", "error", err) - } + childErr <- b.RunChildProcess(os.Args) }() // Setup signal handling BEFORE any setup @@ -89,7 +89,16 @@ func (b *LandJail) Run(ctx context.Context) error { b.logger.Info("Command completed, shutting down...") } - return nil + // Drain the child result if available. In the ctx.Done path the + // error is already buffered. In the signal path the child may still + // be running; return nil so deferred cleanup (iptables, proxy) can + // proceed before the process exits. + select { + case err := <-childErr: + return err + default: + return nil + } } func (b *LandJail) RunChildProcess(command []string) error { diff --git a/nsjail_manager/child.go b/nsjail_manager/child.go index d27c10a4..bf8ea0b3 100644 --- a/nsjail_manager/child.go +++ b/nsjail_manager/child.go @@ -92,18 +92,11 @@ func RunChild(logger *slog.Logger, cfg config.AppConfig) error { } err = cmd.Run() if err != nil { - // Check if this is a normal exit with non-zero status code if exitError, ok := err.(*exec.ExitError); ok { - exitCode := exitError.ExitCode() - // Log at debug level for non-zero exits (normal behavior) - logger.Debug("Command exited with non-zero status", "exit_code", exitCode) - // Exit with the same code as the command - don't log as error - // This is normal behavior (commands can exit with any code) - os.Exit(exitCode) + logger.Debug("Command exited with non-zero status", "exit_code", exitError.ExitCode()) + } else { + logger.Error("Command execution failed", "error", err) } - // This is an unexpected error (not just a non-zero exit) - // Only log actual errors like "command not found" or "permission denied" - logger.Error("Command execution failed", "error", err) return err } diff --git a/nsjail_manager/manager.go b/nsjail_manager/manager.go index b0ddfda1..348620d3 100644 --- a/nsjail_manager/manager.go +++ b/nsjail_manager/manager.go @@ -71,9 +71,12 @@ func (b *NSJailManager) Run(ctx context.Context) error { ctx, cancel := context.WithCancel(ctx) defer cancel() + // childErr receives the result of RunChildProcess so we can + // propagate the child's exit code to our caller. + childErr := make(chan error, 1) go func() { defer cancel() - b.RunChildProcess(os.Args) + childErr <- b.RunChildProcess(os.Args) }() // Setup signal handling BEFORE any setup @@ -90,23 +93,32 @@ func (b *NSJailManager) Run(ctx context.Context) error { b.logger.Info("Command completed, shutting down...") } - return nil + // Drain the child result if available. In the ctx.Done path the + // error is already buffered. In the signal path the child may still + // be running; return nil so deferred cleanup (iptables, proxy) can + // proceed before the process exits. + select { + case err := <-childErr: + return err + default: + return nil + } } -func (b *NSJailManager) RunChildProcess(command []string) { +func (b *NSJailManager) RunChildProcess(command []string) error { cmd := b.jailer.Command(command) b.logger.Debug("Executing command in boundary", "command", strings.Join(os.Args, " ")) err := cmd.Start() if err != nil { b.logger.Error("Command failed to start", "error", err) - return + return err } err = b.jailer.ConfigureHostNsCommunication(cmd.Process.Pid) if err != nil { b.logger.Error("configuration after command execution failed", "error", err) - return + return err } b.logger.Debug("waiting on a child process to finish") @@ -121,9 +133,10 @@ func (b *NSJailManager) RunChildProcess(command []string) { // This is an unexpected error (not just a non-zero exit) b.logger.Error("Command execution failed", "error", err) } - return + return err } b.logger.Debug("Command completed successfully") + return nil } func (b *NSJailManager) setupHostAndStartProxy() error { From 8b03bb7d6c9a75a3fcaaacb89f488a58f36f3650 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 20 May 2026 13:40:42 +0000 Subject: [PATCH 2/2] fix: wrap exit error with descriptive message using %w Addresses review feedback: re-add error wrapping for ExitError with fmt.Errorf and %w verb so the error message is descriptive while preserving the *exec.ExitError type for errors.As(). --- landjail/child.go | 6 +++--- nsjail_manager/child.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/landjail/child.go b/landjail/child.go index 4ecc3285..ad9c9dc9 100644 --- a/landjail/child.go +++ b/landjail/child.go @@ -63,10 +63,10 @@ func RunChild(logger *slog.Logger, config config.AppConfig) error { if err != nil { if exitError, ok := err.(*exec.ExitError); ok { logger.Debug("Command exited with non-zero status", "exit_code", exitError.ExitCode()) - } else { - logger.Error("Command execution failed", "error", err) + return fmt.Errorf("command exited with code %d: %w", exitError.ExitCode(), err) } - return err + logger.Error("Command execution failed", "error", err) + return fmt.Errorf("command execution failed: %w", err) } logger.Debug("Command completed successfully") diff --git a/nsjail_manager/child.go b/nsjail_manager/child.go index bf8ea0b3..41321ec1 100644 --- a/nsjail_manager/child.go +++ b/nsjail_manager/child.go @@ -94,10 +94,10 @@ func RunChild(logger *slog.Logger, cfg config.AppConfig) error { if err != nil { if exitError, ok := err.(*exec.ExitError); ok { logger.Debug("Command exited with non-zero status", "exit_code", exitError.ExitCode()) - } else { - logger.Error("Command execution failed", "error", err) + return fmt.Errorf("command exited with code %d: %w", exitError.ExitCode(), err) } - return err + logger.Error("Command execution failed", "error", err) + return fmt.Errorf("command execution failed: %w", err) } // Command exited successfully