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

[gitulisca]

Problem

main() in main.go handled the server.ListenAndServe() failure case with the pattern:

if err := server.ListenAndServe(); err != http.ErrServerClosed {
    log.Println(err)
    return
}

A bare return from main() allows the process to exit normally, which Go reports as exit code 0, even though a fatal error occurred (a real listen/bind failure, as opposed to http.ErrServerClosed, which is the expected result of a graceful shutdown).

Why this matters

This can happen for two distinct reasons:

1. Port collision with a third-party process. The proxy's default listen port happens to already be in use by some unrelated application on the host, purely by coincidence.
2. Multiple concurrent proxy processes on the same host. Customers running several non-containerized GitHub Actions Runner Application processes on a single host/VM can end up starting multiple proxy processes that all attempt to bind the same port at the same time. In this scenario the collision isn't coincidental third-party interference — it's an inherent consequence of running multiple runner processes side by side without container-level network isolation, so it's expected to recur reliably, not as a rare edge case.

In both cases, the proxy logged the bind error and then exited with status 0. Any integration that gates on exit code interpreted that as a successful start and proceeded as if the proxy were listening and ready to serve traffic, when in fact no proxy was running. This is a silent failure: the calling process has no reliable signal that something went wrong.

Concrete impact: github/codeql-action

This is not a hypothetical concern — github/codeql-action's start-proxy action explicitly relies on update-job-proxy's exit code to detect a failed startup and retry on a different port. See start-proxy-action.ts#L164-L170:

subprocess.on("exit", (code) => {
  if (code !== 0) {
    // If the proxy failed to start, try a different port from the ephemeral range [49152, 65535]
    port = Math.floor(Math.random() * (65535 - 49152) + 49152);
    subprocess = undefined;
  }
});

