fix(chromium): ceiling the CONNECT tunnels in flight and bound tunnel writes

pkg/modules/chromium/network_aggregate.go

@@ -11,6 +11,15 @@ import (
 // pathological page cannot grow the set without limit.
 const maxTrackedOrigins = 64
 
+// maxTrackedRequests bounds the request id to URL map. Entries are dropped as
+// soon as the request settles, so the map normally holds only what is in
+// flight, but a request that never reports a loading-finished or
+// loading-failed event never settles. Without a cap, a page that opens
+// requests it never resolves would grow the map for the whole conversion, at
+// the cost of one full response URL per entry. Losing an entry only costs the
+// heaviest-resource URL attribution for that request.
+const maxTrackedRequests = 1024
+
 // networkAggregate accumulates per-conversion network activity from Chromium
 // DevTools events. It is safe for concurrent use by the chromedp event listener
 // goroutine and the conversion goroutine that reads the snapshot afterwards.
@@ -60,8 +69,10 @@ func (a *networkAggregate) onResponseReceived(ev *network.EventResponseReceived)
 			a.origins[origin] = struct{}{}
 		}
 	}
+	if len(a.requestURLByID) < maxTrackedRequests {
 		a.requestURLByID[ev.RequestID] = ev.Response.URL
 	}
+}
 
 // onLoadingFinished records a successfully completed request and its size,
 // tracking the single heaviest resource.
@@ -81,6 +92,11 @@ func (a *networkAggregate) onLoadingFinished(ev *network.EventLoadingFinished) {
 		a.heaviestBytes = size
 		a.heaviestURL = a.requestURLByID[ev.RequestID]
 	}
+
+	// The request has settled and nothing reads its URL again. Dropping it
+	// keeps the map proportional to the requests in flight rather than to
+	// every request the page ever made.
+	delete(a.requestURLByID, ev.RequestID)
 }
 
 // onLoadingFailed records a request that failed to complete.
@@ -94,6 +110,9 @@ func (a *networkAggregate) onLoadingFailed(ev *network.EventLoadingFailed) {
 
 	a.requestCount++
 	a.failedCount++
+
+	// Settled, like a finished request: its URL is never read again.
+	delete(a.requestURLByID, ev.RequestID)
 }
 
 func (a *networkAggregate) snapshot() networkStats {

pkg/modules/chromium/network_aggregate_test.go

@@ -98,3 +98,86 @@ func TestNetworkAggregate_ConcurrentSafe(t *testing.T) {
 		t.Errorf("requestCount = %d, want 100", got)
 	}
 }
