fix: close late proxy listeners and validate HTTP usernames

cmd/autocar/client.go

@@ -64,7 +64,7 @@ func runClient(parent context.Context, args []string) error {
 		if err != nil {
 			return err
 		}
-		if err := validateProxyCredentials(*proxyUser, password, *socksAddress != ""); err != nil {
+		if err := validateProxyCredentials(*proxyUser, password, *socksAddress != "", *httpAddress != "" || *httpsAddress != ""); err != nil {
 			return err
 		}
 		authenticator = proxy.StaticAuthenticator(*proxyUser, password)
@@ -214,10 +214,13 @@ func runClient(parent context.Context, args []string) error {
 	return nil
 }
 
-func validateProxyCredentials(username, password string, socksEnabled bool) error {
+func validateProxyCredentials(username, password string, socksEnabled, httpEnabled bool) error {
 	if len(password) < 16 {
 		return errors.New("local proxy password must be at least 16 bytes")
 	}
+	if httpEnabled && strings.Contains(username, ":") {
+		return errors.New("local proxy username must not contain ':' when HTTP or HTTPS is enabled")
+	}
 	// RFC 1929 encodes both lengths in one byte. HTTP Basic itself allows
 	// longer values, so apply this compatibility bound only when SOCKS is on.
 	if socksEnabled && (len(username) > 255 || len(password) > 255) {

cmd/autocar/main_test.go

@@ -144,19 +144,19 @@ func TestEnsureProtectedPlaintextListener(t *testing.T) {
 }
 
 func TestValidateProxyCredentials(t *testing.T) {
-	if err := validateProxyCredentials("alice", strings.Repeat("x", 16), true); err != nil {
+	if err := validateProxyCredentials("alice", strings.Repeat("x", 16), true, true); err != nil {
 		t.Fatal(err)
 	}
-	if err := validateProxyCredentials("alice", "too-short", true); err == nil {
+	if err := validateProxyCredentials("alice", "too-short", true, true); err == nil {
 		t.Fatal("short local proxy password accepted")
 	}
-	if err := validateProxyCredentials(strings.Repeat("u", 256), strings.Repeat("p", 16), true); err == nil {
+	if err := validateProxyCredentials(strings.Repeat("u", 256), strings.Repeat("p", 16), true, true); err == nil {
 		t.Fatal("oversized SOCKS5 username accepted")
 	}
-	if err := validateProxyCredentials("alice", strings.Repeat("p", 256), true); err == nil {
+	if err := validateProxyCredentials("alice", strings.Repeat("p", 256), true, true); err == nil {
 		t.Fatal("oversized SOCKS5 password accepted")
 	}
-	if err := validateProxyCredentials(strings.Repeat("u", 256), strings.Repeat("p", 256), false); err != nil {
+	if err := validateProxyCredentials(strings.Repeat("u", 256), strings.Repeat("p", 256), false, true); err != nil {
 		t.Fatalf("HTTP-only credentials were incorrectly limited to RFC 1929: %v", err)
 	}
 }

cmd/autocar/proxy_credentials_test.go

@@ -0,0 +1,97 @@
+package main
+
+import (
+	"context"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestValidateProxyCredentialsHTTPCompatibility(t *testing.T) {
+	const password = "diagnostic:password:with:colons"
+	for _, test := range []struct {
+		name, username            string
+		socksEnabled, httpEnabled bool
+		wantErr                   bool
+	}{
+		{name: "HTTP colon username", username: "user:name", httpEnabled: true, wantErr: true},
+		{name: "combined colon username", username: "user:name", socksEnabled: true, httpEnabled: true, wantErr: true},
+		{name: "SOCKS-only colon username", username: "user:name", socksEnabled: true},
+		{name: "HTTP colon password", username: "user", httpEnabled: true},
+		{name: "combined colon password", username: "user", socksEnabled: true, httpEnabled: true},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			err := validateProxyCredentials(test.username, password, test.socksEnabled, test.httpEnabled)
+			if (err != nil) != test.wantErr {
+				t.Fatalf("credential validation error = %v, want error=%t", err, test.wantErr)
+			}
+			if err != nil && (strings.Contains(err.Error(), test.username) || strings.Contains(err.Error(), password)) {
+				t.Fatal("credential validation disclosed a credential value")
+			}
+		})
+	}
+}
+
+func TestClientRejectsHTTPColonUsernameBeforeStartup(t *testing.T) {
+	clearPreflightEnvironment(t)
+	t.Setenv("AUTOCAR_PROXY_PASSWORD", "diagnostic:password:with:colons")
+	files := newPreflightFiles(t)
+	dnsCalls := denyPreflightDNS(t)
+	for _, check := range []bool{false, true} {
+		mode := "startup"
+		if check {
+			mode = "check"
+		}
+		for _, test := range []struct {
+			name string
+			args []string
+		}{
+			{name: "default HTTP and SOCKS"},
+			{name: "HTTP only", args: []string{"--socks="}},
+			{name: "HTTPS only", args: []string{"--socks=", "--http=", "--https=127.0.0.1:8443", "--proxy-cert", files.cert, "--proxy-key", files.key}},
+		} {
+			t.Run(mode+"/"+test.name, func(t *testing.T) {
+				// Drop the helper's --check for the startup variant. The
+				// incompatible credentials must fail before binding or dialing.
+				args := append(files.clientArgs()[1:], "--proxy-user=user:name")
+				args = append(args, test.args...)
+				if check {
+					args = append(args, "--check")
+				}
+				ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+				defer cancel()
+				err := runClient(ctx, args)
+				if err == nil || err.Error() != "local proxy username must not contain ':' when HTTP or HTTPS is enabled" {
+					t.Fatalf("client error = %v, want HTTP username compatibility error", err)
+				}
+			})
+		}
+	}
+	if dnsCalls.Load() != 0 {
+		t.Fatal("invalid local credentials triggered DNS")
+	}
+}
+
+func TestClientCheckPreservesColonCredentialCompatibility(t *testing.T) {
+	clearPreflightEnvironment(t)
+	t.Setenv("AUTOCAR_PROXY_PASSWORD", "diagnostic:password:with:colons")
+	files := newPreflightFiles(t)
+	dnsCalls := denyPreflightDNS(t)
+	for _, test := range []struct {
+		name string
+		args []string
+	}{
+		{name: "SOCKS-only colon username", args: []string{"--http=", "--proxy-user=user:name"}},
+		{name: "HTTP colon password", args: []string{"--socks=", "--proxy-user=user"}},
+		{name: "HTTPS colon password", args: []string{"--socks=", "--http=", "--https=127.0.0.1:8443", "--proxy-user=user", "--proxy-cert", files.cert, "--proxy-key", files.key}},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			if err := runClient(context.Background(), append(files.clientArgs(), test.args...)); err != nil {
+				t.Fatalf("compatible local credentials rejected: %v", err)
+			}
+		})
+	}
+	if dnsCalls.Load() != 0 {
+		t.Fatal("local credential checks triggered DNS")
+	}
+}

docs/DEPLOYMENT.md

@@ -420,6 +420,11 @@ those listeners on loopback or enable the local HTTPS proxy. A non-loopback
 plaintext listener requires an explicit override and should still be protected
 by a trusted private network/firewall.
 
+When HTTP or HTTPS is enabled, `--proxy-user` must not contain `:` because HTTP
+Basic uses it to separate the username and password. Startup and `--check`
+reject this configuration. SOCKS-only usernames and passwords may contain `:`;
+passwords containing `:` also remain valid for HTTP/HTTPS.
+
 ## 7. systemd example
 
 `/etc/systemd/system/autocar.service`:

internal/proxy/http.go

@@ -85,6 +85,9 @@ func NewHTTPServer(cfg Config) (*HTTPServer, error) {
 func (s *HTTPServer) Serve(listener net.Listener) error {
 	managed, err := s.lifecycle.manage(listener)
 	if err != nil {
+		if errors.Is(err, net.ErrClosed) {
+			return nil
+		}
 		return err
 	}
 	err = s.server.Serve(managed)

internal/proxy/lifecycle.go

@@ -237,6 +237,7 @@ type serverLifecycle struct {
 	mu       sync.Mutex
 	listener net.Listener
 	tracker  *connTracker
+	stopped  bool
 }
 
 func newServerLifecycle(max int) *serverLifecycle {
@@ -248,17 +249,26 @@ func (s *serverLifecycle) manage(listener net.Listener) (*managedListener, error
 		return nil, errors.New("proxy: nil listener")
 	}
 	s.mu.Lock()
-	defer s.mu.Unlock()
+	if s.stopped {
+		s.mu.Unlock()
+		// Shutdown may finish before the Serve goroutine is scheduled. Own
+		// and close this late listener, but never perform I/O under the lock.
+		_ = listener.Close()
+		return nil, net.ErrClosed
+	}
 	if s.listener != nil {
+		s.mu.Unlock()
 		return nil, errors.New("proxy: server is already serving")
 	}
 	s.listener = listener
+	s.mu.Unlock()
 	return &managedListener{Listener: listener, tracker: s.tracker}, nil
 }
 
 func (s *serverLifecycle) stopAccepting() error {
-	s.tracker.stopAccepting()
 	s.mu.Lock()
+	s.stopped = true
+	s.tracker.stopAccepting()
 	listener := s.listener
 	s.mu.Unlock()
 	if listener == nil {