From 7ae5230287636242ad1bb5fde9e87662ac475f59 Mon Sep 17 00:00:00 2001 From: Rafael Garcia Date: Thu, 11 Jun 2026 10:17:45 -0400 Subject: [PATCH 1/3] fix(chromedriverproxy,e2e): BiDi WebSocket scheme over a TLS-terminating ingress MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ChromeDriver WebDriver-BiDi over a TLS-terminating ingress (the hypeman :9224 ingress; serves https/wss) was broken because the BiDi WebSocket URLs used the wrong scheme. Docker is plaintext (http/ws) so it never showed there. Two shared-code fixes: 1. server/e2e/container.go: - ChromeDriverAddr only stripped "http://", so for an "https://host:9224" ChromeDriver URL it returned "https://host:9224" with the scheme still attached. - ChromeDriverWSURL hardcoded "ws://", producing the malformed "ws://https://host:9224/session". Fix: strip any scheme in ChromeDriverAddr; pick ws:// vs wss:// in ChromeDriverWSURL based on whether the ChromeDriver URL is https. 2. server/lib/chromedriverproxy/proxy.go: rewriteWebSocketURL kept chromedriver's ws:// scheme, so the webSocketUrl returned from POST /session was ws://host:9224/... — unreachable through the TLS ingress. Fix: a new clientWSScheme(r) helper returns wss when the request arrives with X-Forwarded-Proto: https (set by the Caddy ingress) or r.TLS != nil, and rewriteWebSocketURL applies it. Docker (plaintext, no forwarded proto) keeps ws://. Plus a regression test TestHandler_PostSession_WSSchemeFromForwardedProto. Validated in kernel-images-private #237: all 5 TestBidi* (Selenium, Puppeteer, HTTPSession, Vibium, WebSocket) now pass against the staging hypeman host over Tailscale. Co-Authored-By: Claude Opus 4.8 (1M context) --- server/e2e/container.go | 23 ++++++++++++---- server/lib/chromedriverproxy/proxy.go | 30 ++++++++++++++++++-- server/lib/chromedriverproxy/proxy_test.go | 32 ++++++++++++++++++++++ 3 files changed, 77 insertions(+), 8 deletions(-) diff --git a/server/e2e/container.go b/server/e2e/container.go index 6b643fdc..11077263 100644 --- a/server/e2e/container.go +++ b/server/e2e/container.go @@ -76,16 +76,27 @@ func (c *TestContainer) ChromeDriverURL() string { } // ChromeDriverAddr returns the host:port for the instance's ChromeDriver proxy, -// derived from ChromeDriverURL (without scheme). Useful for substring assertions -// on proxy-rewritten URLs. +// derived from ChromeDriverURL (scheme stripped). Useful for substring +// assertions on proxy-rewritten URLs. Handles both http:// (docker) and +// https:// (hypeman ingress) ChromeDriver URLs. func (c *TestContainer) ChromeDriverAddr() string { - return strings.TrimPrefix(c.backend.ChromeDriverURL(), "http://") + u := c.backend.ChromeDriverURL() + if i := strings.Index(u, "://"); i >= 0 { + return u[i+3:] + } + return u } -// ChromeDriverWSURL returns the WebSocket URL (ws://host:port/path) for the -// instance's ChromeDriver proxy. path should include a leading slash. +// ChromeDriverWSURL returns the WebSocket URL for the instance's ChromeDriver +// proxy. The scheme matches the ChromeDriver endpoint's transport: wss:// when +// it's served over TLS (the hypeman ingress on :9224), ws:// otherwise (docker). +// path should include a leading slash. func (c *TestContainer) ChromeDriverWSURL(path string) string { - return "ws://" + c.ChromeDriverAddr() + path + scheme := "ws" + if strings.HasPrefix(c.backend.ChromeDriverURL(), "https://") { + scheme = "wss" + } + return scheme + "://" + c.ChromeDriverAddr() + path } // APIClient creates an OpenAPI client for this instance's API. diff --git a/server/lib/chromedriverproxy/proxy.go b/server/lib/chromedriverproxy/proxy.go index e481dbb3..289ed1c0 100644 --- a/server/lib/chromedriverproxy/proxy.go +++ b/server/lib/chromedriverproxy/proxy.go @@ -170,7 +170,7 @@ func handleCreateSession(w http.ResponseWriter, r *http.Request, logger *slog.Lo return } - respBody = rewriteWebSocketURL(respBody, r.Host, logger) + respBody = rewriteWebSocketURL(respBody, r.Host, clientWSScheme(r), logger) for k, vv := range resp.Header { for _, v := range vv { @@ -182,6 +182,25 @@ func handleCreateSession(w http.ResponseWriter, r *http.Request, logger *slog.Lo w.Write(respBody) } +// clientWSScheme returns the WebSocket scheme the external client must use to +// reach this proxy. A TLS-terminating ingress (e.g. the hypeman :9224 ingress) +// forwards plaintext HTTP to the proxy but sets X-Forwarded-Proto: https, so +// the rewritten webSocketUrl has to be wss:// — a ws:// URL would be +// unreachable through the TLS listener. Returns "" (keep upstream scheme) when +// there's no TLS indication (e.g. the docker plaintext path). +func clientWSScheme(r *http.Request) string { + if proto := r.Header.Get("X-Forwarded-Proto"); proto != "" { + if strings.EqualFold(proto, "https") || strings.EqualFold(proto, "wss") { + return "wss" + } + return "ws" + } + if r.TLS != nil { + return "wss" + } + return "" +} + // rewriteWebSocketURL rewrites `value.capabilities.webSocketUrl` in a // WebDriver new-session response to point at this proxy. // @@ -191,7 +210,7 @@ func handleCreateSession(w http.ResponseWriter, r *http.Request, logger *slog.Lo // Example: // // ws://127.0.0.1:9225/session/abc -> ws://proxy-host:9224/session/abc -func rewriteWebSocketURL(body []byte, proxyHost string, logger *slog.Logger) []byte { +func rewriteWebSocketURL(body []byte, proxyHost, scheme string, logger *slog.Logger) []byte { var respPayload map[string]interface{} if err := json.Unmarshal(body, &respPayload); err != nil { return body @@ -215,6 +234,13 @@ func rewriteWebSocketURL(body []byte, proxyHost string, logger *slog.Logger) []b return body } parsed.Host = proxyHost + // Match the external transport. Behind a TLS-terminating ingress the client + // reaches this proxy over wss://, so a ws:// webSocketUrl (chromedriver's + // default) would be unreachable; clientWSScheme upgrades it from + // X-Forwarded-Proto. + if scheme != "" { + parsed.Scheme = scheme + } caps["webSocketUrl"] = parsed.String() out, err := json.Marshal(respPayload) diff --git a/server/lib/chromedriverproxy/proxy_test.go b/server/lib/chromedriverproxy/proxy_test.go index 53528c9e..4b129bfc 100644 --- a/server/lib/chromedriverproxy/proxy_test.go +++ b/server/lib/chromedriverproxy/proxy_test.go @@ -126,6 +126,38 @@ func TestHandler_PostSession_InjectsDebuggerAddress(t *testing.T) { "webSocketUrl in capabilities should be rewritten to proxy address") } +// TestHandler_PostSession_WSSchemeFromForwardedProto verifies that behind a +// TLS-terminating ingress (X-Forwarded-Proto: https) the rewritten +// webSocketUrl is wss:// — a ws:// URL would be unreachable through the TLS +// listener (this is what broke the BiDi tests on the hypeman :9224 ingress). +func TestHandler_PostSession_WSSchemeFromForwardedProto(t *testing.T) { + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"value":{"sessionId":"abc123","capabilities":{"webSocketUrl":"ws://127.0.0.1:9225/session/abc123"}}}`)) + })) + defer backend.Close() + + backendURL, _ := url.Parse(backend.URL) + handler := Handler(silentLogger(), testOptions(backendURL.Host, "127.0.0.1:9911")) + + req := httptest.NewRequest(http.MethodPost, "/session", strings.NewReader(`{"capabilities":{}}`)) + req.Host = "inst.dev-yul-hypeman-1.kernel.sh:9224" + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Forwarded-Proto", "https") + rec := httptest.NewRecorder() + + handler.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + + var respBody map[string]interface{} + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &respBody)) + value := respBody["value"].(map[string]interface{}) + respCaps := value["capabilities"].(map[string]interface{}) + assert.Equal(t, "wss://inst.dev-yul-hypeman-1.kernel.sh:9224/session/abc123", respCaps["webSocketUrl"], + "webSocketUrl must be wss:// when X-Forwarded-Proto is https") +} + // TestHandler_RewritesHostAndStripsOrigin is a regression test for the // ChromeDriver "Host header or origin header ... is not whitelisted or // localhost" HTTP 500. ChromeDriver (Chrome 111+) rejects requests whose From e4d51ece5ced9a0899c750989663e17801f07f17 Mon Sep 17 00:00:00 2001 From: Rafael Garcia Date: Thu, 11 Jun 2026 10:39:45 -0400 Subject: [PATCH 2/3] e2e: fetch CDP /json/version over https on a TLS-terminating ingress fetchBrowserWebSocketURL hardcoded http:// for the CDP /json/version probe, so it failed against a TLS-terminating CDP ingress (an http:// request hits a TLS listener and errors); plaintext (docker) works. Derive the HTTP scheme from the CDP URL's transport (wss -> https, ws -> http), matching the ChromeDriver accessor scheme handling in this PR. Co-Authored-By: Claude Opus 4.8 (1M context) --- server/e2e/e2e_cdp_reconnect_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/server/e2e/e2e_cdp_reconnect_test.go b/server/e2e/e2e_cdp_reconnect_test.go index 7c1ed226..687990db 100644 --- a/server/e2e/e2e_cdp_reconnect_test.go +++ b/server/e2e/e2e_cdp_reconnect_test.go @@ -10,6 +10,7 @@ import ( "net/url" "os/exec" "path/filepath" + "strings" "sync" "testing" "time" @@ -456,7 +457,14 @@ func touchContainerFile(ctx context.Context, client *instanceoapi.ClientWithResp } func fetchBrowserWebSocketURL(ctx context.Context, c *TestContainer) (string, error) { - versionURL := fmt.Sprintf("http://%s/json/version", c.CDPAddr()) + // The CDP endpoint is served over TLS behind the hypeman :9222 ingress + // (wss), so /json/version must be fetched over https there; docker is + // plaintext (ws/http). Derive the HTTP scheme from the CDP URL's transport. + scheme := "http" + if strings.HasPrefix(c.CDPURL(), "wss://") { + scheme = "https" + } + versionURL := fmt.Sprintf("%s://%s/json/version", scheme, c.CDPAddr()) req, err := http.NewRequestWithContext(ctx, http.MethodGet, versionURL, nil) if err != nil { return "", err From ae8abde84068b78228a31bb49e41c991afec04f2 Mon Sep 17 00:00:00 2001 From: Rafael Garcia Date: Tue, 16 Jun 2026 14:29:40 -0400 Subject: [PATCH 3/3] chromedriverproxy: handle comma-separated X-Forwarded-Proto MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Forwarded-Proto can be a comma-separated list ("https, http") when a request traverses multiple proxies. clientWSScheme used the raw header value, so it wouldn't match "https" and fell back to ws:// — silently reintroducing the TLS-ingress bug this PR fixes. Take the first (client-facing) token before comparing. Adds a "https, http" case to the regression test. Co-Authored-By: Claude Opus 4.8 (1M context) --- server/lib/chromedriverproxy/proxy.go | 7 +++ server/lib/chromedriverproxy/proxy_test.go | 52 ++++++++++++---------- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/server/lib/chromedriverproxy/proxy.go b/server/lib/chromedriverproxy/proxy.go index 289ed1c0..0b83a5c7 100644 --- a/server/lib/chromedriverproxy/proxy.go +++ b/server/lib/chromedriverproxy/proxy.go @@ -190,6 +190,13 @@ func handleCreateSession(w http.ResponseWriter, r *http.Request, logger *slog.Lo // there's no TLS indication (e.g. the docker plaintext path). func clientWSScheme(r *http.Request) string { if proto := r.Header.Get("X-Forwarded-Proto"); proto != "" { + // X-Forwarded-Proto may be a comma-separated list when the request + // traverses multiple proxies (e.g. "https, http"); the first value is + // the original client-facing scheme. + if i := strings.IndexByte(proto, ','); i >= 0 { + proto = proto[:i] + } + proto = strings.TrimSpace(proto) if strings.EqualFold(proto, "https") || strings.EqualFold(proto, "wss") { return "wss" } diff --git a/server/lib/chromedriverproxy/proxy_test.go b/server/lib/chromedriverproxy/proxy_test.go index 4b129bfc..7346169c 100644 --- a/server/lib/chromedriverproxy/proxy_test.go +++ b/server/lib/chromedriverproxy/proxy_test.go @@ -131,31 +131,37 @@ func TestHandler_PostSession_InjectsDebuggerAddress(t *testing.T) { // webSocketUrl is wss:// — a ws:// URL would be unreachable through the TLS // listener (this is what broke the BiDi tests on the hypeman :9224 ingress). func TestHandler_PostSession_WSSchemeFromForwardedProto(t *testing.T) { - backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - w.Write([]byte(`{"value":{"sessionId":"abc123","capabilities":{"webSocketUrl":"ws://127.0.0.1:9225/session/abc123"}}}`)) - })) - defer backend.Close() - - backendURL, _ := url.Parse(backend.URL) - handler := Handler(silentLogger(), testOptions(backendURL.Host, "127.0.0.1:9911")) - - req := httptest.NewRequest(http.MethodPost, "/session", strings.NewReader(`{"capabilities":{}}`)) - req.Host = "inst.dev-yul-hypeman-1.kernel.sh:9224" - req.Header.Set("Content-Type", "application/json") - req.Header.Set("X-Forwarded-Proto", "https") - rec := httptest.NewRecorder() + // "https" plus the multi-proxy comma-separated form ("https, http"), where + // only the first (client-facing) value should decide the scheme. + for _, proto := range []string{"https", "https, http"} { + t.Run(proto, func(t *testing.T) { + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"value":{"sessionId":"abc123","capabilities":{"webSocketUrl":"ws://127.0.0.1:9225/session/abc123"}}}`)) + })) + defer backend.Close() + + backendURL, _ := url.Parse(backend.URL) + handler := Handler(silentLogger(), testOptions(backendURL.Host, "127.0.0.1:9911")) + + req := httptest.NewRequest(http.MethodPost, "/session", strings.NewReader(`{"capabilities":{}}`)) + req.Host = "inst.dev-yul-hypeman-1.kernel.sh:9224" + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Forwarded-Proto", proto) + rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - require.Equal(t, http.StatusOK, rec.Code) + handler.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) - var respBody map[string]interface{} - require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &respBody)) - value := respBody["value"].(map[string]interface{}) - respCaps := value["capabilities"].(map[string]interface{}) - assert.Equal(t, "wss://inst.dev-yul-hypeman-1.kernel.sh:9224/session/abc123", respCaps["webSocketUrl"], - "webSocketUrl must be wss:// when X-Forwarded-Proto is https") + var respBody map[string]interface{} + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &respBody)) + value := respBody["value"].(map[string]interface{}) + respCaps := value["capabilities"].(map[string]interface{}) + assert.Equal(t, "wss://inst.dev-yul-hypeman-1.kernel.sh:9224/session/abc123", respCaps["webSocketUrl"], + "webSocketUrl must be wss:// when X-Forwarded-Proto's first value is https (got %q)", proto) + }) + } } // TestHandler_RewritesHostAndStripsOrigin is a regression test for the