+
+// TestNetworkAggregate_SettledRequestsAreDropped covers the growth where every
+// response URL stayed in the map for the whole conversion even though nothing
+// reads it again once the request settles.
+func TestNetworkAggregate_SettledRequestsAreDropped(t *testing.T) {
+	a := newNetworkAggregate()
+
+	for i := range 500 {
+		id := network.RequestID(fmt.Sprintf("r%d", i))
+		a.onResponseReceived(&network.EventResponseReceived{
+			RequestID: id,
+			Response:  &network.Response{URL: fmt.Sprintf("https://host.example.com/%d", i)},
+		})
+
+		if i%2 == 0 {
+			a.onLoadingFinished(&network.EventLoadingFinished{RequestID: id, EncodedDataLength: 10})
+			continue
+		}
+
+		a.onLoadingFailed(&network.EventLoadingFailed{RequestID: id})
+	}
+
+	a.mu.Lock()
+	tracked := len(a.requestURLByID)
+	a.mu.Unlock()
+
+	if tracked != 0 {
+		t.Errorf("tracked requests = %d, want 0: settled requests must not be retained", tracked)
+	}
+
+	// The bookkeeping the map feeds must survive the pruning.
+	got := a.snapshot()
+	if got.requestCount != 500 {
+		t.Errorf("requestCount = %d, want 500", got.requestCount)
+	}
+	if got.failedCount != 250 {
+		t.Errorf("failedCount = %d, want 250", got.failedCount)
+	}
+}
+
+// TestNetworkAggregate_UnsettledRequestCap verifies the ceiling that applies
+// when requests never settle, which is the only way the map can still grow.
+func TestNetworkAggregate_UnsettledRequestCap(t *testing.T) {
+	a := newNetworkAggregate()
+
+	for i := range maxTrackedRequests + 500 {
+		a.onResponseReceived(&network.EventResponseReceived{
+			RequestID: network.RequestID(fmt.Sprintf("r%d", i)),
+			Response:  &network.Response{URL: fmt.Sprintf("https://host.example.com/%d", i)},
+		})
+	}
+
+	a.mu.Lock()
+	tracked := len(a.requestURLByID)
+	a.mu.Unlock()
+
+	if tracked != maxTrackedRequests {
+		t.Errorf("tracked requests = %d, want %d (capped)", tracked, maxTrackedRequests)
+	}
+}
+
+// TestNetworkAggregate_HeaviestURLSurvivesPruning guards the attribution the
+// map exists for: the URL must still be resolved before the entry is dropped.
+func TestNetworkAggregate_HeaviestURLSurvivesPruning(t *testing.T) {
+	a := newNetworkAggregate()
+
+	a.onResponseReceived(&network.EventResponseReceived{
+		RequestID: "small",
+		Response:  &network.Response{URL: "https://example.com/small.css"},
+	})
+	a.onLoadingFinished(&network.EventLoadingFinished{RequestID: "small", EncodedDataLength: 10})
+
+	a.onResponseReceived(&network.EventResponseReceived{
+		RequestID: "big",
+		Response:  &network.Response{URL: "https://example.com/big.png"},
+	})
+	a.onLoadingFinished(&network.EventLoadingFinished{RequestID: "big", EncodedDataLength: 4096})
+
+	got := a.snapshot()
+	if got.heaviestURL != "https://example.com/big.png" || got.heaviestBytes != 4096 {
+		t.Errorf("heaviest = (%q, %d), want (%q, 4096)", got.heaviestURL, got.heaviestBytes, "https://example.com/big.png")
+	}
+}

pkg/modules/chromium/pinning_proxy.go

