From 48ae2df5f2347a62e27e1cb9d082b960ad3fe947 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 20 Aug 2026 00:52:55 -0400 Subject: [PATCH] refactor(plugins): reuse extractHostname in the websocket allowlist The IPv6 host parsing added for isHostAllowed duplicated extractHostname, which already lives in the same package and backs the HTTP client's identical allowlist check. Two copies of a security-relevant parser can drift, so the websocket service now calls the existing helper. The port-stripping specs move into the URL Validation block that already covered them. --- plugins/host_websocket.go | 10 +-------- plugins/host_websocket_test.go | 41 ++++++++++------------------------ 2 files changed, 13 insertions(+), 38 deletions(-) diff --git a/plugins/host_websocket.go b/plugins/host_websocket.go index 8bedf5dc7..82aded0cb 100644 --- a/plugins/host_websocket.go +++ b/plugins/host_websocket.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "maps" - "net" "net/http" "net/url" "strings" @@ -244,14 +243,7 @@ func (s *webSocketServiceImpl) getConnection(connectionID string) (*wsConnection } func (s *webSocketServiceImpl) isHostAllowed(host string) bool { - // Strip port from host if present - hostWithoutPort := host - if h, _, err := net.SplitHostPort(host); err == nil { - hostWithoutPort = h - } else if h, ok := strings.CutPrefix(host, "["); ok { - // Bracketed IPv6 literal with no port - hostWithoutPort = strings.TrimSuffix(h, "]") - } + hostWithoutPort := extractHostname(host) for _, pattern := range s.requiredHosts { if matchHostPattern(pattern, hostWithoutPort) { diff --git a/plugins/host_websocket_test.go b/plugins/host_websocket_test.go index 15028acd2..2aa85cd21 100644 --- a/plugins/host_websocket_test.go +++ b/plugins/host_websocket_test.go @@ -144,20 +144,18 @@ var _ = Describe("WebSocketService", Ordered, func() { Expect(allowed).To(BeFalse()) }) - It("should strip port before checking host", func() { - // Implementation strips port before matching against patterns - // test-websocket manifest has "localhost:*" which matches "localhost" - // after port stripping - // Note: The port wildcard pattern isn't actually implemented, but - // since port is stripped, "localhost:*" is compared against "localhost" - // which won't match. To make localhost work, we'd need exact "localhost" - // in the allowed hosts list. - - // Testing that port is properly stripped - // The pattern "localhost:*" won't match "localhost" due to exact match - allowed := testService.isHostAllowed("localhost:8080") - Expect(allowed).To(BeFalse()) - }) + DescribeTable("should match against the host with its port stripped", + func(allowed []string, host string, expected bool) { + svc := &webSocketServiceImpl{requiredHosts: allowed} + Expect(svc.isHostAllowed(host)).To(Equal(expected)) + }, + Entry("hostname with port", []string{"example.com"}, "example.com:8080", true), + Entry("IPv6 with port", []string{"::1"}, "[::1]:8080", true), + Entry("IPv6 without port", []string{"::1"}, "[::1]", true), + Entry("host not in the list", []string{"::2"}, "[::1]:8080", false), + // "localhost:*" is matched against the stripped "localhost", so it never hits + Entry("port wildcards are not supported", []string{"localhost:*"}, "localhost:8080", false), + ) }) Describe("Connection Management", func() { @@ -614,18 +612,3 @@ func findWebSocketService(m *Manager, pluginName string) *webSocketServiceImpl { } return nil } - -var _ = Describe("isHostAllowed", func() { - DescribeTable("strips the port before matching", - func(allowed []string, host string, expected bool) { - svc := &webSocketServiceImpl{requiredHosts: allowed} - Expect(svc.isHostAllowed(host)).To(Equal(expected)) - }, - Entry("hostname with port", []string{"example.com"}, "example.com:8080", true), - Entry("hostname without port", []string{"example.com"}, "example.com", true), - Entry("IPv4 with port", []string{"127.0.0.1"}, "127.0.0.1:4533", true), - Entry("IPv6 with port", []string{"::1"}, "[::1]:8080", true), - Entry("IPv6 without port", []string{"::1"}, "[::1]", true), - Entry("IPv6 not in the list", []string{"::2"}, "[::1]:8080", false), - ) -})