fix: propagate child process exit code

[mtojek]

Fixes #190

Both LandJail.Run() and NSJailManager.Run() always returned nil, discarding the child process exit code. The boundary process exited 0 regardless of what the target command returned.

Changes

• landjail/manager.go, nsjail_manager/manager.go: Capture the child process error via a buffered channel instead of discarding it in the goroutine. Run() now blocks on the channel after the select and returns the error (which includes *exec.ExitError with the correct code).
• landjail/child.go, nsjail_manager/child.go: Return the raw *exec.ExitError from cmd.Run() instead of calling os.Exit() or wrapping it in fmt.Errorf(). Serpent's RunCommandError has Unwrap(), so errors.As can find *exec.ExitError through the entire chain.
• cli/cli.go: The handler calls os.Exit(exitCode) when the child process exits with a non-zero code. All cleanup (proxy stop, etc.) has already happened inside Run() via defers. This ensures the correct exit code is propagated regardless of how the caller handles errors, both as a standalone binary and when embedded as a coder boundary subcommand (no changes needed in coder/coder).

How to test

Build and run:

go build -o ./boundary ./cmd/boundary/

# success → 0
./boundary --jail-type landjail -- true; echo $?

# false → 1
./boundary --jail-type landjail -- false; echo $?

# arbitrary exit code → 42
./boundary --jail-type landjail -- bash -c 'exit 42'; echo $?

# arbitrary exit code → 127
./boundary --jail-type landjail -- bash -c 'exit 127'; echo $?

# command not found → 1
./boundary --jail-type landjail -- no-such-cmd; echo $?

landjail requires kernel 6.7+ (Landlock V4). Use --jail-type nsjail with appropriate privileges on older kernels.

---

Generated by Coder Agents

[SasSwart]

Nothing blocking given this is easy to patch and a minor fix. But a few comments to consider nonetheless.

[SasSwart]

This is a convenient solution, but it makes me nervous to have an os.Exit that is more than a single level of indirection from the entrypoint like this. Looking at this bit of code here, we don't how what cleanup would have happened on the return path between here and the entry point.

I think the proper solution is to do the error checking near the entry point both here and in the coder subcommand.

In practice, the risk is low and its easy to patch later, so I don't think this blocks the PR. Its worth a mention though.

[mtojek]

Agreed it's not ideal. The problem is the embedded mode (coder boundary ...), we don't control coder's entrypoint, and serpent wraps our returned error in RunCommandError, so coder's main() just does os.Exit(1) losing the actual code.

To do it "properly" we'd need changes in coder/coder or serpent. This is a conscious tradeoff: all cleanup (proxy, iptables) already ran inside Run() via defers before the error returns, so the os.Exit is safe.

Let me know your thoughts!

[SasSwart]

nit: I feel like the error wrapping here was useful. It provides a better single description of the failure path than disjoint debug logs that might be filtered out.

[mtojek]

Ok, brought it back 👍

[SasSwart]

Asking for clarity:

why do we need a second select here instead of a three case select above?

select {
case sig := <-sigChan:
// ...
case err := <-childErr:
// ...
case <-ctx.Done():
// ...
}

[mtojek]

When the child finishes, the goroutine sends on childErr AND calls defer cancel(), which closes ctx.Done(). So both channels are ready at roughly the same time. Go picks randomly between ready cases - if we land in ctx.Done() instead of childErr, we lose the exit code error and return nil.

Two selects avoid that: first one waits for signal or context cancellation, second one (non-blocking) drains the child result. In the ctx.Done path the error is already buffered so we always get it. In the signal path the child may still be running, so default: return nil lets deferred cleanup proceed.