diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 3feba168b..366b6db12 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -1087,6 +1087,13 @@ keys through the managed `ssh_known_hosts` store before any automated deploy 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. That same shared `internal/api/` lifecycle boundary also assumes tenant-scoped resource helpers stay on canonical unified-resource seeds: adjacent fleet and install surfaces may not revive raw tenant `StateSnapshot` fallback through diff --git a/internal/agentupdate/coverage_test.go b/internal/agentupdate/coverage_test.go index c9380c74d..4befe33b8 100644 --- a/internal/agentupdate/coverage_test.go +++ b/internal/agentupdate/coverage_test.go @@ -212,11 +212,18 @@ func TestValidatePulseURL(t *testing.T) { } }) - t.Run("AllowRemoteHTTPInInsecureMode", func(t *testing.T) { + t.Run("RejectRemoteHTTPInInsecureMode", func(t *testing.T) { u := newUpdaterForTest("http://pulse.example.com") u.cfg.InsecureSkipVerify = true - if err := u.validatePulseURL(); err != nil { - t.Fatalf("expected remote http URL in insecure mode to be valid, got %v", err) + if err := u.validatePulseURL(); err == nil { + t.Fatalf("expected remote http URL in insecure mode to be rejected") + } + }) + + t.Run("RejectPrivateNetworkHTTP", func(t *testing.T) { + u := newUpdaterForTest("http://10.0.0.5:7655") + if err := u.validatePulseURL(); err == nil { + t.Fatalf("expected private-network http URL to be rejected") } }) @@ -804,8 +811,7 @@ func TestPerformUpdateDownloadErrorAndHeaders(t *testing.T) { func TestPerformUpdateDownloadRetriesTransientError(t *testing.T) { _, execPath := writeTempExec(t) - u := newUpdaterForTest("http://example") - u.cfg.InsecureSkipVerify = true + u := newUpdaterForTest("https://example") u.cfg.APIToken = "token" data := testBinary() diff --git a/internal/agentupdate/update.go b/internal/agentupdate/update.go index 4d4159ee6..d412a89ad 100644 --- a/internal/agentupdate/update.go +++ b/internal/agentupdate/update.go @@ -568,14 +568,6 @@ func isLoopbackHost(host string) bool { return ip != nil && ip.IsLoopback() } -func isPrivateNetworkHost(host string) bool { - if host == "" { - return false - } - ip := net.ParseIP(strings.Trim(host, "[]")) - return ip != nil && ip.IsPrivate() -} - func (u *Updater) validatePulseURL() error { pulseURL := strings.TrimSpace(u.cfg.PulseURL) if pulseURL == "" { @@ -601,10 +593,10 @@ func (u *Updater) validatePulseURL() error { case "https": return nil case "http": - if u.cfg.InsecureSkipVerify || isLoopbackHost(parsed.Hostname()) || isPrivateNetworkHost(parsed.Hostname()) { + if isLoopbackHost(parsed.Hostname()) { return nil } - return fmt.Errorf("HTTP Pulse URL is only allowed for localhost/loopback, private networks, or when insecure mode is enabled") + return fmt.Errorf("HTTP Pulse URL is only allowed for localhost/loopback") default: return fmt.Errorf("unsupported Pulse URL scheme %q", parsed.Scheme) } diff --git a/internal/hostagent/agent.go b/internal/hostagent/agent.go index 1dd7965a1..6eaef02cb 100644 --- a/internal/hostagent/agent.go +++ b/internal/hostagent/agent.go @@ -867,8 +867,8 @@ func normalizePulseURL(rawURL string) (string, error) { 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) + 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) @@ -881,19 +881,19 @@ func normalizePulseURL(rawURL string) (string, error) { 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") { +// 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(host) + ip := net.ParseIP(normalized) if ip == nil { return false } - return ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() + return ip.IsLoopback() } // collectTemperatures attempts to collect temperature data from the local system. diff --git a/internal/hostagent/agent_new_test.go b/internal/hostagent/agent_new_test.go index 7a6834932..a1387c71c 100644 --- a/internal/hostagent/agent_new_test.go +++ b/internal/hostagent/agent_new_test.go @@ -390,7 +390,12 @@ func TestNew_RejectsInvalidPulseURL(t *testing.T) { { name: "non-loopback http rejected", url: "http://example.com", - want: "must use https unless host is loopback or private network", + want: "must use https unless host is loopback", + }, + { + name: "private-network http rejected", + url: "http://10.0.0.5:7655", + want: "must use https unless host is loopback", }, { name: "query rejected", diff --git a/internal/hostagent/command_client_test.go b/internal/hostagent/command_client_test.go index fca5a8318..655e5c2ae 100644 --- a/internal/hostagent/command_client_test.go +++ b/internal/hostagent/command_client_test.go @@ -98,11 +98,21 @@ func TestCommandClientBuildWebSocketURL(t *testing.T) { pulseURL: "http://example.invalid", wantErr: true, }, + { + name: "private-network http rejected", + pulseURL: "http://10.0.0.5:7655", + wantErr: true, + }, { name: "non-loopback ws rejected", pulseURL: "ws://example.invalid", wantErr: true, }, + { + name: "private-network ws rejected", + pulseURL: "ws://10.0.0.5:7655", + wantErr: true, + }, { name: "query rejected", pulseURL: "https://example.invalid?x=1", diff --git a/internal/hostagent/commands.go b/internal/hostagent/commands.go index ca0faaa7d..6074d4cc2 100644 --- a/internal/hostagent/commands.go +++ b/internal/hostagent/commands.go @@ -374,15 +374,15 @@ func (c *CommandClient) buildWebSocketURL() (string, error) { case "https": parsed.Scheme = "wss" case "http": - if !isLoopbackOrPrivateHost(parsed.Hostname()) { - return "", fmt.Errorf("pulse URL %q must use https/wss unless host is loopback or private network", c.pulseURL) + 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 !isLoopbackOrPrivateHost(parsed.Hostname()) { - return "", fmt.Errorf("pulse URL %q must use https/wss unless host is loopback or private network", c.pulseURL) + if !isLoopbackHost(parsed.Hostname()) { + return "", fmt.Errorf("pulse URL %q must use https/wss unless host is loopback", c.pulseURL) } parsed.Scheme = "ws" default: diff --git a/internal/hostagent/commands_deploy.go b/internal/hostagent/commands_deploy.go index 1331486a5..3e910685b 100644 --- a/internal/hostagent/commands_deploy.go +++ b/internal/hostagent/commands_deploy.go @@ -256,6 +256,13 @@ func (c *CommandClient) preflightTarget( "preflight_complete", "failed", fmt.Sprintf("Invalid node IP: %s", target.NodeIP), data, false) return false } + normalizedPulseURL, err := normalizePulseURL(pulseURL) + if err != nil { + data := marshalPreflightResult(false, false, false, "", err.Error()) + c.sendDeployProgress(conn, requestID, jobID, target.TargetID, + "preflight_complete", "failed", fmt.Sprintf("Invalid Pulse URL: %v", err), data, false) + return false + } // 1. SSH reachability check. c.sendDeployProgress(conn, requestID, jobID, target.TargetID, @@ -303,7 +310,7 @@ func (c *CommandClient) preflightTarget( c.sendDeployProgress(conn, requestID, jobID, target.TargetID, "preflight_reachability", "started", "Checking Pulse URL reachability", "", false) - reachable := c.checkPulseReachabilitySSH(tctx, target.NodeIP, pulseURL) + reachable := c.checkPulseReachabilitySSH(tctx, target.NodeIP, normalizedPulseURL) reachStatus := "ok" reachMsg := "Pulse URL reachable" if !reachable { @@ -348,6 +355,13 @@ func (c *CommandClient) installTarget( "install_complete", "failed", fmt.Sprintf("Invalid node IP: %s", target.NodeIP), data, false) return false } + normalizedPulseURL, err := normalizePulseURL(pulseURL) + if err != nil { + data := marshalInstallResult(-1, err.Error()) + c.sendDeployProgress(conn, requestID, jobID, target.TargetID, + "install_complete", "failed", fmt.Sprintf("Invalid Pulse URL: %v", err), data, false) + return false + } // 1. Write bootstrap token to target via SSH stdin. c.sendDeployProgress(conn, requestID, jobID, target.TargetID, @@ -366,7 +380,7 @@ func (c *CommandClient) installTarget( c.sendDeployProgress(conn, requestID, jobID, target.TargetID, "install_execute", "started", "Running install script", "", false) - exitCode, output, err := c.runInstallSSH(tctx, target.NodeIP, pulseURL) + exitCode, output, err := c.runInstallSSH(tctx, target.NodeIP, normalizedPulseURL) if err != nil || exitCode != 0 { msg := fmt.Sprintf("Install failed (exit %d)", exitCode) if err != nil { @@ -484,13 +498,18 @@ func (c *CommandClient) writeTokenSSH(ctx context.Context, nodeIP, token string) // runInstallSSH runs the Pulse install script on a remote node via SSH. func (c *CommandClient) runInstallSSH(ctx context.Context, nodeIP, pulseURL string) (int, string, error) { + normalizedPulseURL, err := normalizePulseURL(pulseURL) + if err != nil { + return -1, "", err + } + // SSH concatenates all command arguments into a single string passed to the // remote shell. We use shellescape (single quotes) for the URL, which prevents // all shell expansion ($(), backticks, $VAR). We invoke bash explicitly so // pipefail works regardless of the remote user's default shell (e.g. dash/ash). // The single-quoted URL is embedded in the outer single-quoted bash -c argument // using the '\'' escape pattern (end quote, literal quote, resume quote). - escapedURL := shellescape(pulseURL) + escapedURL := shellescape(normalizedPulseURL) innerCmd := fmt.Sprintf( "set -o pipefail; curl -sfL -- %s/install.sh | bash -s -- --non-interactive --token-file /run/pulse-agent/bootstrap.token --pulse-url %s --enroll --enable-commands --enable-proxmox", escapedURL, escapedURL, diff --git a/internal/hostagent/commands_deploy_test.go b/internal/hostagent/commands_deploy_test.go index 7589934c9..7f4ad01b5 100644 --- a/internal/hostagent/commands_deploy_test.go +++ b/internal/hostagent/commands_deploy_test.go @@ -150,7 +150,7 @@ func TestRunInstallSSH_IncludesEnrollAndEnableCommands(t *testing.T) { sshKnownHosts: stubKnownHostsManager{path: "/tmp/pulse-test-known-hosts"}, } - exitCode, _, err := c.runInstallSSH(context.Background(), "10.0.0.1", "http://10.0.0.1:7655") + exitCode, _, err := c.runInstallSSH(context.Background(), "10.0.0.1", "https://10.0.0.1:7655") if err != nil { t.Fatalf("runInstallSSH error: %v", err) } @@ -179,3 +179,14 @@ func TestRunInstallSSH_IncludesEnrollAndEnableCommands(t *testing.T) { t.Error("SSH install command does not isolate global known_hosts state") } } + +func TestRunInstallSSH_RejectsNonLoopbackPlainHTTP(t *testing.T) { + c := &CommandClient{ + logger: zerolog.New(zerolog.NewTestWriter(t)), + sshKnownHosts: stubKnownHostsManager{path: "/tmp/pulse-test-known-hosts"}, + } + + if _, _, err := c.runInstallSSH(context.Background(), "10.0.0.1", "http://10.0.0.1:7655"); err == nil { + t.Fatal("expected non-loopback plain HTTP Pulse URL to be rejected") + } +}