@@ -11,6 +11,7 @@ import (
 	"net/netip"
 	"net/url"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/dlclark/regexp2"
@@ -57,6 +58,18 @@ type pinningProxy struct {
 	server   *http.Server
 	wg       sync.WaitGroup
 
+	// closing is closed by Stop to force in-flight CONNECT tunnels shut.
+	// [http.Server.Shutdown] cannot do it: net/http untracks a connection once
+	// a handler hijacks it, so a tunnel would otherwise outlive the proxy that
+	// created it. Recreated on every Start.
+	closing chan struct{}
+
+	// maxTunnels ceilings the CONNECT handlers in flight. Tests may lower it.
+	maxTunnels int64
+
+	// tunnels counts the CONNECT handlers in flight.
+	tunnels atomic.Int64
+
 	logger  *slog.Logger
 	started bool
 	mu      sync.Mutex
@@ -82,6 +95,7 @@ func newPinningProxy(allowList, denyList []*regexp2.Regexp, denyPrivateIPs, deny
 			dialer := &net.Dialer{Timeout: 10 * time.Second}
 			return dialer.DialContext(ctx, network, addr)
 		},
+		maxTunnels: maxConcurrentTunnels,
 	}
 
 	if enableEnvironmentProxy {
@@ -110,6 +124,7 @@ func (p *pinningProxy) Start(logger *slog.Logger) error {
 	}
 
 	p.listener = l
+	p.closing = make(chan struct{})
 	p.logger = logger.With(slog.String("logger", "pinning-proxy"))
 	p.server = &http.Server{
 		Handler: http.HandlerFunc(p.serveHTTP),
@@ -140,9 +155,18 @@ func (p *pinningProxy) Stop(logger *slog.Logger) error {
 		return nil
 	}
 	srv := p.server
+	closing := p.closing
+	p.closing = nil
 	p.started = false
 	p.mu.Unlock()
 
+	// Force in-flight tunnels shut before draining the server. Shutdown does
+	// not reach them, so a tunnel whose upstream never answers would otherwise
+	// survive the proxy, and with it every Chromium restart.
+	if closing != nil {
+		close(closing)
+	}
+
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
 
@@ -181,6 +205,25 @@ func (p *pinningProxy) serveHTTP(w http.ResponseWriter, req *http.Request) {
 // Chromium then negotiates TLS end-to-end with the original hostname in
 // SNI.
 func (p *pinningProxy) handleConnect(w http.ResponseWriter, req *http.Request) {
+	// A ceiling, not a tuning knob: it bounds what a tunnel that refuses to end
+	// can accumulate, whatever keeps it alive. [spliceIdleTimeout] ends a silent
+	// tunnel, but a peer trickling a byte just under it stays "active" forever,
+	// and a compromised renderer can hold the client side open to match.
+	//
+	// Chromium caps itself well below this. Its socket pool manager allows 128
+	// sockets per proxy chain for normal traffic plus 128 for WebSocket
+	// traffic, and every request Gotenberg's Chromium makes traverses this one
+	// proxy chain, so an honest browser cannot exceed 256 tunnels here. At
+	// double that, a real page never meets the ceiling and a hostile one stops
+	// at it.
+	if !p.acquireTunnel() {
+		p.logger.WarnContext(req.Context(), fmt.Sprintf("CONNECT to '%s' refused: %d tunnels already in flight", req.Host, p.maxTunnels))
+		http.Error(w, "too many tunnels", http.StatusServiceUnavailable)
+
+		return
+	}
+	defer p.releaseTunnel()
+
 	_, port, err := net.SplitHostPort(req.Host)
 	if err != nil {
 		http.Error(w, "bad CONNECT target", http.StatusBadRequest)
@@ -264,24 +307,142 @@ func (p *pinningProxy) handleConnect(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	// Splice bytes in both directions until either side closes.
+	p.mu.Lock()
-	var splice sync.WaitGroup
+	closing := p.closing
-	splice.Add(2)
+	p.mu.Unlock()
+
+	spliceTunnel(client, upstream, closing, spliceIdleTimeout)
+}
+
+// maxConcurrentTunnels is the default for [pinningProxy.maxTunnels]. See
+// [pinningProxy.handleConnect] for how the value is derived.
+const maxConcurrentTunnels = 512
+
+// acquireTunnel reserves a slot for one CONNECT handler, reporting false when
+// the proxy is already at [pinningProxy.maxTunnels]. The compare-and-swap loop
+// keeps the check and the increment atomic, so concurrent handlers cannot
+// overshoot the ceiling between them.
+func (p *pinningProxy) acquireTunnel() bool {
+	for {
+		current := p.tunnels.Load()
+		if current >= p.maxTunnels {
+			return false
+		}
+		if p.tunnels.CompareAndSwap(current, current+1) {
+			return true
+		}
+	}
+}
+
+// releaseTunnel returns a slot taken by [pinningProxy.acquireTunnel].
+func (p *pinningProxy) releaseTunnel() {
+	p.tunnels.Add(-1)
+}
+
+// spliceIdleTimeout bounds a CONNECT tunnel in which no byte has moved in
+// either direction.
+//
+// Nothing else bounds one. The hijacked connections carry no deadline: the
+// server clears the header read deadline once the request line is in, and
+// net.Dialer.Timeout only covers the connect. net/http also untracks a
+// connection once it is hijacked, so neither Server.Shutdown nor a Chromium
+// restart reaps it. Left alone, an upstream that accepts the tunnel and then
+// answers nothing holds two goroutines and two sockets until the process dies.
+//
+// Sized well above any legitimate pause between a request and its response, so
+// a slow origin is never cut off. A transfer that keeps making progress
+// refreshes the deadline and runs for as long as it needs.
+const spliceIdleTimeout = 2 * time.Minute
+
+// spliceTunnel copies bytes between the two ends of a CONNECT tunnel until
+// both directions finish, the tunnel sits idle for idleTimeout, or closing is
+// closed because the proxy is shutting down. Callers pass
+// [spliceIdleTimeout]; only tests shorten it.
+//
+// Each direction half-closes its destination once its source reaches EOF, so a
+// peer that waits for the request to end before answering still sees the EOF.
+// Idleness is tracked across both directions rather than per direction: the
+// client sends nothing for the length of a download, and half-closing its write
+// side then would tell the origin the client had gone away.
+func spliceTunnel(client, upstream net.Conn, closing <-chan struct{}, idleTimeout time.Duration) {
+	var lastActivity atomic.Int64
+	lastActivity.Store(time.Now().UnixNano())
+
+	var wg sync.WaitGroup
+	wg.Add(2)
 	go func() {
-		defer splice.Done()
+		defer wg.Done()
-		_, _ = io.Copy(upstream, client)
+		copyTracking(upstream, client, &lastActivity, idleTimeout)
 		if cw, ok := upstream.(interface{ CloseWrite() error }); ok {
 			_ = cw.CloseWrite()
 		}
 	}()
 	go func() {
-		defer splice.Done()
+		defer wg.Done()
-		_, _ = io.Copy(client, upstream)
+		copyTracking(client, upstream, &lastActivity, idleTimeout)
 		if cw, ok := client.(interface{ CloseWrite() error }); ok {
 			_ = cw.CloseWrite()
 		}
 	}()
-	splice.Wait()
+
+	done := make(chan struct{})
+	go func() {
+		wg.Wait()
+		close(done)
+	}()
+
+	ticker := time.NewTicker(idleTimeout / 4)
+	defer ticker.Stop()
+
+	for {
+		select {
+		case <-done:
+			return
+		case <-closing:
+		case <-ticker.C:
+			if time.Since(time.Unix(0, lastActivity.Load())) < idleTimeout {
+				continue
+			}
+		}
+
+		// Closing both ends unblocks whichever copy is still reading. The
+		// caller's own deferred Close calls then become no-ops.
+		_ = client.Close()
+		_ = upstream.Close()
+		<-done
+
+		return
+	}
+}
+
+// copyTracking copies src into dst, recording the time of every chunk that
+// moves so [spliceTunnel] can tell a busy tunnel from an idle one.
+func copyTracking(dst, src net.Conn, lastActivity *atomic.Int64, writeTimeout time.Duration) {
+	buf := make([]byte, 32*1024)
+
+	for {
+		n, readErr := src.Read(buf)
+		if n > 0 {
+			lastActivity.Store(time.Now().UnixNano())
+
+			// Bound the write. A destination that has gone away accepts the
+			// first chunk into its send buffer and only fails on the next one,
+			// so without a deadline this direction keeps a dead tunnel alive
+			// for one more chunk. A destination that stops reading altogether
+			// would block here forever.
+			_ = dst.SetWriteDeadline(time.Now().Add(writeTimeout))
+
+			_, writeErr := dst.Write(buf[:n])
+			if writeErr != nil {
+				return
+			}
+
+			lastActivity.Store(time.Now().UnixNano())
+		}
+		if readErr != nil {
+			return
+		}
+	}
 }
 
 // handleForward handles plain HTTP requests sent to the proxy as absolute

pkg/modules/chromium/pinning_proxy_test.go

@@ -898,3 +898,280 @@ func TestPinningProxy_StopIdempotent(t *testing.T) {
 		t.Fatalf("second Stop on stopped proxy: %v", err)
 	}
 }
+
+// tcpPair returns the two ends of a connected loopback TCP connection. Both
+// ends are closed when the test finishes.
+func tcpPair(t *testing.T) (net.Conn, net.Conn) {
+	t.Helper()
+
+	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	defer func() { _ = listener.Close() }()
+
+	type accepted struct {
+		conn net.Conn
+		err  error
+	}
+
+	acceptChan := make(chan accepted, 1)
+	go func() {
+		conn, acceptErr := listener.Accept()
+		acceptChan <- accepted{conn: conn, err: acceptErr}
+	}()
+
+	dialed, err := net.Dial("tcp", listener.Addr().String())
+	if err != nil {
+		t.Fatalf("dial: %v", err)
+	}
+
+	res := <-acceptChan
+	if res.err != nil {
+		t.Fatalf("accept: %v", res.err)
+	}
+
+	t.Cleanup(func() {
+		_ = dialed.Close()
+		_ = res.conn.Close()
+	})
+
+	return dialed, res.conn
+}
+
+// TestSpliceTunnel_IdleTunnelIsClosed covers the leak where an upstream that
+// accepted a CONNECT tunnel and then never spoke pinned both splice goroutines
+// and both sockets for the lifetime of the process. A hijacked connection
+// carries no deadline and net/http stops tracking it, so the idle bound in
+// spliceTunnel is the only thing that ends such a tunnel.
+func TestSpliceTunnel_IdleTunnelIsClosed(t *testing.T) {
+	// The peers are kept open by the pair's cleanup: the tunnel is silent, not
+	// finished.
+	client, _ := tcpPair(t)
+	upstream, _ := tcpPair(t)
+
+	done := make(chan struct{})
+	go func() {
+		spliceTunnel(client, upstream, nil, 100*time.Millisecond)
+		close(done)
+	}()
+
+	select {
+	case <-done:
+	case <-time.After(10 * time.Second):
+		t.Fatal("spliceTunnel did not return on an idle tunnel")
+	}
+}
+
+// TestSpliceTunnel_ClosingShutsTunnelDown verifies that stopping the proxy
+// reaps in-flight tunnels. http.Server.Shutdown cannot: it stops tracking a
+// connection once a handler hijacks it, so without this signal a tunnel would
+// outlive the proxy and every Chromium restart after it.
+func TestSpliceTunnel_ClosingShutsTunnelDown(t *testing.T) {
+	client, _ := tcpPair(t)
+	upstream, _ := tcpPair(t)
+
+	closing := make(chan struct{})
+	done := make(chan struct{})
+
+	go func() {
+		// An idle timeout far beyond the test: only closing can end this.
+		spliceTunnel(client, upstream, closing, time.Hour)
+		close(done)
+	}()
+
+	close(closing)
+
+	select {
+	case <-done:
+	case <-time.After(10 * time.Second):
+		t.Fatal("spliceTunnel did not return when the proxy shut down")
+	}
+}
+
+// TestSpliceTunnel_ActiveTransferOutlivesIdleTimeout guards the idle bound
+// against cutting a healthy transfer. Idleness is tracked across both
+// directions, so a download that keeps making progress must survive well past
+// the timeout even though the client sends nothing throughout.
+func TestSpliceTunnel_ActiveTransferOutlivesIdleTimeout(t *testing.T) {
+	const (
+		idleTimeout = 100 * time.Millisecond
+		chunks      = 10
+		interval    = 30 * time.Millisecond
+	)
+
+	client, clientPeer := tcpPair(t)
+	upstream, upstreamPeer := tcpPair(t)
+
+	done := make(chan struct{})
+	go func() {
+		spliceTunnel(client, upstream, nil, idleTimeout)
+		close(done)
+	}()
+
+	// Trickle a response for well over the idle timeout, then finish.
+	go func() {
+		for range chunks {
+			_, _ = upstreamPeer.Write([]byte("x"))
+			time.Sleep(interval)
+		}
+		_ = upstreamPeer.Close()
+	}()
+
+	received := 0
+	buf := make([]byte, chunks)
+
+	for received < chunks {
+		err := clientPeer.SetReadDeadline(time.Now().Add(10 * time.Second))
+		if err != nil {
+			t.Fatalf("set read deadline: %v", err)
+		}
+
+		n, readErr := clientPeer.Read(buf)
+		received += n
+		if readErr != nil {
+			break
+		}
+	}
+
+	if received != chunks {
+		t.Fatalf("received %d bytes, want %d: the tunnel was cut while still transferring", received, chunks)
+	}
+
+	select {
+	case <-done:
+	case <-time.After(10 * time.Second):
+		t.Fatal("spliceTunnel did not return after the upstream closed")
+	}
+}
+
+// TestPinningProxy_CONNECT_TunnelCeiling verifies the ceiling that bounds what
+// tunnels refusing to end can accumulate. The idle bound cannot cover a peer
+// that trickles just under it, so the count is what stops the growth.
+func TestPinningProxy_CONNECT_TunnelCeiling(t *testing.T) {
+	// An upstream that accepts and then says nothing: the tunnel stays open.
+	upstreamAddr, stop := newRawTCPServer(t, func(c net.Conn) {
+		<-make(chan struct{})
+	})
+	t.Cleanup(stop)
+
+	p := newPinningProxy(nil, nil, false, false, false)
+	p.maxTunnels = 1
+	p.decide = func(_ context.Context, _ string, _, _ []*regexp2.Regexp, _ time.Time) (gotenberg.OutboundDecision, error) {
+		return gotenberg.OutboundDecision{Pinned: []netip.Addr{netip.MustParseAddr("127.0.0.1")}}, nil
+	}
+	p.dialPinned = func(_ context.Context, network string, _ []netip.Addr, _ string) (net.Conn, error) {
+		return net.Dial(network, upstreamAddr)
+	}
+	proxyURL := newProxyForTest(t, p)
+	proxyAddr := strings.TrimPrefix(proxyURL, "http://")
+
+	connect := func(t *testing.T) *bufio.Reader {
+		t.Helper()
+
+		conn, err := net.Dial("tcp", proxyAddr)
+		if err != nil {
+			t.Fatalf("dial proxy: %v", err)
+		}
+		t.Cleanup(func() { _ = conn.Close() })
+
+		err = conn.SetDeadline(time.Now().Add(10 * time.Second))
+		if err != nil {
+			t.Fatalf("set deadline: %v", err)
+		}
+
+		_, err = fmt.Fprintf(conn, "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n\r\n")
+		if err != nil {
+			t.Fatalf("write CONNECT: %v", err)
+		}
+
+		return bufio.NewReader(conn)
+	}
+
+	first := connect(t)
+	statusLine, err := first.ReadString('\n')
+	if err != nil {
+		t.Fatalf("read first status: %v", err)
+	}
+	if !strings.Contains(statusLine, " 200 ") {
+		t.Fatalf("first CONNECT status = %q, want 200", statusLine)
+	}
+
+	// The first tunnel now holds the only slot.
+	second := connect(t)
+	resp, err := http.ReadResponse(second, nil)
+	if err != nil {
+		t.Fatalf("read second response: %v", err)
+	}
+	defer func() { _ = resp.Body.Close() }()
+
+	if resp.StatusCode != http.StatusServiceUnavailable {
+		t.Fatalf("second CONNECT status = %d, want %d", resp.StatusCode, http.StatusServiceUnavailable)
+	}
+
+	if got := p.tunnels.Load(); got != 1 {
+		t.Errorf("tunnels in flight = %d, want 1: a refused CONNECT must not consume a slot", got)
+	}
+}
+
+// TestPinningProxy_TunnelSlotIsReleased verifies a completed tunnel gives its
+// slot back, so the ceiling bounds concurrency rather than lifetime totals.
+func TestPinningProxy_TunnelSlotIsReleased(t *testing.T) {
+	upstreamAddr, stop := newRawTCPServer(t, func(c net.Conn) {
+		defer c.Close()
+		_, _ = c.Write([]byte("HI"))
+	})
+	t.Cleanup(stop)
+
+	p := newPinningProxy(nil, nil, false, false, false)
+	p.maxTunnels = 1
+	p.decide = func(_ context.Context, _ string, _, _ []*regexp2.Regexp, _ time.Time) (gotenberg.OutboundDecision, error) {
+		return gotenberg.OutboundDecision{Pinned: []netip.Addr{netip.MustParseAddr("127.0.0.1")}}, nil
+	}
+	p.dialPinned = func(_ context.Context, network string, _ []netip.Addr, _ string) (net.Conn, error) {
+		return net.Dial(network, upstreamAddr)
+	}
+	proxyURL := newProxyForTest(t, p)
+	proxyAddr := strings.TrimPrefix(proxyURL, "http://")
+
+	for attempt := range 3 {
+		conn, err := net.Dial("tcp", proxyAddr)
+		if err != nil {
+			t.Fatalf("attempt %d dial proxy: %v", attempt, err)
+		}
+
+		err = conn.SetDeadline(time.Now().Add(10 * time.Second))
+		if err != nil {
+			t.Fatalf("attempt %d set deadline: %v", attempt, err)
+		}
+
+		_, err = fmt.Fprintf(conn, "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n\r\n")
+		if err != nil {
+			t.Fatalf("attempt %d write CONNECT: %v", attempt, err)
+		}
+
+		br := bufio.NewReader(conn)
+		statusLine, err := br.ReadString('\n')
+		if err != nil {
+			t.Fatalf("attempt %d read status: %v", attempt, err)
+		}
+		if !strings.Contains(statusLine, " 200 ") {
+			t.Fatalf("attempt %d CONNECT status = %q, want 200: the slot was not released", attempt, statusLine)
+		}
+
+		// Drain until the upstream's close ends the tunnel, then release it.
+		_, _ = io.ReadAll(br)
+		_ = conn.Close()
+
+		// The handler returns just after the splice ends.
+		for range 100 {
+			if p.tunnels.Load() == 0 {
+				break
+			}
+			time.Sleep(20 * time.Millisecond)
+		}
+		if got := p.tunnels.Load(); got != 0 {
+			t.Fatalf("attempt %d: tunnels in flight = %d, want 0", attempt, got)
+		}
+	}
+}

pkg/modules/libreoffice/api/libreoffice.go

@@ -68,6 +68,59 @@ func (p *libreOfficeProcess) Start(logger *slog.Logger) error {
 
 	userProfileDirPath := p.fs.NewDirPath()
 
+	var (
+		cmd     *gotenberg.Cmd
+		success bool
+	)
+
+	// Registered here, right after the proxy starts listening, so that every
+	// failure below tears it down. A return between the proxy start and this
+	// point strands it: its listener stays bound, its Serve goroutine and HTTP
+	// client stay alive, and p.proxy is only assigned on success, so nothing
+	// could ever reach it to stop it. exec.Cmd.Start fails precisely under fd
+	// or memory pressure, and the supervisor retries the launch on the next
+	// request, so each stranded proxy compounds the condition that caused it.
+	defer func() {
+		if success {
+			p.cfgMu.Lock()
+			defer p.cfgMu.Unlock()
+
+			p.socketPort = port
+			p.userProfileDirPath = userProfileDirPath
+			p.cmd = cmd
+			p.proxy = proxy
+			p.isStarted.Store(true)
+
+			return
+		}
+
+		// LibreOffice failed to start; tear the proxy down too.
+		stopErr := proxy.Stop(context.Background())
+		if stopErr != nil {
+			logger.WarnContext(context.Background(), fmt.Sprintf("stop LibreOffice outbound proxy after failed start: %s", stopErr))
+		}
+
+		// Let's make sure the process is killed. It is nil when the failure
+		// happened before the command was built.
+		if cmd != nil {
+			killErr := cmd.Kill()
+			if killErr != nil {
+				logger.DebugContext(context.Background(), fmt.Sprintf("kill LibreOffice process: %v", killErr))
+			}
+		}
+
+		// And the user profile directory is deleted. It may never have been
+		// created, which RemoveAll reports as success.
+		removeErr := os.RemoveAll(userProfileDirPath)
+		if removeErr != nil {
+			logger.ErrorContext(context.Background(), fmt.Sprintf("remove LibreOffice's user profile directory: %v", removeErr))
+
+			return
+		}
+
+		logger.DebugContext(context.Background(), fmt.Sprintf("'%s' LibreOffice's user profile directory removed", userProfileDirPath))
+	}()
+
 	// LibreOffice fetches external content (OOXML images via
 	// TargetMode=External, RTF INCLUDEPICTURE, ODT linked images) inside
 	// its own libcurl. The profile config routes those fetches through the
@@ -75,7 +128,6 @@ func (p *libreOfficeProcess) Start(logger *slog.Logger) error {
 	// blocks content linked from untrusted locations so absolute-path
 	// (file://) and direct fetches are dropped at the source.
 	if err := writeSofficeProfileConfig(userProfileDirPath, proxy.Addr()); err != nil {
-		_ = proxy.Stop(context.Background())
 		return fmt.Errorf("write soffice profile config: %w", err)
 	}
 	sofficeEnv := sofficeProxyEnv(os.Environ(), proxy.Addr())
@@ -95,9 +147,8 @@ func (p *libreOfficeProcess) Start(logger *slog.Logger) error {
 	ctx, cancel := context.WithTimeout(context.Background(), p.arguments.startTimeout)
 	defer cancel()
 
-	cmd, err := gotenberg.CommandContext(ctx, logger, p.arguments.binPath, args...)
+	cmd, err = gotenberg.CommandContext(ctx, logger, p.arguments.binPath, args...)
 	if err != nil {
-		_ = proxy.Stop(context.Background())
 		return fmt.Errorf("create LibreOffice command: %w", err)
 	}
 	cmd.SetEnv(sofficeEnv)
@@ -106,7 +157,6 @@ func (p *libreOfficeProcess) Start(logger *slog.Logger) error {
 	// able to run as a daemon.
 	exitCode, err := cmd.Exec()
 	if err != nil && exitCode != 81 {
-		_ = proxy.Stop(context.Background())
 		return fmt.Errorf("execute LibreOffice: %w", err)
 	}
 
@@ -155,43 +205,6 @@ func (p *libreOfficeProcess) Start(logger *slog.Logger) error {
 		}
 	}()
 
-	var success bool
-
-	defer func() {
-		if success {
-			p.cfgMu.Lock()
-			defer p.cfgMu.Unlock()
-
-			p.socketPort = port
-			p.userProfileDirPath = userProfileDirPath
-			p.cmd = cmd
-			p.proxy = proxy
-			p.isStarted.Store(true)
-
-			return
-		}
-
-		// LibreOffice failed to start; tear the proxy down too.
-		stopErr := proxy.Stop(context.Background())
-		if stopErr != nil {
-			logger.WarnContext(context.Background(), fmt.Sprintf("stop LibreOffice outbound proxy after failed start: %s", stopErr))
-		}
-
-		// Let's make sure the process is killed.
-		err = cmd.Kill()
-		if err != nil {
-			logger.DebugContext(context.Background(), fmt.Sprintf("kill LibreOffice process: %v", err))
-		}
-
-		// And the user profile directory is deleted.
-		err = os.RemoveAll(userProfileDirPath)
-		if err != nil {
-			logger.ErrorContext(context.Background(), fmt.Sprintf("remove LibreOffice's user profile directory: %v", err))
-		}
-
-		logger.DebugContext(context.Background(), fmt.Sprintf("'%s' LibreOffice's user profile directory removed", userProfileDirPath))
-	}()
-
 	logger.DebugContext(context.Background(), "waiting for the LibreOffice socket to be available...")
 
 	for {

pkg/modules/webhook/client.go

@@ -143,13 +143,12 @@ func (c client) send(ctx context.Context, body io.Reader, headers map[string]str
 		return fmt.Errorf("send '%s' request to '%s': %w", method, url, err)
 	}
 
-	if resp.StatusCode >= http.StatusBadRequest {
+	// Registered before the status check below. [retryablehttp.Client.Do] hands
-		err := fmt.Errorf("send '%s' request to '%s': got status: '%s'", method, url, resp.Status)
+	// back a live body for a status it does not retry, which is every 4xx but
-		span.RecordError(err)
+	// 429, so returning early without closing it strands the connection and the
-		span.SetStatus(codes.Error, err.Error())
+	// transport goroutines that serve it for the lifetime of the process. The
-		return err
+	// transport is built per delivery in [gotenberg.NewOutboundHttpClient], so
-	}
+	// nothing reclaims it later either.
-
 	defer func() {
 		err := resp.Body.Close()
 		if err != nil {
@@ -157,6 +156,13 @@ func (c client) send(ctx context.Context, body io.Reader, headers map[string]str
 		}
 	}()
 
+	if resp.StatusCode >= http.StatusBadRequest {
+		err := fmt.Errorf("send '%s' request to '%s': got status: '%s'", method, url, resp.Status)
+		span.RecordError(err)
+		span.SetStatus(codes.Error, err.Error())
+		return err
+	}
+
 	// Last piece for calculating the latency.
 	finishTime := time.Now()