Remove ~900 lines of unused code identified by static analysis:
Go:
- internal/logging: Remove 10 unused functions (InitFromConfig, New,
FromContext, WithLogger, etc.) that were built but never integrated
- cmd/pulse-sensor-proxy: Remove 7 dead validation functions for a
removed command execution feature
- internal/metrics: Remove 8 unused notification metric functions and
10 Prometheus metrics that were never wired up
Frontend:
- Delete ActivationBanner.tsx stub component
- Remove unused exports: stopMetricsSampler, getSamplerStatus,
formatSpeedCompact, parseMetricKey, getResourceAlerts
- Fix SA4006 unused value issues in ssh.go, validation.go, generator.go
- Replace deprecated ioutil with io/os in config.go
- Replace deprecated tar.TypeRegA with tar.TypeReg
- Remove deprecated rand.Seed calls (auto-seeded in Go 1.20+)
- Fix always-true nil check in main.go
- Fix impossible nil comparison in tempproxy/client.go
- Add nil check for config in monitor.New()
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.
Changes based on independent Codex review:
1. Elevated log level from Debug to Warn for permissive mode fallback
- Operators now see "SECURITY: Cluster validation unavailable" in
journalctl at default log level
- Added similar warning on startup when running in permissive mode
- Makes it obvious when node validation is bypassed
2. Added runtime fallback for AF_NETLINK restrictions
- New discoverLocalHostAddressesFallback() shells out to 'ip addr'
- Triggered when net.Interfaces() fails with netlinkrib error
- Ensures existing installations work even without systemd unit update
- Logs recommendation to update systemd unit for better performance
3. Improved security awareness
- Changed message to explicitly state "allowing all nodes"
- Recommends configuring allowed_nodes for security
- Makes permissive fallback behavior transparent to operators
Related to #571 - temperature monitoring on standalone nodes
These changes ensure the fix works for existing installations that
haven't updated their systemd units, while clearly communicating when
the proxy is running in an insecure permissive mode.
Root cause: pulse-sensor-proxy runs with strict systemd hardening that prevents
access to Proxmox corosync IPC (abstract UNIX sockets). When pvecm fails with
IPC errors, the code incorrectly treated it as "standalone mode" and only
discovered localhost addresses, rejecting legitimate cluster members and external
nodes.
Changes:
1. **Distinguish IPC failures from true standalone mode**
- Detect ipcc_send_rec and access control list errors specifically
- These indicate a cluster exists but isn't accessible (LXC, systemd restrictions)
- Return error to disable cluster validation instead of misusing standalone logic
2. **Graceful degradation when cluster validation fails**
- When cluster IPC is unavailable, fall through to permissive mode
- Log debug message suggesting allowed_nodes configuration
- Allows requests to proceed rather than blocking all temperature monitoring
3. **Improve local address discovery for true standalone nodes**
- Use Go's native net.Interfaces() instead of shelling out to 'ip addr'
- More reliable and works with AF_NETLINK restrictions
- Add helpful logging when only hostnames are discovered
4. **Systemd hardening adjustments**
- Add AF_NETLINK to RestrictAddressFamilies (for net.Interfaces())
- Remove RemoveIPC=true (attempted fix for corosync, insufficient)
- Add ReadWritePaths=-/run/corosync (optional path, corosync uses abstract sockets anyway)
Result: Temperature monitoring now works in:
- Clustered Proxmox hosts (falls back to permissive when IPC blocked)
- LXC containers (correctly detects IPC failure, allows requests)
- Standalone nodes (proper local address discovery with IPs)
Workaround for maximum security: Configure allowed_nodes in /etc/pulse-sensor-proxy/config.yaml
when cluster validation cannot be used.
CRITICAL security fixes for pulse-sensor-proxy:
1. Strengthened hostname validation regex:
- Now requires hostnames to start with alphanumeric character
- Prevents SSH option injection via hostnames starting with '-'
- Pattern: ^[a-zA-Z0-9][a-zA-Z0-9._-]{0,63}$ (1-64 chars total)
- Added IPv4 and IPv6 validation regexes for future use
2. Added validation to vulnerable V1 RPC handlers:
- handleGetTemperature: Now validates node parameter before SSH
- handleRegisterNodes: Now validates discovered cluster nodes
- Previously these handlers passed unsanitized input directly to SSH
3. Defense in depth:
- V2 handlers already had validation (now using improved regex)
- Multiple layers of protection against malicious node identifiers
- Validation prevents container from passing SSH options as hostnames
Without these fixes, a compromised container could potentially inject SSH
options by providing malicious node names, though the 'root@' prefix
provided some mitigation.
Addresses high-severity finding from security audit.
The name "temp-proxy" implied a temporary or incomplete implementation. The new name better reflects its purpose as a secure sensor data bridge for containerized Pulse deployments.
Changes:
- Renamed cmd/pulse-temp-proxy/ to cmd/pulse-sensor-proxy/
- Updated all path constants and binary references
- Renamed environment variables: PULSE_TEMP_PROXY_* to PULSE_SENSOR_PROXY_*
- Updated systemd service and service account name
- Updated installation, rotation, and build scripts
- Renamed hardening documentation
- Maintained backward compatibility for key removal during upgrades
