From bcd8d4e0fafaa8be987710d9403ab88b225d3122 Mon Sep 17 00:00:00 2001
From: rcourtman <courtmanr@gmail.com>
Date: Sat, 15 Nov 2025 00:33:41 +0000
Subject: [PATCH] Fix critical cleanup implementation issues found by Codex
 review

**Host Detection**:
- Now detects localhost by hostname and FQDN, not just IP
- Fixes issue where nodes configured as https://hostname:8006 would skip
  localhost cleanup (API tokens, bind mounts, service removal)

**Systemd Sandbox**:
- Added /etc/pve and /etc/systemd/system to ReadWritePaths
- Allows cleanup script to modify Proxmox configs and systemd units

**Uninstaller Improvements**:
- Use UUID for transient unit names (prevents same-second collisions)
- Added --purge flag for complete removal
- Added --wait and --collect flags to capture exit code
- Now fails cleanup if uninstaller exits non-zero

**Path Migration**:
- Fixed all /usr/local references to use /opt/pulse/sensor-proxy
- Updated forced command in SSH authorized_keys
- Updated self-heal script installer path
- Updated Go backend removal helpers (supports both new and legacy paths)

These fixes address Codex findings: hostname detection, sandbox permissions,
transient unit collisions, incomplete purging, and incomplete path migration.

Related to cleanup implementation testing.
---
 internal/api/config_handlers.go | 18 ++++++++++------
 scripts/install-sensor-proxy.sh | 38 ++++++++++++++++++++++-----------
 2 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/internal/api/config_handlers.go b/internal/api/config_handlers.go
index b5b6934..b647f12 100644
--- a/internal/api/config_handlers.go
+++ b/internal/api/config_handlers.go
@@ -3528,8 +3528,12 @@ if [[ $MAIN_ACTION =~ ^[2Rr]$ ]]; then
     echo ""
 
     # Run cleanup helper to remove SSH keys from remote nodes
-    if [ -x /usr/local/bin/pulse-sensor-cleanup.sh ]; then
+    if [ -x /opt/pulse/sensor-proxy/bin/pulse-sensor-cleanup.sh ]; then
         echo "  • Running cleanup helper..."
+        /opt/pulse/sensor-proxy/bin/pulse-sensor-cleanup.sh 2>/dev/null || echo "  ℹ️  Cleanup helper completed"
+        echo ""
+    elif [ -x /usr/local/bin/pulse-sensor-cleanup.sh ]; then
+        echo "  • Running cleanup helper (legacy path)..."
         /usr/local/bin/pulse-sensor-cleanup.sh 2>/dev/null || echo "  ℹ️  Cleanup helper completed"
         echo ""
     fi
@@ -3569,15 +3573,17 @@ if [[ $MAIN_ACTION =~ ^[2Rr]$ ]]; then
             fi
         fi
 
-        # Remove pulse-sensor-proxy binary
+        # Remove pulse-sensor-proxy binaries (both new and legacy paths)
+        if [ -d /opt/pulse/sensor-proxy ]; then
+            echo "  • Removing pulse-sensor-proxy installation..."
+            rm -rf /opt/pulse/sensor-proxy
+        fi
         if [ -f /usr/local/bin/pulse-sensor-proxy ]; then
-            echo "  • Removing pulse-sensor-proxy binary..."
+            echo "  • Removing legacy pulse-sensor-proxy binary..."
             rm -f /usr/local/bin/pulse-sensor-proxy
         fi
-
-        # Remove cleanup helper script
         if [ -f /usr/local/bin/pulse-sensor-cleanup.sh ]; then
-            echo "  • Removing cleanup helper script..."
+            echo "  • Removing legacy cleanup helper script..."
             rm -f /usr/local/bin/pulse-sensor-cleanup.sh
         fi
 
diff --git a/scripts/install-sensor-proxy.sh b/scripts/install-sensor-proxy.sh
index a9c2c66..27f25a0 100755
--- a/scripts/install-sensor-proxy.sh
+++ b/scripts/install-sensor-proxy.sh
@@ -1761,13 +1761,16 @@ if [[ -z "$HOST" ]]; then
 else
     log_info "Cleaning up specific host: $HOST"
 
-    # Extract IP from host URL
+    # Extract hostname/IP from host URL
     HOST_CLEAN=$(echo "$HOST" | sed -e 's|^https\?://||' -e 's|:.*$||')
 
-    # Check if this is localhost
+    # Check if this is localhost (by IP, hostname, or FQDN)
     LOCAL_IPS=$(hostname -I 2>/dev/null || echo "")
+    LOCAL_HOSTNAME=$(hostname 2>/dev/null || echo "")
+    LOCAL_FQDN=$(hostname -f 2>/dev/null || echo "")
     IS_LOCAL=false
 
+    # Check against all local IPs
     for local_ip in $LOCAL_IPS; do
         if [[ "$HOST_CLEAN" == "$local_ip" ]]; then
             IS_LOCAL=true
@@ -1775,7 +1778,9 @@ else
         fi
     done
 
