From ccb2edc3b829c8b617976cf1152066f05be38ba6 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 22 Apr 2026 04:46:13 +0100 Subject: [PATCH] Require explicit websocket origin continuity --- .../v6/internal/subsystems/agent-lifecycle.md | 7 +++ .../v6/internal/subsystems/api-contracts.md | 7 ++- internal/agentexec/server.go | 30 +-------- internal/agentexec/server_coverage_test.go | 22 +++---- internal/agentexec/server_websocket_test.go | 53 +++++++++++++--- ..._handlers_investigation_additional_test.go | 16 ++++- internal/api/contract_test.go | 4 +- internal/api/router_integration_test.go | 20 +++++- internal/api/security_regression_test.go | 20 +++--- .../api/websocket_origin_security_test.go | 24 +++++++ internal/hostagent/agent_new_test.go | 47 ++++++++++++++ internal/hostagent/command_client_test.go | 53 ++++++++++++++++ internal/hostagent/commands.go | 13 +++- .../hostagent/commands_registration_test.go | 22 +++++-- internal/securityutil/httpurl_test.go | 58 +++++++++++++++++ internal/securityutil/websocket_origin.go | 62 +++++++++++++++++++ internal/websocket/hub.go | 37 ++--------- internal/websocket/hub_more2_test.go | 18 +++++- internal/websocket/hub_test.go | 4 +- 19 files changed, 410 insertions(+), 107 deletions(-) create mode 100644 internal/securityutil/websocket_origin.go diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 0607b0714..879f3d7a0 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -788,6 +788,13 @@ workspace is opened through the control plane. That proxy-trust boundary must also reject wildcard trust ranges such as `0.0.0.0/0` or `::/0` at startup, and agent-adjacent forwarded-header reads must fail closed if invalid wildcard proxy trust configuration is present. +That same lifecycle-owned command websocket now derives an explicit +same-origin HTTP `Origin` header for `/api/agent/ws` from the canonical Pulse +base URL through `internal/securityutil/websocket_origin.go`, and the agent +receiver must reject missing or cross-host origins before registration. +Runtime command sockets therefore stay on the same fail-closed host/proxy +continuity contract as the browser websocket path instead of accepting +originless upgrades. That same shared helper layer also now assumes the Pulse Mobile relay runtime credential reaches only the explicit backend-owned route inventory, so lifecycle-adjacent setup and install flows cannot accidentally widen the diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 10933cb35..043cc2913 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -679,8 +679,11 @@ because the backend hop is plain HTTP. Forwarded host/proto headers may extend that same-origin boundary only after explicit trusted proxy CIDRs are injected, so hosted tenants and proxies that rewrite hostnames still fail closed onto the trusted forwarded-origin contract instead of weakening cross-site websocket -checks. `PULSE_TRUSTED_PROXY_CIDRS` must also reject wildcard trust ranges such -as `0.0.0.0/0` or `::/0` at startup, while runtime forwarded-header parsing +checks. Browser-facing websocket upgrades must also require an explicit +`Origin` header even when `allowedOrigins` is wildcarded, so missing-origin +requests cannot silently bypass the cross-site websocket boundary. +`PULSE_TRUSTED_PROXY_CIDRS` must also reject wildcard trust ranges such as +`0.0.0.0/0` or `::/0` at startup, while runtime forwarded-header parsing fails closed if an invalid wildcard proxy trust range somehow reaches the process. That same shared boundary now also owns outbound SSO metadata and discovery diff --git a/internal/agentexec/server.go b/internal/agentexec/server.go index f2f2a24be..cf405619c 100644 --- a/internal/agentexec/server.go +++ b/internal/agentexec/server.go @@ -5,9 +5,7 @@ import ( "encoding/json" "errors" "fmt" - "net" "net/http" - "net/url" "regexp" "strings" "sync" @@ -15,6 +13,7 @@ import ( "github.com/google/uuid" "github.com/gorilla/websocket" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" "github.com/rs/zerolog/log" ) @@ -232,35 +231,10 @@ func validateReadFilePayload(req *ReadFilePayload) error { func isAllowedWebSocketOrigin(r *http.Request) bool { origin := strings.TrimSpace(r.Header.Get("Origin")) if origin == "" { - // Non-browser clients (expected for agents) usually omit Origin. - return true - } - - parsed, err := url.Parse(origin) - if err != nil || parsed.Host == "" { - return false - } - if parsed.Scheme != "http" && parsed.Scheme != "https" { return false } - return normalizeOriginHost(parsed.Host) == normalizeOriginHost(r.Host) -} - -func normalizeOriginHost(host string) string { - normalized := strings.TrimSpace(strings.ToLower(host)) - if normalized == "" { - return normalized - } - - parsedHost, parsedPort, err := net.SplitHostPort(normalized) - if err != nil { - return normalized - } - if parsedPort == "80" || parsedPort == "443" { - return parsedHost - } - return net.JoinHostPort(parsedHost, parsedPort) + return securityutil.SameHostWebSocketOrigin(origin, r.Host) } // HandleWebSocket handles incoming WebSocket connections from agents diff --git a/internal/agentexec/server_coverage_test.go b/internal/agentexec/server_coverage_test.go index d6b213111..7c5229054 100644 --- a/internal/agentexec/server_coverage_test.go +++ b/internal/agentexec/server_coverage_test.go @@ -40,7 +40,7 @@ func newConnPair(t *testing.T) (*websocket.Conn, *websocket.Conn, func()) { serverConnCh <- conn })) - clientConn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + clientConn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { ts.Close() t.Fatalf("Dial: %v", err) @@ -75,7 +75,7 @@ func TestHandleWebSocket_RegistrationReadError(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -87,7 +87,7 @@ func TestHandleWebSocket_RegistrationMessageJSONError(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -108,7 +108,7 @@ func TestHandleWebSocket_RegistrationPayloadMissing(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -127,7 +127,7 @@ func TestHandleWebSocket_RegistrationPayloadUnmarshalError(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -154,7 +154,7 @@ func TestHandleWebSocket_InvalidTokenRejectionSendFailure(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -184,7 +184,7 @@ func TestHandleWebSocket_RegistrationAckSendFailure(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -212,7 +212,7 @@ func TestHandleWebSocket_PongHandler(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -550,7 +550,7 @@ func TestReadFileRoundTrip(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -651,7 +651,7 @@ func TestShutdownRejectsNewWebSocketConnections(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, resp, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, resp, err := dialAgentExecWebSocket(t, ts.URL) if conn != nil { conn.Close() } @@ -672,7 +672,7 @@ func TestShutdownClosesActiveConnectionsAndIsIdempotent(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } diff --git a/internal/agentexec/server_websocket_test.go b/internal/agentexec/server_websocket_test.go index a305be0d8..692e58259 100644 --- a/internal/agentexec/server_websocket_test.go +++ b/internal/agentexec/server_websocket_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/gorilla/websocket" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" ) type wsRawMessage struct { @@ -31,6 +32,24 @@ func wsURLForHTTP(serverURL string) string { return "ws" + strings.TrimPrefix(serverURL, "http") } +func wsHeadersForHTTP(t *testing.T, serverURL string) http.Header { + t.Helper() + + origin, err := securityutil.HTTPOriginForWebSocketBaseURL(serverURL) + if err != nil { + t.Fatalf("failed to derive websocket origin: %v", err) + } + + headers := http.Header{} + headers.Set("Origin", origin) + return headers +} + +func dialAgentExecWebSocket(t *testing.T, serverURL string) (*websocket.Conn, *http.Response, error) { + t.Helper() + return websocket.DefaultDialer.Dial(wsURLForHTTP(serverURL), wsHeadersForHTTP(t, serverURL)) +} + func wsWriteMessage(t *testing.T, conn *websocket.Conn, msg Message) { t.Helper() _ = conn.SetWriteDeadline(time.Now().Add(2 * time.Second)) @@ -103,7 +122,7 @@ func TestHandleWebSocket_RegistrationSuccessAndDisconnectRemovesAgent(t *testing ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -131,12 +150,30 @@ func TestHandleWebSocket_RegistrationSuccessAndDisconnectRemovesAgent(t *testing waitFor(t, 2*time.Second, func() bool { return !s.IsAgentConnected("a1") }) } +func TestHandleWebSocket_RejectsMissingOrigin(t *testing.T) { + s := NewServer(allowAllTestTokens) + ts := newWSServer(t, s) + defer ts.Close() + + conn, resp, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + if err == nil { + conn.Close() + t.Fatalf("expected websocket upgrade to reject missing Origin") + } + if resp == nil { + t.Fatalf("expected HTTP response for rejected websocket upgrade") + } + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("expected %d, got %d", http.StatusForbidden, resp.StatusCode) + } +} + func TestHandleWebSocket_InvalidTokenRejected(t *testing.T) { s := NewServer(func(string, string) bool { return false }) ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -169,7 +206,7 @@ func TestHandleWebSocket_MissingAgentIDRejected(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -200,7 +237,7 @@ func TestHandleWebSocket_FirstMessageMustBeRegister(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -220,7 +257,7 @@ func TestHandleWebSocket_RejectsOversizedRegistrationMessage(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -243,7 +280,7 @@ func TestHandleWebSocket_AgentPingRespondsWithPong(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -271,7 +308,7 @@ func TestExecuteCommand_RoundTripViaWebSocket(t *testing.T) { ts := newWSServer(t, s) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } @@ -358,7 +395,7 @@ func TestHandleWebSocket_ReconnectSameAgentIDClosesOldConnection(t *testing.T) { dial := func() *websocket.Conn { t.Helper() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL), nil) + conn, _, err := dialAgentExecWebSocket(t, ts.URL) if err != nil { t.Fatalf("Dial: %v", err) } diff --git a/internal/api/ai_handlers_investigation_additional_test.go b/internal/api/ai_handlers_investigation_additional_test.go index b22d7fd4c..30f24a397 100644 --- a/internal/api/ai_handlers_investigation_additional_test.go +++ b/internal/api/ai_handlers_investigation_additional_test.go @@ -17,6 +17,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/ai" "github.com/rcourtman/pulse-go-rewrite/internal/ai/chat" "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" "github.com/rcourtman/pulse-go-rewrite/pkg/aicontracts" ) @@ -597,10 +598,23 @@ func wsURLForHTTP(url string) string { return "ws://" + strings.TrimPrefix(url, "http://") } +func wsHeadersForHTTP(t *testing.T, url string) http.Header { + t.Helper() + + origin, err := securityutil.HTTPOriginForWebSocketBaseURL(url) + if err != nil { + t.Fatalf("failed to derive websocket origin: %v", err) + } + + headers := http.Header{} + headers.Set("Origin", origin) + return headers +} + func registerAgent(t *testing.T, url, agentID, hostname string) *websocket.Conn { t.Helper() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(url), nil) + conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(url), wsHeadersForHTTP(t, url)) if err != nil { t.Fatalf("failed to dial websocket: %v", err) } diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index ae20b477c..8681cd99c 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -10315,7 +10315,7 @@ func TestContract_AgentExecWebSocketRejectsUnboundToken(t *testing.T) { ts := newIPv4HTTPServer(t, router.Handler()) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL)+"/api/agent/ws", nil) + conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL)+"/api/agent/ws", wsHeadersForHTTP(t, ts.URL)) if err != nil { t.Fatalf("Dial: %v", err) } @@ -10350,7 +10350,7 @@ func TestContract_AgentExecWebSocketAcceptsLegacyHostnameBinding(t *testing.T) { ts := newIPv4HTTPServer(t, router.Handler()) defer ts.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL)+"/api/agent/ws", nil) + conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(ts.URL)+"/api/agent/ws", wsHeadersForHTTP(t, ts.URL)) if err != nil { t.Fatalf("Dial: %v", err) } diff --git a/internal/api/router_integration_test.go b/internal/api/router_integration_test.go index f0cde54ca..d0d87e004 100644 --- a/internal/api/router_integration_test.go +++ b/internal/api/router_integration_test.go @@ -24,6 +24,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" "github.com/rcourtman/pulse-go-rewrite/internal/recovery" recoverymanager "github.com/rcourtman/pulse-go-rewrite/internal/recovery/manager" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" "github.com/rcourtman/pulse-go-rewrite/internal/updates" internalws "github.com/rcourtman/pulse-go-rewrite/internal/websocket" internalauth "github.com/rcourtman/pulse-go-rewrite/pkg/auth" @@ -37,6 +38,19 @@ type integrationServer struct { config *config.Config } +func wsHeadersForHTTP(t *testing.T, serverURL string) http.Header { + t.Helper() + + origin, err := securityutil.HTTPOriginForWebSocketBaseURL(serverURL) + if err != nil { + t.Fatalf("failed to derive websocket origin: %v", err) + } + + headers := http.Header{} + headers.Set("Origin", origin) + return headers +} + func newIntegrationServer(t *testing.T) *integrationServer { return newIntegrationServerWithConfig(t, nil) } @@ -1471,7 +1485,7 @@ func TestWebSocketSendsInitialState(t *testing.T) { wsURL := "ws" + strings.TrimPrefix(srv.server.URL, "http") + "/ws?org_id=default" - conn, _, err := gorillaws.DefaultDialer.Dial(wsURL, nil) + conn, _, err := gorillaws.DefaultDialer.Dial(wsURL, wsHeadersForHTTP(t, srv.server.URL)) if err != nil { t.Fatalf("websocket dial failed: %v", err) } @@ -1548,7 +1562,7 @@ func TestWebsocketPayloadContractShape(t *testing.T) { wsURL := "ws" + strings.TrimPrefix(srv.server.URL, "http") + "/ws?org_id=default" - conn, _, err := gorillaws.DefaultDialer.Dial(wsURL, nil) + conn, _, err := gorillaws.DefaultDialer.Dial(wsURL, wsHeadersForHTTP(t, srv.server.URL)) if err != nil { t.Fatalf("websocket dial failed: %v", err) } @@ -1657,7 +1671,7 @@ func TestWebsocketPayloadUsesCanonicalStateContract(t *testing.T) { wsURL := "ws" + strings.TrimPrefix(srv.server.URL, "http") + "/ws?org_id=default" - conn, _, err := gorillaws.DefaultDialer.Dial(wsURL, nil) + conn, _, err := gorillaws.DefaultDialer.Dial(wsURL, wsHeadersForHTTP(t, srv.server.URL)) if err != nil { t.Fatalf("websocket dial failed: %v", err) } diff --git a/internal/api/security_regression_test.go b/internal/api/security_regression_test.go index c5125f4fc..8d9e254f0 100644 --- a/internal/api/security_regression_test.go +++ b/internal/api/security_regression_test.go @@ -499,7 +499,7 @@ func TestAgentExecTokenBindingEnforced(t *testing.T) { wsURL := wsURLForHTTP(ts.URL) + "/api/agent/ws" // Mismatched agent ID should be rejected - conn, _, err := websocket.DefaultDialer.Dial(wsURL, nil) + conn, _, err := websocket.DefaultDialer.Dial(wsURL, wsHeadersForHTTP(t, ts.URL)) if err != nil { t.Fatalf("Dial: %v", err) } @@ -526,7 +526,7 @@ func TestAgentExecTokenBindingEnforced(t *testing.T) { conn.Close() // Matching agent ID should succeed - conn, _, err = websocket.DefaultDialer.Dial(wsURL, nil) + conn, _, err = websocket.DefaultDialer.Dial(wsURL, wsHeadersForHTTP(t, ts.URL)) if err != nil { t.Fatalf("Dial: %v", err) } @@ -563,7 +563,7 @@ func TestSecurityTokens_AgentExecRejectsUnboundToken(t *testing.T) { defer ts.Close() wsURL := wsURLForHTTP(ts.URL) + "/api/agent/ws" - conn, _, err := websocket.DefaultDialer.Dial(wsURL, nil) + conn, _, err := websocket.DefaultDialer.Dial(wsURL, wsHeadersForHTTP(t, ts.URL)) if err != nil { t.Fatalf("Dial: %v", err) } @@ -600,7 +600,7 @@ func TestSecurityTokens_AgentExecRequiresAgentExecScope(t *testing.T) { defer ts.Close() wsURL := "ws" + strings.TrimPrefix(ts.URL, "http") + "/api/agent/ws" - conn, _, err := websocket.DefaultDialer.Dial(wsURL, nil) + conn, _, err := websocket.DefaultDialer.Dial(wsURL, wsHeadersForHTTP(t, ts.URL)) if err != nil { t.Fatalf("Dial: %v", err) } @@ -642,7 +642,7 @@ func TestSecurityTokens_AgentExecRejectsLegacySingleAPIToken(t *testing.T) { defer ts.Close() wsURL := wsURLForHTTP(ts.URL) + "/api/agent/ws" - conn, _, err := websocket.DefaultDialer.Dial(wsURL, nil) + conn, _, err := websocket.DefaultDialer.Dial(wsURL, wsHeadersForHTTP(t, ts.URL)) if err != nil { t.Fatalf("Dial: %v", err) } @@ -683,7 +683,7 @@ func TestWebSocketAllowsMonitoringReadScope(t *testing.T) { defer ts.Close() wsURL := "ws" + strings.TrimPrefix(ts.URL, "http") + "/ws?org_id=default" - headers := http.Header{} + headers := wsHeadersForHTTP(t, ts.URL) headers.Set("X-API-Token", rawToken) conn, _, err := websocket.DefaultDialer.Dial(wsURL, headers) if err != nil { @@ -706,7 +706,7 @@ func TestWebSocketAllowsBearerToken(t *testing.T) { defer ts.Close() wsURL := "ws" + strings.TrimPrefix(ts.URL, "http") + "/ws?org_id=default" - headers := http.Header{} + headers := wsHeadersForHTTP(t, ts.URL) headers.Set("Authorization", "Bearer "+rawToken) conn, _, err := websocket.DefaultDialer.Dial(wsURL, headers) if err != nil { @@ -729,7 +729,7 @@ func TestWebSocketAllowsTokenQueryParam(t *testing.T) { defer ts.Close() wsURL := "ws" + strings.TrimPrefix(ts.URL, "http") + "/ws?org_id=default&token=" + rawToken - conn, resp, err := websocket.DefaultDialer.Dial(wsURL, nil) + conn, resp, err := websocket.DefaultDialer.Dial(wsURL, wsHeadersForHTTP(t, ts.URL)) if err != nil { t.Fatalf("Dial: %v", err) } @@ -754,7 +754,7 @@ func TestWebSocketRejectsOrgQueryMismatchWithTenantContext(t *testing.T) { defer ts.Close() wsURL := "ws" + strings.TrimPrefix(ts.URL, "http") + "/ws?org_id=tenant-b&token=" + rawToken - conn, resp, err := websocket.DefaultDialer.Dial(wsURL, nil) + conn, resp, err := websocket.DefaultDialer.Dial(wsURL, wsHeadersForHTTP(t, ts.URL)) if err == nil { conn.Close() t.Fatalf("expected websocket org mismatch rejection") @@ -781,7 +781,7 @@ func TestWebSocketRejectsInvalidOrgQueryID(t *testing.T) { defer ts.Close() wsURL := "ws" + strings.TrimPrefix(ts.URL, "http") + "/ws?org_id=../tenant-b&token=" + rawToken - conn, resp, err := websocket.DefaultDialer.Dial(wsURL, nil) + conn, resp, err := websocket.DefaultDialer.Dial(wsURL, wsHeadersForHTTP(t, ts.URL)) if err == nil { conn.Close() t.Fatalf("expected websocket invalid org rejection") diff --git a/internal/api/websocket_origin_security_test.go b/internal/api/websocket_origin_security_test.go index cd6bf15b6..4c6add4de 100644 --- a/internal/api/websocket_origin_security_test.go +++ b/internal/api/websocket_origin_security_test.go @@ -74,6 +74,30 @@ func TestWebSocketOriginRejectedWhenNotAllowed(t *testing.T) { } } +func TestWebSocketOriginRejectedWhenMissing(t *testing.T) { + rawToken := "ws-origin-missing-123.12345678" + record := newTokenRecord(t, rawToken, []string{config.ScopeMonitoringRead}, nil) + + server, cleanup := newWebSocketRouter(t, []string{"*"}, record) + defer cleanup() + + wsURL := "ws" + strings.TrimPrefix(server.URL, "http") + "/ws?org_id=default" + headers := http.Header{} + headers.Set("X-API-Token", rawToken) + + conn, resp, err := websocket.DefaultDialer.Dial(wsURL, headers) + if err == nil { + conn.Close() + t.Fatalf("expected websocket origin rejection when Origin header is missing") + } + if resp == nil { + t.Fatalf("expected HTTP response for rejected websocket origin") + } + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("expected status %d, got %d", http.StatusForbidden, resp.StatusCode) + } +} + func TestWebSocketOriginAllowedWhenConfigured(t *testing.T) { rawToken := "ws-origin-allow-123.12345678" record := newTokenRecord(t, rawToken, []string{config.ScopeMonitoringRead}, nil) diff --git a/internal/hostagent/agent_new_test.go b/internal/hostagent/agent_new_test.go index e79919b7b..8f979c848 100644 --- a/internal/hostagent/agent_new_test.go +++ b/internal/hostagent/agent_new_test.go @@ -201,6 +201,53 @@ func TestNewCommandClient_SetsSecureCommandDefaults(t *testing.T) { } } +func TestNewCommandClient_BuildWebSocketOriginFollowsCanonicalPulseURL(t *testing.T) { + logger := zerolog.Nop() + + tests := []struct { + name string + pulseURL string + want string + wantErr bool + }{ + { + name: "hosted https origin", + pulseURL: "https://pulse.example/base/", + want: "https://pulse.example", + }, + { + name: "loopback http origin", + pulseURL: "http://127.0.0.1:7655/pulse", + want: "http://127.0.0.1:7655", + }, + { + name: "rejects non loopback plaintext", + pulseURL: "http://pulse.example", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := NewCommandClient(Config{ + PulseURL: tt.pulseURL, + Logger: &logger, + }, "agent-1", "node-1", "linux", "1.0.0") + + got, err := client.buildWebSocketOrigin() + if (err != nil) != tt.wantErr { + t.Fatalf("buildWebSocketOrigin() err = %v, wantErr %v", err, tt.wantErr) + } + if tt.wantErr { + return + } + if got != tt.want { + t.Fatalf("buildWebSocketOrigin() = %q, want %q", got, tt.want) + } + }) + } +} + func TestNew_UsesPinnedServerFingerprintForHTTPTransport(t *testing.T) { mc := &mockCollector{ hostInfoFn: func(context.Context) (*gohost.InfoStat, error) { diff --git a/internal/hostagent/command_client_test.go b/internal/hostagent/command_client_test.go index 655e5c2ae..058e316b0 100644 --- a/internal/hostagent/command_client_test.go +++ b/internal/hostagent/command_client_test.go @@ -151,3 +151,56 @@ func TestCommandClientBuildWebSocketURL(t *testing.T) { }) } } + +func TestCommandClientBuildWebSocketOrigin(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + pulseURL string + want string + wantErr bool + }{ + { + name: "https becomes https origin", + pulseURL: "https://example.invalid/pulse/", + want: "https://example.invalid", + }, + { + name: "loopback http stays http origin", + pulseURL: "http://localhost:7655/pulse", + want: "http://localhost:7655", + }, + { + name: "wss becomes https origin", + pulseURL: "wss://example.invalid", + want: "https://example.invalid", + }, + { + name: "non-loopback http rejected", + pulseURL: "http://example.invalid", + wantErr: true, + }, + { + name: "missing host rejected", + pulseURL: "/relative/path", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := &CommandClient{pulseURL: tt.pulseURL} + got, err := client.buildWebSocketOrigin() + if (err != nil) != tt.wantErr { + t.Fatalf("buildWebSocketOrigin() err = %v, wantErr %v", err, tt.wantErr) + } + if tt.wantErr { + return + } + if got != tt.want { + t.Fatalf("buildWebSocketOrigin() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/internal/hostagent/commands.go b/internal/hostagent/commands.go index 11e838d3c..2b024978e 100644 --- a/internal/hostagent/commands.go +++ b/internal/hostagent/commands.go @@ -8,6 +8,7 @@ import ( "fmt" "math/rand" "net" + "net/http" "os" "os/exec" "path/filepath" @@ -269,6 +270,10 @@ func (c *CommandClient) connectAndHandle(ctx context.Context) error { if err != nil { return fmt.Errorf("build websocket url: %w", err) } + origin, err := c.buildWebSocketOrigin() + if err != nil { + return fmt.Errorf("build websocket origin: %w", err) + } c.logger.Debug().Str("url", wsURL).Msg("Connecting to Pulse command server") @@ -282,9 +287,11 @@ func (c *CommandClient) connectAndHandle(ctx context.Context) error { TLSClientConfig: tlsConfig, HandshakeTimeout: 45 * time.Second, } + headers := http.Header{} + headers.Set("Origin", origin) // Connect - conn, _, err := dialer.DialContext(ctx, wsURL, nil) + conn, _, err := dialer.DialContext(ctx, wsURL, headers) if err != nil { return fmt.Errorf("dial websocket: %w", err) } @@ -368,6 +375,10 @@ func (c *CommandClient) buildWebSocketURL() (string, error) { return parsed.String(), nil } +func (c *CommandClient) buildWebSocketOrigin() (string, error) { + return securityutil.HTTPOriginForWebSocketBaseURL(c.pulseURL) +} + func (c *CommandClient) sendRegistration(conn *websocket.Conn) error { payload, err := json.Marshal(registerPayload{ AgentID: c.agentID, diff --git a/internal/hostagent/commands_registration_test.go b/internal/hostagent/commands_registration_test.go index 9382cc730..eadd7b92b 100644 --- a/internal/hostagent/commands_registration_test.go +++ b/internal/hostagent/commands_registration_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/gorilla/websocket" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" "github.com/rs/zerolog" ) @@ -16,6 +17,19 @@ func wsURLForHTTP(serverURL string) string { return "ws" + strings.TrimPrefix(serverURL, "http") } +func wsHeadersForHTTP(t *testing.T, serverURL string) http.Header { + t.Helper() + + origin, err := securityutil.HTTPOriginForWebSocketBaseURL(serverURL) + if err != nil { + t.Fatalf("failed to derive websocket origin: %v", err) + } + + headers := http.Header{} + headers.Set("Origin", origin) + return headers +} + func TestCommandClient_sendRegistration_WritesExpectedPayload(t *testing.T) { upgrader := websocket.Upgrader{CheckOrigin: func(*http.Request) bool { return true }} @@ -48,7 +62,7 @@ func TestCommandClient_sendRegistration_WritesExpectedPayload(t *testing.T) { })) defer server.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(server.URL), nil) + conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(server.URL), wsHeadersForHTTP(t, server.URL)) if err != nil { t.Fatalf("Dial: %v", err) } @@ -110,7 +124,7 @@ func TestCommandClient_waitForRegistration_AcceptsSuccess(t *testing.T) { })) defer server.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(server.URL), nil) + conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(server.URL), wsHeadersForHTTP(t, server.URL)) if err != nil { t.Fatalf("Dial: %v", err) } @@ -139,7 +153,7 @@ func TestCommandClient_waitForRegistration_RejectsFailure(t *testing.T) { })) defer server.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(server.URL), nil) + conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(server.URL), wsHeadersForHTTP(t, server.URL)) if err != nil { t.Fatalf("Dial: %v", err) } @@ -167,7 +181,7 @@ func TestCommandClient_waitForRegistration_UnexpectedMessageType(t *testing.T) { })) defer server.Close() - conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(server.URL), nil) + conn, _, err := websocket.DefaultDialer.Dial(wsURLForHTTP(server.URL), wsHeadersForHTTP(t, server.URL)) if err != nil { t.Fatalf("Dial: %v", err) } diff --git a/internal/securityutil/httpurl_test.go b/internal/securityutil/httpurl_test.go index 49c42acae..debd2efab 100644 --- a/internal/securityutil/httpurl_test.go +++ b/internal/securityutil/httpurl_test.go @@ -164,6 +164,64 @@ func TestNormalizePulseWebSocketBaseURL(t *testing.T) { } } +func TestSameHostWebSocketOrigin(t *testing.T) { + tests := []struct { + name string + origin string + requestHost string + want bool + }{ + {name: "same host", origin: "https://tenant.example.com", requestHost: "tenant.example.com", want: true}, + {name: "default port normalized", origin: "https://tenant.example.com:443", requestHost: "tenant.example.com", want: true}, + {name: "different host", origin: "https://evil.example.com", requestHost: "tenant.example.com", want: false}, + {name: "bad scheme", origin: "ws://tenant.example.com", requestHost: "tenant.example.com", want: false}, + {name: "invalid origin", origin: "://bad", requestHost: "tenant.example.com", want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := SameHostWebSocketOrigin(tt.origin, tt.requestHost); got != tt.want { + t.Fatalf("SameHostWebSocketOrigin(%q, %q) = %v, want %v", tt.origin, tt.requestHost, got, tt.want) + } + }) + } +} + +func TestHTTPOriginForWebSocketBaseURL(t *testing.T) { + tests := []struct { + name string + raw string + want string + wantError string + }{ + {name: "wss becomes https origin", raw: "wss://example.invalid/pulse", want: "https://example.invalid"}, + {name: "ws becomes http origin", raw: "ws://localhost:7655/pulse", want: "http://localhost:7655"}, + {name: "https input becomes https origin", raw: "https://example.invalid/base", want: "https://example.invalid"}, + {name: "rejects invalid input", raw: "ftp://example.invalid", wantError: "unsupported scheme"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := HTTPOriginForWebSocketBaseURL(tt.raw) + if tt.wantError != "" { + if err == nil { + t.Fatalf("HTTPOriginForWebSocketBaseURL(%q) expected error", tt.raw) + } + if !strings.Contains(err.Error(), tt.wantError) { + t.Fatalf("HTTPOriginForWebSocketBaseURL(%q) error = %q, want substring %q", tt.raw, err.Error(), tt.wantError) + } + return + } + if err != nil { + t.Fatalf("HTTPOriginForWebSocketBaseURL(%q) error = %v", tt.raw, err) + } + if got != tt.want { + t.Fatalf("HTTPOriginForWebSocketBaseURL(%q) = %q, want %q", tt.raw, got, tt.want) + } + }) + } +} + func TestResolveRelativeURLRejectsAbsoluteURL(t *testing.T) { base, err := NormalizeHTTPBaseURL("https://example.com/api", "") if err != nil { diff --git a/internal/securityutil/websocket_origin.go b/internal/securityutil/websocket_origin.go new file mode 100644 index 000000000..b5d9cebcd --- /dev/null +++ b/internal/securityutil/websocket_origin.go @@ -0,0 +1,62 @@ +package securityutil + +import ( + "fmt" + "net" + "net/url" + "strings" +) + +// NormalizeWebSocketOriginHost normalizes Origin/Host values for same-origin comparison. +func NormalizeWebSocketOriginHost(host string) string { + normalized := strings.TrimSpace(strings.ToLower(host)) + if normalized == "" { + return normalized + } + + parsedHost, parsedPort, err := net.SplitHostPort(normalized) + if err != nil { + return normalized + } + if parsedPort == "80" || parsedPort == "443" { + return parsedHost + } + return net.JoinHostPort(parsedHost, parsedPort) +} + +// SameHostWebSocketOrigin validates that an Origin header is http(s) and matches the request host. +func SameHostWebSocketOrigin(origin string, requestHost string) bool { + parsed, err := url.Parse(strings.TrimSpace(origin)) + if err != nil || parsed.Host == "" { + return false + } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return false + } + + return NormalizeWebSocketOriginHost(parsed.Host) == NormalizeWebSocketOriginHost(requestHost) +} + +// HTTPOriginForWebSocketBaseURL returns the http(s) Origin header for a Pulse websocket base URL. +func HTTPOriginForWebSocketBaseURL(raw string) (string, error) { + parsed, err := NormalizePulseWebSocketBaseURL(raw) + if err != nil { + return "", err + } + + switch parsed.Scheme { + case "ws": + parsed.Scheme = "http" + case "wss": + parsed.Scheme = "https" + default: + return "", fmt.Errorf("unsupported websocket origin scheme %q", parsed.Scheme) + } + + parsed.Path = "" + parsed.RawPath = "" + parsed.RawQuery = "" + parsed.Fragment = "" + + return parsed.String(), nil +} diff --git a/internal/websocket/hub.go b/internal/websocket/hub.go index 8a6a9bbc8..8962fd670 100644 --- a/internal/websocket/hub.go +++ b/internal/websocket/hub.go @@ -8,7 +8,6 @@ import ( "math" "net" "net/http" - "net/url" "strings" "sync" "sync/atomic" @@ -16,6 +15,7 @@ import ( "github.com/gorilla/websocket" "github.com/rcourtman/pulse-go-rewrite/internal/alerts" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" "github.com/rcourtman/pulse-go-rewrite/internal/utils" "github.com/rcourtman/pulse-go-rewrite/pkg/audit" "github.com/rcourtman/pulse-go-rewrite/pkg/auth" @@ -126,10 +126,9 @@ func (h *Hub) SetAllowedOrigins(origins []string) { // checkOrigin validates the origin against allowed origins func (h *Hub) checkOrigin(r *http.Request) bool { - origin := r.Header.Get("Origin") + origin := strings.TrimSpace(r.Header.Get("Origin")) if origin == "" { - // No origin header, allow for non-browser clients - return true + return false } h.mu.RLock() @@ -172,7 +171,7 @@ func (h *Hub) checkOrigin(r *http.Request) bool { if origin == requestOrigin { return true } - if sameHostOrigin(origin, host) { + if securityutil.SameHostWebSocketOrigin(origin, host) { return true } @@ -246,34 +245,6 @@ func (h *Hub) checkOrigin(r *http.Request) bool { return false } -func sameHostOrigin(origin, requestHost string) bool { - parsed, err := url.Parse(origin) - if err != nil || parsed.Host == "" { - return false - } - if parsed.Scheme != "http" && parsed.Scheme != "https" { - return false - } - - return normalizeOriginHost(parsed.Host) == normalizeOriginHost(requestHost) -} - -func normalizeOriginHost(host string) string { - normalized := strings.TrimSpace(strings.ToLower(host)) - if normalized == "" { - return normalized - } - - parsedHost, parsedPort, err := net.SplitHostPort(normalized) - if err != nil { - return normalized - } - if parsedPort == "80" || parsedPort == "443" { - return parsedHost - } - return net.JoinHostPort(parsedHost, parsedPort) -} - // Client represents a WebSocket client type Client struct { hub *Hub diff --git a/internal/websocket/hub_more2_test.go b/internal/websocket/hub_more2_test.go index c5d196bcd..4f63e13b1 100644 --- a/internal/websocket/hub_more2_test.go +++ b/internal/websocket/hub_more2_test.go @@ -10,8 +10,22 @@ import ( "time" "github.com/gorilla/websocket" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" ) +func wsHeadersForHTTP(t *testing.T, serverURL string) http.Header { + t.Helper() + + origin, err := securityutil.HTTPOriginForWebSocketBaseURL(serverURL) + if err != nil { + t.Fatalf("failed to derive websocket origin: %v", err) + } + + headers := http.Header{} + headers.Set("Origin", origin) + return headers +} + func TestBroadcastStateEnqueuesRawData(t *testing.T) { hub := NewHub(nil) state := struct { @@ -168,7 +182,7 @@ func TestHandleWebSocketPingPong(t *testing.T) { defer server.Close() wsURL := "ws" + strings.TrimPrefix(server.URL, "http") + "?org_id=default" - conn, _, err := websocket.DefaultDialer.Dial(wsURL, nil) + conn, _, err := websocket.DefaultDialer.Dial(wsURL, wsHeadersForHTTP(t, server.URL)) if err != nil { t.Fatalf("dial websocket: %v", err) } @@ -214,7 +228,7 @@ func TestHandleWebSocket_ReadLimitExceededClosesConnection(t *testing.T) { EnableCompression: false, } - conn, _, err := dialer.Dial(wsURL, nil) + conn, _, err := dialer.Dial(wsURL, wsHeadersForHTTP(t, server.URL)) if err != nil { t.Fatalf("dial websocket: %v", err) } diff --git a/internal/websocket/hub_test.go b/internal/websocket/hub_test.go index affa88d64..73a0bf475 100644 --- a/internal/websocket/hub_test.go +++ b/internal/websocket/hub_test.go @@ -708,12 +708,12 @@ func TestHub_CheckOrigin(t *testing.T) { remoteAddr string // Simulated peer IP for CSWSH checks expected bool }{ - // No origin header - always allowed for non-browser clients + // Browser-facing hub requires an explicit Origin header. { name: "no origin header", origin: "", host: "localhost:8080", - expected: true, + expected: false, }, // Same-origin requests