From 70096b533d42b684ab13651aaae884047e01e43d Mon Sep 17 00:00:00 2001 From: James O'Doherty Date: Fri, 29 May 2026 19:21:49 -0400 Subject: refactor: optimize file cleanups, propagate exit codes, and fix Makefile - Unlink the temporary bootstrap launcher binary immediately after opening a read-only descriptor to it, then execute via `/proc/self/fd/` to ensure zero-disk footprint on execution. - Unlink temporary `/tmp/resolvconf*` files immediately after successful bind-mounting over `/etc/resolv.conf`. - Prune parent ephemeral profile directories when unpinning a namespace, leaving zero directories behind once empty. - Propagate the exact exit status of the wrapped command to the host process using `errors.As` and `*exec.ExitError` instead of defaulting to exit code 1. - Added E2E automated test `TestExitCodePropagation` to verify exit status delivery. - Added the `$(BINARY)` target to `.PHONY` in the Makefile to delegate dependency tracking to Go's compiler cache, ensuring modified Go files are rebuilt during `make test`. --- Makefile | 2 +- internal/namespace/namespace.go | 22 +++++++++++++++++++++- internal/namespace/pinning.go | 4 +++- internal/wireguard/wireguard.go | 5 +++++ tests/e2e/e2e_test.go | 23 +++++++++++++++++++++++ 5 files changed, 53 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index dc57dfa..0597d84 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ BINARY = wg-wrap FUZZ_PARALLEL ?= 2 FUZZ_TIME ?= 30s -.PHONY: all clean test fuzz +.PHONY: all clean test fuzz $(BINARY) # Default target: build the final binary all: $(BINARY) diff --git a/internal/namespace/namespace.go b/internal/namespace/namespace.go index a1e7ad9..e2ef2f1 100644 --- a/internal/namespace/namespace.go +++ b/internal/namespace/namespace.go @@ -100,16 +100,36 @@ func Bootstrap() error { // 2. Write the embedded launcher binary to the temp file. if _, err := tmpFile.Write(launcherBytes); err != nil { _ = tmpFile.Close() + _ = os.Remove(launcherPath) return fmt.Errorf("failed to write launcher binary: %w", err) } // Ensure the binary is executable (0700) if err := tmpFile.Chmod(0700); err != nil { _ = tmpFile.Close() + _ = os.Remove(launcherPath) return fmt.Errorf("failed to set launcher permissions: %w", err) } + + // 2b. Open a read-only fd of the launcher to exec + execFd, err := syscall.Open(launcherPath, syscall.O_RDONLY, 0) + if err != nil { + _ = tmpFile.Close() + _ = os.Remove(launcherPath) + return fmt.Errorf("failed to open launcher for exec: %w", err) + } + + // Close the write file descriptor (to avoid ETXTBSY) _ = tmpFile.Close() + // Unlink the file from disk (makes it invisible and ensures it is deleted on exit) + _ = os.Remove(launcherPath) + + // Clear close-on-exec so it remains open across syscall.Exec + if flags, err := unix.FcntlInt(uintptr(execFd), unix.F_GETFD, 0); err == nil { + _, _ = unix.FcntlInt(uintptr(execFd), unix.F_SETFD, flags&^unix.FD_CLOEXEC) + } + // 3. Prepare arguments for the launcher. // The launcher expects: launcher [args...] args := []string{self} @@ -153,7 +173,7 @@ func Bootstrap() error { } } - err = syscall.Exec(launcherPath, args, env) + err = syscall.Exec(fmt.Sprintf("/proc/self/fd/%d", execFd), args, env) if err != nil { return fmt.Errorf("launcher exec failed: %w", err) } diff --git a/internal/namespace/pinning.go b/internal/namespace/pinning.go index 7976937..eb0a376 100644 --- a/internal/namespace/pinning.go +++ b/internal/namespace/pinning.go @@ -44,8 +44,10 @@ func UnpinNamespace(pm *paths.PathManager, profile string) error { return fmt.Errorf("failed to unpin namespace %s: %w", nsPath, err) } - // Try to remove pids directory + // Try to remove pids directory and empty parent directories _ = os.Remove(pidsDir) + _ = os.Remove(filepath.Dir(pidsDir)) + _ = os.Remove(filepath.Dir(filepath.Dir(pidsDir))) return nil } diff --git a/internal/wireguard/wireguard.go b/internal/wireguard/wireguard.go index a45401c..5bbc518 100644 --- a/internal/wireguard/wireguard.go +++ b/internal/wireguard/wireguard.go @@ -244,9 +244,14 @@ func ConfigureResolvConf(dns string) error { // 1. Bind-mount the temp file over /etc/resolv.conf if err := unix.Mount(tmpFile.Name(), "/etc/resolv.conf", "", unix.MS_BIND, ""); err != nil { + _ = os.Remove(tmpFile.Name()) return fmt.Errorf("failed to bind-mount %s to /etc/resolv.conf: %w", tmpFile.Name(), err) } + // Unlink the temporary source file. Since /etc/resolv.conf is a bind mount, + // the kernel will keep the inode alive, but the file is removed from /tmp. + _ = os.Remove(tmpFile.Name()) + // 2. Make the mount private to ensure it doesn't propagate back to the host // and to satisfy kernel requirements for mount transitions in some environments. if err := unix.Mount("/etc/resolv.conf", "/etc/resolv.conf", "", unix.MS_REMOUNT|unix.MS_BIND|unix.MS_PRIVATE, ""); err != nil { diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index e37810a..939d231 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -286,3 +286,26 @@ func TestMTUFragmentation(t *testing.T) { t.Errorf("expected command to pass, got: %v", err) } } + +func TestExitCodePropagation(t *testing.T) { + binaryPath, err := GetBinaryPath() + if err != nil { + t.Skipf("Skipping test: %v", err) + } + + // Run a command that exits with code 42 + cmd := exec.Command(binaryPath, "--profile", "default", "--", "sh", "-c", "exit 42") + err = cmd.Run() + if err == nil { + t.Fatalf("expected command to fail with exit status 42, but it succeeded") + } + + exitErr, ok := err.(*exec.ExitError) + if !ok { + t.Fatalf("expected error of type *exec.ExitError, got %T: %v", err, err) + } + + if exitErr.ExitCode() != 42 { + t.Errorf("expected exit code 42, got %d", exitErr.ExitCode()) + } +} -- cgit v1.2.3