From 7d2a2bd97843f2a665563821a9e491f6a28539a7 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Thu, 13 Nov 2025 14:15:51 +0000 Subject: [PATCH] Fix security regression: use localhost-only fallback instead of permissive mode Codex independent review identified a critical security issue: when cluster validation fails, the previous fix fell back to permissive mode (allowing ALL nodes), making the proxy a potential SSRF/network scanner for any container that could reach the socket. NEW BEHAVIOR: When cluster validation is unavailable (IPC blocked), fall back to localhost-only validation instead of permissive mode. This maintains security while still allowing self-monitoring. Implementation: - Added validateAsLocalhost() method to nodeValidator - Calls discoverLocalHostAddresses() to get local IPs/hostnames - Only allows requests matching the local host - Blocks requests to other cluster members or arbitrary hosts Test results on delly (clustered node with IPC blocked): - Request to 192.168.0.5 (self): ALLOWED, temps fetched - Request to 192.168.0.134 (cluster peer): BLOCKED with node_not_localhost - No more "allowing all nodes" security regression Related to #571 - addresses Codex security audit feedback This prevents the proxy from being abused as a network scanner while still solving the original temperature monitoring issue. --- cmd/pulse-sensor-proxy/validation.go | 42 ++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/cmd/pulse-sensor-proxy/validation.go b/cmd/pulse-sensor-proxy/validation.go index 5949eb6..fb294c3 100644 --- a/cmd/pulse-sensor-proxy/validation.go +++ b/cmd/pulse-sensor-proxy/validation.go @@ -358,13 +358,14 @@ func (v *nodeValidator) Validate(ctx context.Context, node string) error { allowed, err := v.matchesCluster(ctx, node) if err != nil { // Cluster query failed (e.g., IPC permission denied, running in LXC) - // Fall through to permissive mode rather than blocking all requests + // Fall back to localhost-only validation instead of permissive mode v.recordFailure(validationReasonClusterFailed) log.Warn(). Err(err). Str("node", node). - Msg("SECURITY: Cluster validation unavailable - allowing all nodes. Configure allowed_nodes in config to restrict access.") - // Fall through to permissive mode below + Msg("SECURITY: Cluster validation unavailable - falling back to localhost-only validation. Configure allowed_nodes for cluster-wide access.") + // Attempt to validate against localhost addresses + return v.validateAsLocalhost(ctx, node) } else if !allowed { return v.deny(node, validationReasonNotClusterMember) } else { @@ -379,6 +380,41 @@ func (v *nodeValidator) Validate(ctx context.Context, node string) error { return nil } +func (v *nodeValidator) validateAsLocalhost(ctx context.Context, node string) error { + // When cluster validation is unavailable, only allow access to localhost + // This maintains security while allowing self-monitoring + if ctx == nil { + ctx = context.Background() + } + + // Try to discover local host addresses + localAddrs, err := discoverLocalHostAddresses() + if err != nil { + log.Warn().Err(err).Msg("Failed to discover local host addresses for fallback validation") + // If we can't even discover localhost, deny access + return v.deny(node, validationReasonClusterFailed) + } + + // Check if the requested node matches any local address + normalized := normalizeAllowlistEntry(node) + if normalized == "" { + normalized = strings.ToLower(strings.TrimSpace(node)) + } + + for _, localAddr := range localAddrs { + if strings.EqualFold(localAddr, normalized) { + log.Debug(). + Str("node", node). + Str("matched_local", localAddr). + Msg("Node validated as localhost (cluster validation unavailable)") + return nil + } + } + + // Node doesn't match any local address - deny + return v.deny(node, "node_not_localhost") +} + func (v *nodeValidator) matchesAllowlist(ctx context.Context, node string) (bool, error) { normalized := normalizeAllowlistEntry(node) if normalized != "" {