This handler is the only signal the action has that the proxy failed to bind its port; when code === 0 it assumes the previous attempt succeeded and moves on to report a successful start (logger.info(\Proxy started on ${host}:${port}`)) using a port that, in reality, has no proxy listening on it. With the current bug, a bind failure always reports code === 0`, so this retry-on-a-different-port logic never triggers, and CodeQL/Actions traffic that should have gone through the MITM proxy for private registry authentication is silently misrouted or fails downstream instead of being caught and retried at the source.

Reproduction of the current (buggy) behavior

# ./linux/update-job-proxy -addr 127.0.0.1:49152 -config - -logfile /tmp/proxy1.log < config.json &
# cat /tmp/proxy1.log
2026/08/20 05:07:19 proxy starting, commit: dev
2026/08/20 05:07:19 GitHubAPIHandler has no app access tokens
2026/08/20 05:07:19 Listening (127.0.0.1:49152)
# ./linux/update-job-proxy -addr 127.0.0.1:49152 -config - -logfile /tmp/proxy2.log < config.json
# echo $?
0
# cat /tmp/proxy2.log
2026/08/20 05:07:58 proxy starting, commit: dev
2026/08/20 05:07:58 GitHubAPIHandler has no app access tokens
2026/08/20 05:07:58 Listening (127.0.0.1:49152)
2026/08/20 05:07:58 listen tcp 127.0.0.1:49152: bind: address already in use

The second proxy process fails to bind the already-occupied port, logs the error, and still exits with code 0, matching the second scenario above (a second runner's proxy process colliding with a first one already listening on the same host).

Fix

Replace the log.Println(err); return pattern in the server.ListenAndServe() branch with log.Fatal(err), which logs the error and then calls os.Exit(1). This is a one-line change, scoped strictly to this branch. The existing err != http.ErrServerClosed guard is preserved, so a graceful shutdown (SIGINT/SIGTERM) still exits 0 as before — only a genuine listen/bind failure now propagates a non-zero exit code, allowing consumers like the start-proxy action's retry logic above to work as designed.

This matches the convention already established elsewhere in this codebase for fatal setup errors: proxy.go's setCA and cache.New failures, and logging.go's log-file-open failure, all already use log.Fatal(err).

Scope note: the config.Parse, setupSentry, and proxy.Close error branches in main() use the same log.Println(err); return pattern and technically share the same silent-exit-0 characteristic. This PR intentionally leaves them untouched, since it's unclear whether that behavior was a deliberate design choice for those specific cases, and the reported defect is specifically about ListenAndServe. Changing them can be considered separately if desired.

Test coverage

Since log.Fatal terminates the process, the resulting exit code can't be observed in-process. main_test.go uses the standard Go "helper process" pattern (the same technique used in the Go standard library's own os/exec and os/signal tests): TestMain re-executes the already-compiled test binary via os.Args[0] under an environment variable flag, so the subprocess runs the real main() and its exit code reflects actual behavior.

New tests:

• TestListenAndServe_AddressInUse_ExitsNonZero — pre-binds a port, then launches the proxy against that same address and asserts the process now exits non-zero. This directly reproduces and guards against the reported defect (both the coincidental-collision and concurrent-runner-processes scenarios manifest identically at this level: two processes contending for one port).
• TestGracefulShutdown_ExitsZero — starts the proxy, sends SIGTERM once it's listening, and asserts it still exits 0. Regression guard for the normal shutdown path.

Verification

• go build ./...
• go vet ./...
• gofmt -l . (clean)
• go test -race -shuffle=on -count=2 ./... — all packages pass

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

[Copilot]

Pull request overview

Ensures fatal proxy startup/runtime errors return a non-zero exit code.

Changes:

• Replaces silent error returns with log.Fatal.
• Adds subprocess tests for bind failures, invalid configuration, and graceful shutdown.

Show a summary per file

File	Description
main.go	Exits non-zero on fatal errors.
main_test.go	Adds exit-code regression tests.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (1)

main_test.go:132

• os.Process.Signal(syscall.SIGTERM) is unsupported on Windows, where it returns os.ErrProcessDone/EWINDOWS rather than delivering a graceful signal. This makes go test fail for a platform this repository explicitly builds; skip this signal-based assertion on Windows (cleaning up the child), or use a Windows-specific console control event.

	require.NoError(t, cmd.Process.Signal(syscall.SIGTERM))

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced

[Nishnha]

I pushed a follow-up because the current log.Fatal fix solves one problem by bringing back another. We need the proxy to return a failure code when it cannot start, but we also need it to close its log file before exiting.

The proxy originally used log.Fatal for these errors. log.Fatal writes the error and exits with status 1, which correctly tells the caller that the proxy failed. The problem is that it exits immediately, before Go runs deferred cleanup such as closing the log file. PR 37 changed log.Fatal to log.Println followed by return for that reason. The cleanup commit shows the exact change.

I traced the current bug back to that cleanup change. Returning from Go's main() function always exits with status 0, which means success. The proxy could fail to bind to its port, log the error, and then tell CodeQL that it succeeded. CodeQL only retries on another port when the proxy exits with a failure status, so it never retried.

I considered replacing log.Fatal with log.Println followed by os.Exit(1), but that would have the same cleanup problem. os.Exit is the part that stops the process immediately and skips deferred work.

I moved the proxy work into a separate run() function instead. run() returns status 1 when the proxy fails and status 0 after a normal shutdown. Because run() returns normally, Go closes the log file before control goes back to main(). main() then calls os.Exit with the status returned by run().

This keeps the cleanup from PR 37 and restores the failure signal that CodeQL needs. I also restored the invalid-config regression test and updated the new process-test helpers to satisfy the repository's context-aware lint rules.

[Copilot]

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

main_test.go:124

• After Start succeeds, any later require failure (especially the readiness timeout or signal failure) exits the test without calling Wait, leaving the helper process running until context cancellation and unreaped afterward. Register cleanup immediately so every failure path kills and waits for the child; the normal path remains unaffected because ProcessState is set after Wait.

	require.NoError(t, cmd.Start())

• Files reviewed: 2/2 changed files
• Comments generated: 3
• Review effort level: Balanced