Fix proxy exiting with code 0 when server.ListenAndServe() fails (e.g. bind address already in use)

main.go

@@ -31,6 +31,10 @@ var (
 )
 
 func main() {
+	os.Exit(run())
+}
+
+func run() int {
 	flag.Parse()
 	file := setupLogging()
 	if file != nil {
@@ -45,13 +49,13 @@ func main() {
 	cfg, err := config.Parse(*configPath)
 	if err != nil {
 		log.Println(err)
-		return
+		return 1
 	}
 
 	sentry, err := setupSentry()
 	if err != nil {
 		log.Println(err)
-		return
+		return 1
 	}
 
 	envSettings := config.ProxyEnvSettings{
@@ -97,13 +101,15 @@ func main() {
 	log.Printf("Listening (%s)", *addr)
 	if err := server.ListenAndServe(); err != http.ErrServerClosed {
 		log.Println(err)
-		return
+		return 1
 	}
 
 	if err := proxy.Close(); err != nil {
 		log.Println(err)
-		return
+		return 1
 	}
+
+	return 0
 }
 
 func setupSentry() (bool, error) {

main_test.go

@@ -0,0 +1,168 @@
+package main
+
+import (
+	"bytes"
+	"encoding/json"
+	"errors"
+	"net"
+	"os"
+	"os/exec"
+	"runtime"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/dependabot/proxy/internal/config"
+)
+
+// helperProcessEnv, when set to "1" in the environment, tells TestMain to run
+// the real main() instead of the test suite. This lets tests re-exec the
+// already-compiled test binary as a standalone proxy process and observe its
+// real exit code - something that can't be done in-process once main() calls
+// log.Fatal/os.Exit. This is the same "helper process" technique the Go
+// standard library uses to test os.Exit/signal behavior (see os/exec and
+// os/signal tests).
+const helperProcessEnv = "PROXY_HELPER_PROCESS"
+
+func TestMain(m *testing.M) {
+	if os.Getenv(helperProcessEnv) == "1" {
+		main()
+		return
+	}
+	os.Exit(m.Run())
+}
+
+// runHelperProcess builds an *exec.Cmd that re-executes this test binary as a
+// standalone proxy process (via the TestMain hook above), passing args as
+// command-line flags and stdin as its stdin.
+func runHelperProcess(t *testing.T, args []string, stdin string) *exec.Cmd {
+	t.Helper()
+	cmd := exec.CommandContext(t.Context(), os.Args[0], args...) //nolint:gosec // args are test-controlled, not user input
+	cmd.Env = append(os.Environ(), helperProcessEnv+"=1")
+	cmd.Stdin = strings.NewReader(stdin)
+	return cmd
+}
+
+// minimalConfigJSON returns a valid proxy config (with a working MITM CA)
+// serialized as JSON, suitable for feeding via stdin so the helper process
+// can get all the way to server.ListenAndServe().
+func minimalConfigJSON(t *testing.T) string {
+	t.Helper()
+	cfg := config.Config{CA: testCA()}
+	b, err := json.Marshal(cfg)
+	require.NoError(t, err)
+	return string(b)
+}
+
+// freeAddr returns a "host:port" address that is free at the time of the
+// call by briefly binding to port 0 and releasing it.
+func freeAddr(t *testing.T) string {
+	t.Helper()
+	var listenConfig net.ListenConfig
+	l, err := listenConfig.Listen(t.Context(), "tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	addr := l.Addr().String()
+	require.NoError(t, l.Close())
+	return addr
+}
+
+// exitCodeFromWaitErr extracts the process exit code from the error returned
+// by exec.Cmd.Wait()/Run(). A nil error means exit code 0.
+func exitCodeFromWaitErr(t *testing.T, err error) int {
+	t.Helper()
+	if err == nil {
+		return 0
+	}
+	var exitErr *exec.ExitError
+	if errors.As(err, &exitErr) {
+		return exitErr.ExitCode()
+	}
+	t.Fatalf("helper process did not exit normally: %v", err)
+	return -1
+}
+
+// TestListenAndServe_AddressInUse_ExitsNonZero reproduces the reported
+// defect: the proxy previously logged the bind error and exited 0, causing
+// integrations that gate on exit code (e.g. github/codeql-action) to
+// silently proceed as if the proxy were listening.
+func TestListenAndServe_AddressInUse_ExitsNonZero(t *testing.T) {
+	var listenConfig net.ListenConfig
+	l, err := listenConfig.Listen(t.Context(), "tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	defer func() {
+		require.NoError(t, l.Close())
+	}()
+	addr := l.Addr().String()
+
+	cmd := runHelperProcess(t, []string{"-addr=" + addr, "-config=-"}, minimalConfigJSON(t))
+	var stderr bytes.Buffer
+	cmd.Stderr = &stderr
+
+	runErr := cmd.Run()
+
+	code := exitCodeFromWaitErr(t, runErr)
+	assert.NotEqual(t, 0, code, "expected non-zero exit code when the listen address is already in use, got 0 (output: %s)", stderr.String())
+	// The OS-level error text differs by platform (e.g. "address already in
+	// use" on Unix vs. "Only one usage of each socket address..." on
+	// Windows), but the "listen tcp <addr>: bind:" prefix is generated by
+	// Go's net package itself and is stable across platforms.
+	assert.Contains(t, stderr.String(), "listen tcp "+addr+": bind:")
+}
+
+// TestGracefulShutdown_ExitsZero is a regression guard: a clean shutdown via
+// SIGTERM (the normal operational path) must still exit 0 after the fix.
+func TestGracefulShutdown_ExitsZero(t *testing.T) {
+	addr := freeAddr(t)
+
+	cmd := runHelperProcess(t, []string{"-addr=" + addr, "-config=-"}, minimalConfigJSON(t))
+	var stderr bytes.Buffer
+	cmd.Stderr = &stderr
+	require.NoError(t, cmd.Start())
+
+	// Note: stderr is not safe to read here - it's being written to
+	// concurrently by the still-running subprocess, so the failure message
+	// below intentionally omits its contents (only safe to read after
+	// cmd.Wait() below).
+	dialer := net.Dialer{Timeout: 100 * time.Millisecond}
+	require.Eventually(t, func() bool {
+		conn, dialErr := dialer.DialContext(t.Context(), "tcp", addr)
+		if dialErr != nil {
+			return false
+		}
+		_ = conn.Close()
+		return true
+	}, 5*time.Second, 50*time.Millisecond, "proxy did not start listening in time")
+
+	if runtime.GOOS == "windows" {
+		// os.Process.Signal(syscall.SIGTERM) is not supported on Windows: it
+		// returns os.ErrProcessDone/EWINDOWS instead of delivering a graceful
+		// shutdown signal, so the assertion below would fail there even
+		// though the exit-code fix itself is correct. Terminate the helper
+		// process directly instead of leaking it, and skip the
+		// signal-based assertion on this platform.
+		_ = cmd.Process.Kill()
+		_ = cmd.Wait()
+		t.Skip("SIGTERM cannot be delivered via os.Process.Signal on Windows; skipping graceful-shutdown assertion")
+	}
+
+	require.NoError(t, cmd.Process.Signal(syscall.SIGTERM))
+
+	waitErr := cmd.Wait()
+	code := exitCodeFromWaitErr(t, waitErr)
+	assert.Equal(t, 0, code, "expected graceful shutdown to exit 0 (output: %s)", stderr.String())
+}
+
+func TestInvalidConfigPath_ExitsNonZero(t *testing.T) {
+	cmd := runHelperProcess(t, []string{"-config=/nonexistent/path/definitely-missing.json"}, "")
+	var stderr bytes.Buffer
+	cmd.Stderr = &stderr
+
+	runErr := cmd.Run()
+
+	code := exitCodeFromWaitErr(t, runErr)
+	assert.NotEqual(t, 0, code, "expected non-zero exit code for an invalid config path, got 0 (output: %s)", stderr.String())
+}