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/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 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