From dd2beffc8cc37bf1376e8f9ec39e690c7dd962e9 Mon Sep 17 00:00:00 2001
From: rcourtman <courtmanr@gmail.com>
Date: Wed, 22 Oct 2025 19:35:51 +0000
Subject: [PATCH] Stop legacy temperature SSH retries when auth fails (#595)

---
 internal/monitoring/temperature.go      | 70 +++++++++++++++++++++++++
 internal/monitoring/temperature_test.go | 21 ++++++++
 2 files changed, 91 insertions(+)

diff --git a/internal/monitoring/temperature.go b/internal/monitoring/temperature.go
index 0a921c7..eb35475 100644
--- a/internal/monitoring/temperature.go
+++ b/internal/monitoring/temperature.go
@@ -12,6 +12,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/rcourtman/pulse-go-rewrite/internal/models"
@@ -40,6 +41,8 @@ type TemperatureCollector struct {
 	proxyMu            sync.Mutex
 	proxyFailures      int
 	proxyCooldownUntil time.Time
+	missingKeyWarned   atomic.Bool
+	legacySSHDisabled  atomic.Bool
 }
 
 // NewTemperatureCollector creates a new temperature collector
@@ -106,10 +109,28 @@ func (tc *TemperatureCollector) CollectTemperature(ctx context.Context, nodeHost
 			return &models.Temperature{Available: false}, nil
 		}
 
+		if tc.legacySSHDisabled.Load() {
+			return &models.Temperature{Available: false}, nil
+		}
+
+		if strings.TrimSpace(tc.sshKeyPath) == "" {
+			tc.logMissingSSHKey(nil)
+			return &models.Temperature{Available: false}, nil
+		}
+
+		if _, keyErr := os.Stat(tc.sshKeyPath); keyErr != nil {
+			tc.logMissingSSHKey(keyErr)
+			return &models.Temperature{Available: false}, nil
+		}
+
 		// Direct SSH (legacy method for non-containerized deployments)
 		// Try sensors first, fall back to Raspberry Pi method if that fails
 		output, err = tc.runSSHCommand(ctx, host, "sensors -j 2>/dev/null")
 		if err != nil || strings.TrimSpace(output) == "" {
+			if tc.disableLegacySSHOnAuthFailure(err, nodeName, host) {
+				return &models.Temperature{Available: false}, nil
+			}
+
 			// Try Raspberry Pi temperature method
 			output, err = tc.runSSHCommand(ctx, host, "cat /sys/class/thermal/thermal_zone0/temp 2>/dev/null")
 			if err == nil && strings.TrimSpace(output) != "" {
@@ -120,6 +141,10 @@ func (tc *TemperatureCollector) CollectTemperature(ctx context.Context, nodeHost
 				}
 			}
 
+			if tc.disableLegacySSHOnAuthFailure(err, nodeName, host) {
+				return &models.Temperature{Available: false}, nil
+			}
+
 			log.Debug().
 				Str("node", nodeName).
 				Str("host", host).
@@ -150,6 +175,12 @@ func (tc *TemperatureCollector) CollectTemperature(ctx context.Context, nodeHost
 
 // runSSHCommand executes a command on a remote node via SSH
 func (tc *TemperatureCollector) runSSHCommand(ctx context.Context, host, command string) (string, error) {
+	if strings.TrimSpace(tc.sshKeyPath) != "" {
+		if _, err := os.Stat(tc.sshKeyPath); err != nil {
+			return "", fmt.Errorf("temperature SSH key unavailable: %w", err)
+		}
+	}
+
 	if err := tc.ensureHostKey(ctx, host); err != nil {
 		return "", err
 	}
@@ -210,6 +241,45 @@ func (tc *TemperatureCollector) runSSHCommand(ctx context.Context, host, command
 	return outputStr, nil
 }
 
+func (tc *TemperatureCollector) logMissingSSHKey(cause error) {
+	if tc.missingKeyWarned.Load() {
+		return
+	}
+	if tc.missingKeyWarned.CompareAndSwap(false, true) {
+		event := log.Debug().
+			Str("sshKeyPath", tc.sshKeyPath)
+		if cause != nil && !errors.Is(cause, os.ErrNotExist) {
+			event = event.Err(cause)
+		}
+		event.Msg("Temperature SSH key not available; skipping legacy SSH collection")
+	}
+}
+
+func (tc *TemperatureCollector) disableLegacySSHOnAuthFailure(err error, nodeName, host string) bool {
+	if err == nil {
+		return false
+	}
+
+	msg := strings.ToLower(err.Error())
+	authFailure := strings.Contains(msg, "permission denied") ||
+		strings.Contains(msg, "authentication failed") ||
+		strings.Contains(msg, "publickey")
+
+	if !authFailure {
+		return false
+	}
+
+	if tc.legacySSHDisabled.CompareAndSwap(false, true) {
+		log.Warn().
+			Str("node", nodeName).
+			Str("host", host).
+			Err(err).
+			Msg("Disabling legacy SSH temperature collection after authentication failure; configure pulse-sensor-proxy or adjust SSH access.")
+	}
+
+	return true
+}
+
 // parseSensorsJSON parses the JSON output from `sensors -j`
 func (tc *TemperatureCollector) parseSensorsJSON(jsonStr string) (*models.Temperature, error) {
 	if strings.TrimSpace(jsonStr) == "" {
diff --git a/internal/monitoring/temperature_test.go b/internal/monitoring/temperature_test.go
index 5712f6b..926aa23 100644
--- a/internal/monitoring/temperature_test.go
+++ b/internal/monitoring/temperature_test.go
@@ -561,3 +561,24 @@ func TestTemperatureCollector_ConcurrentCollectTemperature(t *testing.T) {
 		t.Fatalf("expected proxy failures to stay below disable threshold, got %d", collector.proxyFailures)
 	}
 }
+
+func TestDisableLegacySSHOnAuthFailure(t *testing.T) {
+	collector := &TemperatureCollector{}
+
+	if !collector.disableLegacySSHOnAuthFailure(fmt.Errorf("ssh command failed: Permission denied (publickey)."), "node-1", "host-1") {
+		t.Fatalf("expected authentication errors to disable legacy SSH")
+	}
+	if !collector.legacySSHDisabled.Load() {
+		t.Fatalf("expected legacy SSH to be marked disabled")
+	}
+
+	// Repeated auth errors should still return true but not change the flag.
+	if !collector.disableLegacySSHOnAuthFailure(fmt.Errorf("permission denied"), "node-1", "host-1") {
+		t.Fatalf("expected repeated authentication errors to continue reporting disabled state")
+	}
+
+	// Non-authentication errors should not trigger disablement.
+	if collector.disableLegacySSHOnAuthFailure(fmt.Errorf("connection timed out"), "node-1", "host-1") {
+		t.Fatalf("expected non-authentication errors to leave legacy SSH enabled")
+	}
+}