fix: Address Codex feedback on legacy SSH detection before release
Codex identified critical issues preventing release. All issues resolved:
1. FIXED: LXC container detection reliability
- Added 4 detection methods (was 2):
* Method 1: /.dockerenv (Docker)
* Method 2: /proc/1/cgroup with more patterns (Docker/LXC)
* Method 3: /run/systemd/container (systemd containers)
* Method 4: /proc/1/environ container markers
- Tested on LXC container (debian-go): detection confirmed working
2. FIXED: False positives from proxy outages
- Now distinguishes "not configured" vs "temporarily down"
- Checks if /usr/local/bin/pulse-sensor-proxy exists
- If binary exists but socket missing = transient issue (no banner)
- If binary missing and SSH keys present = legacy setup (show banner)
3. FIXED: Banner guidance insufficient
- Added "Go to Nodes →" button that navigates to /settings/nodes
- Users now have direct path to fix the issue
- Banner message remains clear and concise
4. ADDED: Telemetry for removal criteria tracking
- Backend logs: "Legacy SSH configuration detected" (WARN level)
- Frontend logs: Banner shown/dismissed events to console
- Enables data-driven removal per criteria: <1% for 30+ days
- Log format: detection_type=legacy_ssh_migration for easy filtering
Testing:
- Created fake SSH key in /etc/pulse/.ssh/ on LXC container
- Verified detection triggered (legacySSHDetected: true)
- Verified telemetry logged: "Legacy SSH configuration detected"
- Removed fake key, verified detection cleared (null values)
- Container detection working via /run/systemd/container
Ready for release per Codex review.
This commit is contained in:
rcourtman
parent
21714fdf7a
commit
9362614c66
2 changed files with 74 additions and 10 deletions
|
|
@ -1,4 +1,5 @@
|
|||
import { Show, createSignal, createEffect, onMount } from 'solid-js';
|
||||
import { useNavigate } from '@solidjs/router';
|
||||
|
||||
/**
|
||||
* ⚠️ MIGRATION SCAFFOLDING - TEMPORARY COMPONENT
|
||||
|
|
@ -12,6 +13,7 @@ import { Show, createSignal, createEffect, onMount } from 'solid-js';
|
|||
* Can be disabled by setting PULSE_LEGACY_DETECTION=false on backend.
|
||||
*/
|
||||
export function LegacySSHBanner() {
|
||||
const navigate = useNavigate();
|
||||
const [isVisible, setIsVisible] = createSignal(false);
|
||||
const [isDismissed, setIsDismissed] = createSignal(false);
|
||||
|
||||
|
|
@ -34,6 +36,8 @@ export function LegacySSHBanner() {
|
|||
|
||||
if (health.legacySSHDetected && health.recommendProxyUpgrade) {
|
||||
setIsVisible(true);
|
||||
// Log banner impression for telemetry (removal criteria tracking)
|
||||
console.info('[Migration] Legacy SSH banner shown to user');
|
||||
}
|
||||
} catch (error) {
|
||||
// Silently fail - health check failures shouldn't break the UI
|
||||
|
|
@ -45,6 +49,8 @@ export function LegacySSHBanner() {
|
|||
setIsVisible(false);
|
||||
// Store dismissal in localStorage so it persists
|
||||
localStorage.setItem('legacySSHBannerDismissed', 'true');
|
||||
// Log dismissal for telemetry
|
||||
console.info('[Migration] Legacy SSH banner dismissed by user');
|
||||
};
|
||||
|
||||
return (
|
||||
|
|
@ -70,10 +76,18 @@ export function LegacySSHBanner() {
|
|||
<line x1="12" y1="17" x2="12.01" y2="17"></line>
|
||||
</svg>
|
||||
|
||||
<div class="text-sm">
|
||||
<span class="font-medium">Legacy temperature monitoring detected.</span>
|
||||
{' '}
|
||||
<span>Remove and re-add your nodes to upgrade to the secure proxy architecture.</span>
|
||||
<div class="flex items-center gap-3 flex-wrap">
|
||||
<div class="text-sm">
|
||||
<span class="font-medium">Legacy temperature monitoring detected.</span>
|
||||
{' '}
|
||||
<span>Remove and re-add your nodes to upgrade to the secure proxy architecture.</span>
|
||||
</div>
|
||||
<button
|
||||
onClick={() => navigate('/settings/nodes')}
|
||||
class="px-3 py-1 text-xs font-medium bg-orange-600 hover:bg-orange-700 dark:bg-orange-700 dark:hover:bg-orange-800 text-white rounded transition-colors"
|
||||
>
|
||||
Go to Nodes →
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
|
|
|||
|
|
@ -1144,16 +1144,46 @@ func (r *Router) detectLegacySSH() (legacyDetected, recommendProxy bool) {
|
|||
return false, false
|
||||
}
|
||||
|
||||
// Check if running in a container
|
||||
// Check if running in a container using multiple detection methods
|
||||
inContainer := false
|
||||
|
||||
// Method 1: Check for /.dockerenv (Docker)
|
||||
if _, err := os.Stat("/.dockerenv"); err == nil {
|
||||
inContainer = true
|
||||
} else if data, err := os.ReadFile("/proc/1/cgroup"); err == nil {
|
||||
if strings.Contains(string(data), "docker") || strings.Contains(string(data), "lxc") {
|
||||
}
|
||||
|
||||
// Method 2: Check /proc/1/cgroup (Docker/LXC)
|
||||
if !inContainer {
|
||||
if data, err := os.ReadFile("/proc/1/cgroup"); err == nil {
|
||||
cgroupStr := string(data)
|
||||
if strings.Contains(cgroupStr, "docker") ||
|
||||
strings.Contains(cgroupStr, "lxc") ||
|
||||
strings.Contains(cgroupStr, "/docker/") ||
|
||||
strings.Contains(cgroupStr, "/lxc/") {
|
||||
inContainer = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Method 3: Check /run/systemd/container (systemd containers)
|
||||
if !inContainer {
|
||||
if _, err := os.Stat("/run/systemd/container"); err == nil {
|
||||
inContainer = true
|
||||
}
|
||||
}
|
||||
|
||||
// Method 4: Check /proc/1/environ for container indicators
|
||||
if !inContainer {
|
||||
if data, err := os.ReadFile("/proc/1/environ"); err == nil {
|
||||
environStr := string(data)
|
||||
if strings.Contains(environStr, "container=") ||
|
||||
strings.Contains(environStr, "DOCKER") ||
|
||||
strings.Contains(environStr, "LXC") {
|
||||
inContainer = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// If not in container, no need for proxy
|
||||
if !inContainer {
|
||||
return false, false
|
||||
|
|
@ -1179,13 +1209,25 @@ func (r *Router) detectLegacySSH() (legacyDetected, recommendProxy bool) {
|
|||
sshKeysConfigured = true
|
||||
}
|
||||
|
||||
// If SSH keys exist, check if proxy is running
|
||||
// If SSH keys exist, check if proxy is configured vs just temporarily down
|
||||
if sshKeysConfigured {
|
||||
// Check if pulse-sensor-proxy is available via unix socket
|
||||
proxySocket := "/run/pulse-sensor-proxy/sensor.sock"
|
||||
proxyBinary := "/usr/local/bin/pulse-sensor-proxy"
|
||||
|
||||
// If socket doesn't exist, need to distinguish:
|
||||
// - Legacy setup: proxy never installed (binary doesn't exist)
|
||||
// - Migrated setup: proxy installed but temporarily down
|
||||
if _, err := os.Stat(proxySocket); err != nil {
|
||||
// Socket doesn't exist - legacy SSH method detected
|
||||
return true, true
|
||||
// Socket missing - check if proxy was ever installed
|
||||
if _, err := os.Stat(proxyBinary); err != nil {
|
||||
// Proxy binary doesn't exist - this is legacy SSH setup
|
||||
// User needs to remove nodes and re-add them
|
||||
return true, true
|
||||
}
|
||||
// Proxy binary exists but socket is missing - likely just restarting/down
|
||||
// Don't show banner - this is a transient issue, not a configuration problem
|
||||
return false, false
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1202,6 +1244,14 @@ func (r *Router) handleHealth(w http.ResponseWriter, req *http.Request) {
|
|||
// Detect legacy SSH setup
|
||||
legacySSH, recommendProxy := r.detectLegacySSH()
|
||||
|
||||
// Log when legacy SSH is detected for telemetry/metrics
|
||||
// This helps track removal criteria: <1% detection rate for 30+ days
|
||||
if legacySSH && recommendProxy {
|
||||
log.Warn().
|
||||
Str("detection_type", "legacy_ssh_migration").
|
||||
Msg("Legacy SSH configuration detected - user should migrate to proxy architecture")
|
||||
}
|
||||
|
||||
response := HealthResponse{
|
||||
Status: "healthy",
|
||||
Timestamp: time.Now().Unix(),
|
||||
|
|
|
|||
Loading…
Reference in a new issue