-    if [[ "$HOST_CLEAN" == "127.0.0.1" || "$HOST_CLEAN" == "localhost" ]]; then
+    # Check against hostname and FQDN
+    if [[ "$HOST_CLEAN" == "127.0.0.1" || "$HOST_CLEAN" == "localhost" || \
+          "$HOST_CLEAN" == "$LOCAL_HOSTNAME" || "$HOST_CLEAN" == "$LOCAL_FQDN" ]]; then
         IS_LOCAL=true
     fi
 
@@ -1837,7 +1842,9 @@ else
             # Use systemd-run to create isolated transient unit that won't be killed
             # when we stop pulse-sensor-proxy.service
             if command -v systemd-run >/dev/null 2>&1; then
-                UNINSTALL_UNIT="pulse-uninstall-$(date +%s)"
+                # Use UUID for unique unit name (prevents same-second collisions)
+                UNINSTALL_UUID=$(cat /proc/sys/kernel/random/uuid 2>/dev/null || date +%s%N)
+                UNINSTALL_UNIT="pulse-uninstall-${UNINSTALL_UUID}"
                 log_info "Spawning isolated uninstaller unit: $UNINSTALL_UNIT"
 
                 systemd-run \
@@ -1845,12 +1852,19 @@ else
                     --property="Type=oneshot" \
                     --property="Conflicts=pulse-sensor-proxy.service" \
                     --property="After=pulse-sensor-cleanup.service" \
-                    --no-block \
+                    --collect \
+                    --wait \
                     --quiet \
-                    -- bash -c "$INSTALLER_PATH --uninstall --quiet >> /var/log/pulse/sensor-proxy/uninstall.log 2>&1" \
+                    -- bash -c "$INSTALLER_PATH --uninstall --purge --quiet >> /var/log/pulse/sensor-proxy/uninstall.log 2>&1" \
                     2>&1 | logger -t "$LOG_TAG" -p user.info
 
-                log_info "Uninstaller started in isolated systemd unit (non-blocking)"
+                UNINSTALL_EXIT=$?
+                if [[ $UNINSTALL_EXIT -eq 0 ]]; then
+                    log_info "Uninstaller completed successfully"
+                else
+                    log_error "Uninstaller failed with exit code $UNINSTALL_EXIT"
+                    exit 1
+                fi
             else
                 log_warn "systemd-run not available, attempting direct uninstall (may fail)"
                 bash "$INSTALLER_PATH" --uninstall --quiet >> /var/log/pulse/sensor-proxy/uninstall.log 2>&1 || \
@@ -1938,10 +1952,10 @@ StandardOutput=journal
 StandardError=journal
 SyslogIdentifier=pulse-sensor-cleanup
 
-# Security hardening (less restrictive than the proxy since we need SSH access)
+# Security hardening (less restrictive than the proxy since we need SSH access and Proxmox config access)
 NoNewPrivileges=true
 ProtectSystem=strict
-ReadWritePaths=/var/lib/pulse-sensor-proxy /root/.ssh
+ReadWritePaths=/var/lib/pulse-sensor-proxy /root/.ssh /etc/pve /etc/systemd/system
 ProtectKernelTunables=true
 ProtectKernelModules=true
 ProtectControlGroups=true
@@ -1994,7 +2008,7 @@ if command -v pvecm >/dev/null 2>&1; then
         print_info "Discovered cluster nodes: $(echo $CLUSTER_NODES | tr '\n' ' ')"
 
         # Configure SSH key with forced command restriction
-        FORCED_CMD='command="/usr/local/bin/pulse-sensor-wrapper.sh",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty'
+        FORCED_CMD='command="/opt/pulse/sensor-proxy/bin/pulse-sensor-wrapper.sh",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty'
         AUTH_LINE="${FORCED_CMD} ${PROXY_PUBLIC_KEY} # pulse-managed-key"
 
         # Track SSH key push results
@@ -2084,7 +2098,7 @@ if command -v pvecm >/dev/null 2>&1; then
         print_info "No cluster detected, configuring standalone node..."
 
         # Configure SSH key with forced command restriction
-        FORCED_CMD='command="/usr/local/bin/pulse-sensor-wrapper.sh",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty'
+        FORCED_CMD='command="/opt/pulse/sensor-proxy/bin/pulse-sensor-wrapper.sh",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty'
         AUTH_LINE="${FORCED_CMD} ${PROXY_PUBLIC_KEY} # pulse-managed-key"
 
         print_info "Authorizing proxy key on localhost..."
@@ -2355,7 +2369,7 @@ cat > "$SELFHEAL_SCRIPT" <<'EOF'
 set -euo pipefail
 
 SERVICE="pulse-sensor-proxy"
-INSTALLER="/usr/local/share/pulse/install-sensor-proxy.sh"
+INSTALLER="/opt/pulse/sensor-proxy/install-sensor-proxy.sh"
 CTID_FILE="/etc/pulse-sensor-proxy/ctid"
 LOG_TAG="pulse-sensor-proxy-selfheal"