From d64f5b2917b88906e6a7fedefe55b0d517bebbfd Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 22 Apr 2026 04:11:18 +0100 Subject: [PATCH] Canonicalize loopback-only Pulse transport validation --- .../v6/internal/subsystems/agent-lifecycle.md | 16 +- internal/agentupdate/coverage_test.go | 6 +- internal/agentupdate/update.go | 44 +----- internal/dockeragent/agent.go | 55 +------ internal/dockeragent/agent_internal_test.go | 1 + internal/hostagent/agent.go | 50 +----- internal/hostagent/commands.go | 41 +---- internal/kubernetesagent/agent.go | 61 +------- internal/kubernetesagent/agent_new_test.go | 13 +- internal/remoteconfig/client.go | 29 +--- .../remoteconfig/client_additional_test.go | 11 +- internal/securityutil/httpurl.go | 118 +++++++++++++++ internal/securityutil/httpurl_test.go | 142 ++++++++++++++++++ 13 files changed, 312 insertions(+), 275 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 227ac8b86..44719d432 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -1108,12 +1108,16 @@ fan-out writes a bootstrap token or runs the installer on a remote node, keep `StrictHostKeyChecking=yes`, and fail closed on key mismatch or missing-host- key state instead of downgrading to unauthenticated SSH during install. That same transport boundary also keeps plaintext Pulse URLs loopback-only. -`internal/hostagent/agent.go`, `internal/hostagent/commands.go`, and -`internal/agentupdate/update.go` may keep local-development `http://` or -`ws://` only for loopback hosts, but private-network and remote Pulse URLs -must still use HTTPS/WSS. `InsecureSkipVerify` may relax certificate -verification on TLS transport; it must not reopen plaintext HTTP for -private-network updater, websocket, or reporting paths. +`internal/securityutil/httpurl.go` owns the canonical Pulse transport +normalization used by `internal/hostagent/agent.go`, +`internal/hostagent/commands.go`, `internal/agentupdate/update.go`, +`internal/dockeragent/agent.go`, `internal/kubernetesagent/agent.go`, and +`internal/remoteconfig/client.go`. Those runtime clients may keep local- +development `http://` or `ws://` only for loopback hosts, but private-network +and remote Pulse URLs must still use HTTPS/WSS. `InsecureSkipVerify` may +relax certificate verification on TLS transport; it must not reopen plaintext +HTTP for private-network updater, websocket, reporting, or remote-config +paths. That same first-run lifecycle boundary also keeps unauthenticated setup local. Lifecycle-adjacent quick setup or recovery entrypoints may exist before an operator has configured auth, but they must stay direct-loopback only and any diff --git a/internal/agentupdate/coverage_test.go b/internal/agentupdate/coverage_test.go index 4befe33b8..fbdcee0a6 100644 --- a/internal/agentupdate/coverage_test.go +++ b/internal/agentupdate/coverage_test.go @@ -17,6 +17,8 @@ import ( "sync/atomic" "testing" "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" ) type roundTripperFunc func(*http.Request) (*http.Response, error) @@ -174,7 +176,7 @@ func TestIsLoopbackHost(t *testing.T) { t.Run("true", func(t *testing.T) { cases := []string{"localhost", "LOCALHOST", "agent.localhost", "127.0.0.1", "::1"} for _, tc := range cases { - if !isLoopbackHost(tc) { + if !securityutil.IsLoopbackHost(tc) { t.Fatalf("expected loopback host for %q", tc) } } @@ -183,7 +185,7 @@ func TestIsLoopbackHost(t *testing.T) { t.Run("false", func(t *testing.T) { cases := []string{"", "example.com", "192.168.1.10", "10.0.0.5"} for _, tc := range cases { - if isLoopbackHost(tc) { + if securityutil.IsLoopbackHost(tc) { t.Fatalf("expected non-loopback host for %q", tc) } } diff --git a/internal/agentupdate/update.go b/internal/agentupdate/update.go index 0a70918f9..ee6f5fdbc 100644 --- a/internal/agentupdate/update.go +++ b/internal/agentupdate/update.go @@ -11,9 +11,7 @@ import ( "errors" "fmt" "io" - "net" "net/http" - "net/url" "os" "os/exec" "path/filepath" @@ -23,6 +21,7 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/agenttls" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" "github.com/rcourtman/pulse-go-rewrite/internal/updatesignature" "github.com/rcourtman/pulse-go-rewrite/internal/utils" "github.com/rs/zerolog" @@ -559,52 +558,19 @@ func normalizeAgentName(agentName string) (string, error) { return name, nil } -func isLoopbackHost(host string) bool { - if host == "" { - return false - } - - normalized := strings.ToLower(strings.Trim(host, "[]")) - if normalized == "localhost" || strings.HasSuffix(normalized, ".localhost") { - return true - } - - ip := net.ParseIP(normalized) - return ip != nil && ip.IsLoopback() -} - func (u *Updater) validatePulseURL() error { pulseURL := strings.TrimSpace(u.cfg.PulseURL) if pulseURL == "" { return fmt.Errorf("Pulse URL is empty") } - parsed, err := url.Parse(pulseURL) + parsed, err := securityutil.NormalizePulseHTTPBaseURL(pulseURL) if err != nil { - return fmt.Errorf("failed to parse Pulse URL: %w", err) - } - if parsed.Scheme == "" || parsed.Host == "" { - return fmt.Errorf("Pulse URL must include scheme and host") - } - if parsed.User != nil { - return fmt.Errorf("Pulse URL must not include user credentials") - } - if parsed.RawQuery != "" || parsed.Fragment != "" { - return fmt.Errorf("Pulse URL must not include query or fragment") + return fmt.Errorf("invalid Pulse URL: %w", err) } - scheme := strings.ToLower(parsed.Scheme) - switch scheme { - case "https": - return nil - case "http": - if isLoopbackHost(parsed.Hostname()) { - return nil - } - return fmt.Errorf("HTTP Pulse URL is only allowed for localhost/loopback") - default: - return fmt.Errorf("unsupported Pulse URL scheme %q", parsed.Scheme) - } + u.cfg.PulseURL = parsed.String() + return nil } // performUpdate downloads and installs the new agent binary. diff --git a/internal/dockeragent/agent.go b/internal/dockeragent/agent.go index c7dc5226f..0eb49f518 100644 --- a/internal/dockeragent/agent.go +++ b/internal/dockeragent/agent.go @@ -7,18 +7,16 @@ import ( "encoding/json" "errors" "fmt" - "net" "net/http" - "net/url" "os" "os/exec" - "strconv" "strings" "sync" "time" systemtypes "github.com/moby/moby/api/types/system" "github.com/moby/moby/client" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" "github.com/rcourtman/pulse-go-rewrite/internal/utils" agentsdocker "github.com/rcourtman/pulse-go-rewrite/pkg/agents/docker" "github.com/rs/zerolog" @@ -340,60 +338,11 @@ func normalizeTargets(raw []TargetConfig) ([]TargetConfig, error) { } func normalizeTargetURL(raw string) (string, error) { - raw = strings.TrimSpace(raw) - - parsed, err := url.Parse(raw) + parsed, err := securityutil.NormalizePulseHTTPBaseURL(raw) if err != nil { return "", err } - if parsed.Scheme == "" || parsed.Host == "" { - return "", errors.New("must include http:// or https:// with a valid host") - } - - scheme := strings.ToLower(parsed.Scheme) - if scheme != "http" && scheme != "https" { - return "", fmt.Errorf("unsupported scheme %q", parsed.Scheme) - } - - if scheme == "http" { - host := parsed.Hostname() - ip := net.ParseIP(host) - isLoopbackOrPrivate := strings.EqualFold(host, "localhost") || - (ip != nil && (ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast())) - if !isLoopbackOrPrivate { - return "", fmt.Errorf("http is only allowed for loopback or private network addresses, use https for %s", host) - } - } - - if parsed.User != nil { - return "", errors.New("userinfo is not supported") - } - - if parsed.RawQuery != "" { - return "", errors.New("query parameters are not supported") - } - - if parsed.Fragment != "" { - return "", errors.New("fragments are not supported") - } - - if parsed.Hostname() == "" { - return "", errors.New("host is required") - } - - if port := parsed.Port(); port != "" { - portNum, err := strconv.Atoi(port) - if err != nil || portNum < 1 || portNum > 65535 { - return "", fmt.Errorf("invalid port %q: must be between 1 and 65535", port) - } - } - - parsed.Scheme = scheme - parsed.Host = strings.ToLower(parsed.Host) - parsed.Path = strings.TrimRight(parsed.Path, "/") - parsed.RawPath = "" - normalized := strings.TrimRight(parsed.String(), "/") if normalized == "" { return "", errors.New("URL is empty after normalization") diff --git a/internal/dockeragent/agent_internal_test.go b/internal/dockeragent/agent_internal_test.go index 778ebc415..599af4457 100644 --- a/internal/dockeragent/agent_internal_test.go +++ b/internal/dockeragent/agent_internal_test.go @@ -90,6 +90,7 @@ func TestNormalizeTargetsRejectsInsecureOrInvalidURLs(t *testing.T) { url string }{ {name: "non-loopback http", url: "http://pulse.example.com"}, + {name: "private-network http", url: "http://10.0.0.5:7655"}, {name: "unsupported scheme", url: "ftp://pulse.example.com"}, {name: "missing scheme", url: "pulse.example.com"}, {name: "query string", url: "https://pulse.example.com?x=1"}, diff --git a/internal/hostagent/agent.go b/internal/hostagent/agent.go index ae96dacb4..3d1f9009e 100644 --- a/internal/hostagent/agent.go +++ b/internal/hostagent/agent.go @@ -9,7 +9,6 @@ import ( "net" "net/http" "net/netip" - "net/url" "os" "os/exec" "path/filepath" @@ -22,6 +21,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/agenttls" "github.com/rcourtman/pulse-go-rewrite/internal/agentupdate" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" "github.com/rcourtman/pulse-go-rewrite/internal/sensors" "github.com/rcourtman/pulse-go-rewrite/internal/utils" agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host" @@ -845,58 +845,14 @@ func normalisePlatform(platform string) string { } func normalizePulseURL(rawURL string) (string, error) { - parsed, err := url.Parse(rawURL) + parsed, err := securityutil.NormalizePulseHTTPBaseURL(rawURL) if err != nil { - return "", fmt.Errorf("pulse URL %q is invalid: %w", rawURL, err) + return "", err } - if parsed.Scheme == "" { - return "", fmt.Errorf("pulse URL %q must include scheme (https:// or loopback http://)", rawURL) - } - if parsed.Host == "" { - return "", fmt.Errorf("pulse URL %q must include host", rawURL) - } - if parsed.User != nil { - return "", fmt.Errorf("pulse URL %q must not include user credentials", rawURL) - } - if parsed.RawQuery != "" || parsed.Fragment != "" { - return "", fmt.Errorf("pulse URL %q must not include query or fragment", rawURL) - } - - scheme := strings.ToLower(parsed.Scheme) - switch scheme { - case "https": - // Always allowed. - case "http": - if !isLoopbackHost(parsed.Hostname()) { - return "", fmt.Errorf("pulse URL %q must use https unless host is loopback", rawURL) - } - default: - return "", fmt.Errorf("pulse URL %q has unsupported scheme %q", rawURL, parsed.Scheme) - } - - parsed.Scheme = scheme - parsed.Path = strings.TrimRight(parsed.Path, "/") - parsed.RawPath = strings.TrimRight(parsed.RawPath, "/") - return parsed.String(), nil } -// isLoopbackHost returns true for localhost and loopback IPs only. Plain HTTP -// is never allowed for non-loopback Pulse URLs, even on private networks. -func isLoopbackHost(host string) bool { - normalized := strings.ToLower(strings.Trim(host, "[]")) - if normalized == "localhost" || strings.HasSuffix(normalized, ".localhost") { - return true - } - - ip := net.ParseIP(normalized) - if ip == nil { - return false - } - return ip.IsLoopback() -} - // collectTemperatures attempts to collect temperature data from the local system. // Returns an empty Sensors struct if collection fails (best-effort). func (a *Agent) collectTemperatures(ctx context.Context) agentshost.Sensors { diff --git a/internal/hostagent/commands.go b/internal/hostagent/commands.go index ac5a7e233..11e838d3c 100644 --- a/internal/hostagent/commands.go +++ b/internal/hostagent/commands.go @@ -8,7 +8,6 @@ import ( "fmt" "math/rand" "net" - "net/url" "os" "os/exec" "path/filepath" @@ -21,6 +20,7 @@ import ( "github.com/gorilla/websocket" "github.com/rcourtman/pulse-go-rewrite/internal/agentexec" "github.com/rcourtman/pulse-go-rewrite/internal/agenttls" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" sshknownhosts "github.com/rcourtman/pulse-go-rewrite/internal/ssh/knownhosts" "github.com/rcourtman/pulse-go-rewrite/internal/utils" "github.com/rs/zerolog" @@ -351,44 +351,9 @@ func (c *CommandClient) connectAndHandle(ctx context.Context) error { } func (c *CommandClient) buildWebSocketURL() (string, error) { - parsed, err := url.Parse(c.pulseURL) + parsed, err := securityutil.NormalizePulseWebSocketBaseURL(c.pulseURL) if err != nil { - return "", fmt.Errorf("pulse URL is invalid: %w", err) - } - if parsed.Host == "" { - return "", fmt.Errorf("missing host in pulse URL") - } - - if parsed.Scheme == "" { - return "", fmt.Errorf("pulse URL %q must include scheme", c.pulseURL) - } - if parsed.Host == "" { - return "", fmt.Errorf("pulse URL %q must include host", c.pulseURL) - } - if parsed.User != nil { - return "", fmt.Errorf("pulse URL %q must not include user credentials", c.pulseURL) - } - if parsed.RawQuery != "" || parsed.Fragment != "" { - return "", fmt.Errorf("pulse URL %q must not include query or fragment", c.pulseURL) - } - - switch strings.ToLower(parsed.Scheme) { - case "https": - parsed.Scheme = "wss" - case "http": - if !isLoopbackHost(parsed.Hostname()) { - return "", fmt.Errorf("pulse URL %q must use https/wss unless host is loopback", c.pulseURL) - } - parsed.Scheme = "ws" - case "wss": - parsed.Scheme = "wss" - case "ws": - if !isLoopbackHost(parsed.Hostname()) { - return "", fmt.Errorf("pulse URL %q must use https/wss unless host is loopback", c.pulseURL) - } - parsed.Scheme = "ws" - default: - return "", fmt.Errorf("unsupported URL scheme %q", parsed.Scheme) + return "", err } basePath := strings.TrimRight(parsed.Path, "/") diff --git a/internal/kubernetesagent/agent.go b/internal/kubernetesagent/agent.go index 096d8394c..f90ff9f06 100644 --- a/internal/kubernetesagent/agent.go +++ b/internal/kubernetesagent/agent.go @@ -15,12 +15,12 @@ import ( "net/url" "os" "sort" - "strconv" "strings" "sync" "time" "github.com/IGLOU-EU/go-wildcard/v2" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" "github.com/rcourtman/pulse-go-rewrite/internal/utils" agentsk8s "github.com/rcourtman/pulse-go-rewrite/pkg/agents/kubernetes" "github.com/rs/zerolog" @@ -223,69 +223,14 @@ func New(cfg Config) (*Agent, error) { } func normalizePulseURL(rawURL string) (string, error) { - parsed, err := url.Parse(rawURL) + parsed, err := securityutil.NormalizePulseHTTPBaseURL(rawURL) if err != nil { - return "", fmt.Errorf("pulse URL %q is invalid: %w", rawURL, err) + return "", err } - if parsed.Scheme == "" { - return "", fmt.Errorf("pulse URL %q must include http:// or https:// scheme", rawURL) - } - if parsed.Host == "" { - return "", fmt.Errorf("pulse URL %q must include host", rawURL) - } - if parsed.User != nil { - return "", fmt.Errorf("pulse URL %q: userinfo is not supported", rawURL) - } - if parsed.RawQuery != "" { - return "", fmt.Errorf("pulse URL %q: query parameters are not supported", rawURL) - } - if parsed.Fragment != "" { - return "", fmt.Errorf("pulse URL %q: fragments are not supported", rawURL) - } - - scheme := strings.ToLower(parsed.Scheme) - switch scheme { - case "https": - // Always allowed. - case "http": - if !isLoopbackOrPrivateHost(parsed.Hostname()) { - return "", fmt.Errorf("pulse URL %q must use https unless host is loopback or private network", rawURL) - } - default: - return "", fmt.Errorf("pulse URL %q has unsupported scheme %q", rawURL, parsed.Scheme) - } - - if port := parsed.Port(); port != "" { - portNum, err := strconv.Atoi(port) - if err != nil || portNum < 1 || portNum > 65535 { - return "", fmt.Errorf("invalid port %q: must be between 1 and 65535", port) - } - } - - parsed.Scheme = scheme - parsed.Host = strings.ToLower(parsed.Host) - parsed.Path = strings.TrimRight(parsed.Path, "/") - parsed.RawPath = strings.TrimRight(parsed.RawPath, "/") - return parsed.String(), nil } -// isLoopbackOrPrivateHost returns true for loopback and RFC1918 private -// network addresses. HTTP (non-TLS) is safe over a local/private network; -// the scheme guard only needs to prevent plaintext over the public internet. -func isLoopbackOrPrivateHost(host string) bool { - if strings.EqualFold(host, "localhost") { - return true - } - - ip := net.ParseIP(host) - if ip == nil { - return false - } - return ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() -} - func buildRESTConfig(kubeconfigPath, kubeContext string) (*rest.Config, string, error) { kubeconfigPath = strings.TrimSpace(kubeconfigPath) kubeContext = strings.TrimSpace(kubeContext) diff --git a/internal/kubernetesagent/agent_new_test.go b/internal/kubernetesagent/agent_new_test.go index e93780654..a4c8e1607 100644 --- a/internal/kubernetesagent/agent_new_test.go +++ b/internal/kubernetesagent/agent_new_test.go @@ -123,10 +123,15 @@ func TestNormalizePulseURL(t *testing.T) { raw: "http://localhost:7655/", want: "http://localhost:7655", }, + { + name: "rejects private-network http", + raw: "http://10.0.0.5:7655", + wantError: "must use https unless host is loopback", + }, { name: "missing scheme", raw: "pulse.example.com", - wantError: "must include http:// or https://", + wantError: "must include scheme", }, { name: "unsupported scheme", @@ -141,17 +146,17 @@ func TestNormalizePulseURL(t *testing.T) { { name: "userinfo disallowed", raw: "https://user:pass@pulse.example.com", - wantError: "userinfo is not supported", + wantError: "must not include user credentials", }, { name: "query disallowed", raw: "https://pulse.example.com?x=1", - wantError: "query parameters are not supported", + wantError: "must not include query or fragment", }, { name: "fragment disallowed", raw: "https://pulse.example.com#frag", - wantError: "fragments are not supported", + wantError: "must not include query or fragment", }, } diff --git a/internal/remoteconfig/client.go b/internal/remoteconfig/client.go index c91161698..76686c87a 100644 --- a/internal/remoteconfig/client.go +++ b/internal/remoteconfig/client.go @@ -3,17 +3,16 @@ package remoteconfig import ( "context" "encoding/json" - "errors" "fmt" "io" "net/http" "net/url" - "strconv" "strings" "time" "github.com/rcourtman/pulse-go-rewrite/internal/agenttls" + "github.com/rcourtman/pulse-go-rewrite/internal/securityutil" "github.com/rcourtman/pulse-go-rewrite/internal/utils" "github.com/rs/zerolog" ) @@ -367,33 +366,9 @@ func normalizeConfig(cfg Config) (Config, error) { } func normalizePulseURL(raw string) (string, error) { - parsed, err := url.Parse(raw) + parsed, err := securityutil.NormalizePulseHTTPBaseURL(raw) if err != nil { return "", fmt.Errorf("invalid pulse URL: %w", err) } - - switch parsed.Scheme { - case "http", "https": - default: - return "", fmt.Errorf("invalid pulse URL scheme %q: must be http or https", parsed.Scheme) - } - - if parsed.Hostname() == "" { - return "", errors.New("invalid pulse URL: missing host") - } - if parsed.User != nil { - return "", errors.New("invalid pulse URL: userinfo is not allowed") - } - if parsed.RawQuery != "" || parsed.Fragment != "" { - return "", errors.New("invalid pulse URL: query and fragment are not allowed") - } - - if port := parsed.Port(); port != "" { - portValue, err := strconv.Atoi(port) - if err != nil || portValue < 1 || portValue > 65535 { - return "", fmt.Errorf("invalid pulse URL port %q: must be between 1 and 65535", port) - } - } - return strings.TrimRight(parsed.String(), "/"), nil } diff --git a/internal/remoteconfig/client_additional_test.go b/internal/remoteconfig/client_additional_test.go index 794728662..25b5ebfdf 100644 --- a/internal/remoteconfig/client_additional_test.go +++ b/internal/remoteconfig/client_additional_test.go @@ -284,7 +284,7 @@ func TestClientResolveAgentIDRequestErrors(t *testing.T) { } client = New(Config{ - PulseURL: "http://example.com", + PulseURL: "https://example.com", APIToken: "t", Hostname: "host", }) @@ -309,6 +309,15 @@ func TestClientFetchConfigValidation(t *testing.T) { }, wantText: "invalid remote config client configuration", }, + { + name: "private-network http rejected", + cfg: Config{ + PulseURL: "http://10.0.0.5:7655", + APIToken: "token", + AgentID: "agent-1", + }, + wantText: "must use https unless host is loopback", + }, { name: "missing API token", cfg: Config{ diff --git a/internal/securityutil/httpurl.go b/internal/securityutil/httpurl.go index 983526fbe..22aa94f24 100644 --- a/internal/securityutil/httpurl.go +++ b/internal/securityutil/httpurl.go @@ -4,9 +4,11 @@ import ( "context" "fmt" "io" + "net" "net/http" "net/url" "path" + "strconv" "strings" ) @@ -98,6 +100,122 @@ func NormalizeHTTPBaseURL(raw string, defaultScheme string) (*url.URL, error) { return parsed, nil } +// IsLoopbackHost reports whether host resolves to localhost or a loopback IP literal. +func IsLoopbackHost(host string) bool { + normalized := strings.ToLower(strings.Trim(host, "[]")) + if normalized == "" { + return false + } + if normalized == "localhost" || strings.HasSuffix(normalized, ".localhost") { + return true + } + + ip := net.ParseIP(normalized) + return ip != nil && ip.IsLoopback() +} + +// NormalizePulseHTTPBaseURL validates a Pulse control-plane base URL. +// HTTPS is required for non-loopback hosts; loopback localhost may use HTTP. +func NormalizePulseHTTPBaseURL(raw string) (*url.URL, error) { + parsed, err := normalizePulseBaseURL(raw, false) + if err != nil { + return nil, err + } + return parsed, nil +} + +// NormalizePulseWebSocketBaseURL validates a Pulse command-channel base URL. +// Non-loopback hosts are normalized to WSS; loopback localhost may use WS. +func NormalizePulseWebSocketBaseURL(raw string) (*url.URL, error) { + parsed, err := normalizePulseBaseURL(raw, true) + if err != nil { + return nil, err + } + return parsed, nil +} + +func normalizePulseBaseURL(raw string, websocket bool) (*url.URL, error) { + trimmed := strings.TrimSpace(raw) + if trimmed == "" { + return nil, fmt.Errorf("Pulse URL is required") + } + + parsed, err := url.Parse(trimmed) + if err != nil { + return nil, fmt.Errorf("Pulse URL %q is invalid: %w", raw, err) + } + if parsed.Scheme == "" { + if websocket { + return nil, fmt.Errorf("Pulse URL %q must include scheme (https://, wss://, or loopback http:// / ws://)", raw) + } + return nil, fmt.Errorf("Pulse URL %q must include scheme (https:// or loopback http://)", raw) + } + if parsed.Host == "" { + return nil, fmt.Errorf("Pulse URL %q must include host", raw) + } + if parsed.Hostname() == "" { + return nil, fmt.Errorf("Pulse URL %q must include host", raw) + } + if parsed.User != nil { + return nil, fmt.Errorf("Pulse URL %q must not include user credentials", raw) + } + if parsed.RawQuery != "" || parsed.Fragment != "" { + return nil, fmt.Errorf("Pulse URL %q must not include query or fragment", raw) + } + + if port := parsed.Port(); port != "" { + portNum, err := strconv.Atoi(port) + if err != nil || portNum < 1 || portNum > 65535 { + return nil, fmt.Errorf("invalid port %q: must be between 1 and 65535", port) + } + } + + scheme := strings.ToLower(parsed.Scheme) + switch scheme { + case "https": + if websocket { + parsed.Scheme = "wss" + } else { + parsed.Scheme = "https" + } + case "http": + if !IsLoopbackHost(parsed.Hostname()) { + if websocket { + return nil, fmt.Errorf("Pulse URL %q must use https/wss unless host is loopback", raw) + } + return nil, fmt.Errorf("Pulse URL %q must use https unless host is loopback", raw) + } + if websocket { + parsed.Scheme = "ws" + } else { + parsed.Scheme = "http" + } + case "wss": + if !websocket { + return nil, fmt.Errorf("Pulse URL %q has unsupported scheme %q", raw, parsed.Scheme) + } + parsed.Scheme = "wss" + case "ws": + if !websocket { + return nil, fmt.Errorf("Pulse URL %q has unsupported scheme %q", raw, parsed.Scheme) + } + if !IsLoopbackHost(parsed.Hostname()) { + return nil, fmt.Errorf("Pulse URL %q must use https/wss unless host is loopback", raw) + } + parsed.Scheme = "ws" + default: + return nil, fmt.Errorf("Pulse URL %q has unsupported scheme %q", raw, parsed.Scheme) + } + + parsed.Host = strings.ToLower(parsed.Host) + parsed.Path = strings.TrimRight(parsed.Path, "/") + parsed.RawPath = strings.TrimRight(parsed.RawPath, "/") + parsed.RawQuery = "" + parsed.Fragment = "" + + return parsed, nil +} + // AppendURLPath appends path segments onto a validated base URL. func AppendURLPath(base *url.URL, segments ...string) *url.URL { cloned := cloneURL(base) diff --git a/internal/securityutil/httpurl_test.go b/internal/securityutil/httpurl_test.go index 6e4ff320d..49c42acae 100644 --- a/internal/securityutil/httpurl_test.go +++ b/internal/securityutil/httpurl_test.go @@ -22,6 +22,148 @@ func TestNormalizeHTTPBaseURLRejectsQuery(t *testing.T) { } } +func TestIsLoopbackHost(t *testing.T) { + t.Run("true", func(t *testing.T) { + cases := []string{"localhost", "LOCALHOST", "agent.localhost", "127.0.0.1", "::1"} + for _, tc := range cases { + if !IsLoopbackHost(tc) { + t.Fatalf("expected loopback host for %q", tc) + } + } + }) + + t.Run("false", func(t *testing.T) { + cases := []string{"", "example.com", "192.168.1.10", "10.0.0.5"} + for _, tc := range cases { + if IsLoopbackHost(tc) { + t.Fatalf("expected non-loopback host for %q", tc) + } + } + }) +} + +func TestNormalizePulseHTTPBaseURL(t *testing.T) { + tests := []struct { + name string + raw string + want string + wantError string + }{ + { + name: "normalizes secure url", + raw: "HTTPS://Pulse.Example.com:7655/base/", + want: "https://pulse.example.com:7655/base", + }, + { + name: "allows loopback http", + raw: "http://LOCALHOST:7655/", + want: "http://localhost:7655", + }, + { + name: "rejects private-network http", + raw: "http://10.0.0.5:7655", + wantError: "must use https unless host is loopback", + }, + { + name: "rejects unsupported scheme", + raw: "ftp://pulse.example.com", + wantError: "unsupported scheme", + }, + { + name: "rejects query", + raw: "https://pulse.example.com?x=1", + wantError: "must not include query or fragment", + }, + { + name: "rejects bad port", + raw: "https://pulse.example.com:70000", + wantError: "invalid port", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := NormalizePulseHTTPBaseURL(tt.raw) + if tt.wantError != "" { + if err == nil { + t.Fatalf("NormalizePulseHTTPBaseURL(%q) expected error", tt.raw) + } + if !strings.Contains(err.Error(), tt.wantError) { + t.Fatalf("NormalizePulseHTTPBaseURL(%q) error = %q, want substring %q", tt.raw, err.Error(), tt.wantError) + } + return + } + if err != nil { + t.Fatalf("NormalizePulseHTTPBaseURL(%q) error = %v", tt.raw, err) + } + if got.String() != tt.want { + t.Fatalf("NormalizePulseHTTPBaseURL(%q) = %q, want %q", tt.raw, got.String(), tt.want) + } + }) + } +} + +func TestNormalizePulseWebSocketBaseURL(t *testing.T) { + tests := []struct { + name string + raw string + want string + wantError string + }{ + { + name: "https becomes wss", + raw: "https://example.invalid/pulse/", + want: "wss://example.invalid/pulse", + }, + { + name: "loopback http becomes ws", + raw: "http://localhost:7655", + want: "ws://localhost:7655", + }, + { + name: "wss preserved", + raw: "wss://example.invalid", + want: "wss://example.invalid", + }, + { + name: "non-loopback http rejected", + raw: "http://example.invalid", + wantError: "must use https/wss unless host is loopback", + }, + { + name: "private-network ws rejected", + raw: "ws://10.0.0.5:7655", + wantError: "must use https/wss unless host is loopback", + }, + { + name: "unsupported scheme rejected", + raw: "ftp://example.invalid", + wantError: "unsupported scheme", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := NormalizePulseWebSocketBaseURL(tt.raw) + if tt.wantError != "" { + if err == nil { + t.Fatalf("NormalizePulseWebSocketBaseURL(%q) expected error", tt.raw) + } + if !strings.Contains(err.Error(), tt.wantError) { + t.Fatalf("NormalizePulseWebSocketBaseURL(%q) error = %q, want substring %q", tt.raw, err.Error(), tt.wantError) + } + return + } + if err != nil { + t.Fatalf("NormalizePulseWebSocketBaseURL(%q) error = %v", tt.raw, err) + } + if got.String() != tt.want { + t.Fatalf("NormalizePulseWebSocketBaseURL(%q) = %q, want %q", tt.raw, got.String(), tt.want) + } + }) + } +} + func TestResolveRelativeURLRejectsAbsoluteURL(t *testing.T) { base, err := NormalizeHTTPBaseURL("https://example.com/api", "") if err != nil {