fix: Improve setup script robustness and safety (Codex review)

Applied Codex's security and reliability recommendations:

**SSH Key Safety:**
- Added "pulse-managed-key" comment marker to all SSH keys
- Removal now targets only marked keys (prevents deleting operator keys)
- Uses atomic file replacement via mktemp for authorized_keys edits

**Idempotency Improvements:**
- LXC config glob now uses nullglob to handle empty directories
- pveum token removal handles missing users gracefully (|| printf '')
- All systemctl operations wrapped with || true for non-systemd hosts
- sed operations in loops protected with || true

**Container Detection:**
- Validates container is running before IP check (pct status)
- Confirms container exists with pct config before proceeding
- Uses printf '' instead of || true for command substitution
- Handles IPv6 and multi-IP scenarios more reliably

**Network Operations:**
- curl now uses --fail --show-error --silent --location
- Error messages visible to users instead of silenced
- Better diagnostics when download fails

**Migration Safety:**
- Verifies pulse-sensor-proxy service is active before key removal
- Fallback check for binary existence if systemd unavailable
- Preserves legacy SSH keys if proxy not confirmed healthy
- Clear messaging about deferred cleanup

All cleanup operations are now fully idempotent and safe for
repeated execution, even on partially-configured hosts.

Addresses #123
This commit is contained in:
rcourtman 2025-10-13 14:19:17 +00:00
parent 4fef52ab37
commit 096801e96a

View file

@ -3070,18 +3070,20 @@ if [[ $MAIN_ACTION =~ ^[Rr]$ ]]; then
echo "" echo ""
# Stop and remove pulse-sensor-proxy service # Stop and remove pulse-sensor-proxy service
if systemctl is-active --quiet pulse-sensor-proxy 2>/dev/null; then if command -v systemctl &> /dev/null; then
echo " • Stopping pulse-sensor-proxy service..." if systemctl is-active --quiet pulse-sensor-proxy 2>/dev/null; then
systemctl stop pulse-sensor-proxy echo " • Stopping pulse-sensor-proxy service..."
fi systemctl stop pulse-sensor-proxy || true
if systemctl is-enabled --quiet pulse-sensor-proxy 2>/dev/null; then fi
echo " • Disabling pulse-sensor-proxy service..." if systemctl is-enabled --quiet pulse-sensor-proxy 2>/dev/null; then
systemctl disable pulse-sensor-proxy echo " • Disabling pulse-sensor-proxy service..."
fi systemctl disable pulse-sensor-proxy || true
if [ -f /etc/systemd/system/pulse-sensor-proxy.service ]; then fi
echo " • Removing systemd unit file..." if [ -f /etc/systemd/system/pulse-sensor-proxy.service ]; then
rm -f /etc/systemd/system/pulse-sensor-proxy.service echo " • Removing systemd unit file..."
systemctl daemon-reload rm -f /etc/systemd/system/pulse-sensor-proxy.service
systemctl daemon-reload || true
fi
fi fi
# Remove pulse-sensor-proxy binary # Remove pulse-sensor-proxy binary
@ -3102,31 +3104,37 @@ if [[ $MAIN_ACTION =~ ^[Rr]$ ]]; then
userdel pulse-sensor-proxy 2>/dev/null || true userdel pulse-sensor-proxy 2>/dev/null || true
fi fi
# Remove SSH keys from authorized_keys (all variants) # Remove SSH keys from authorized_keys (only Pulse-managed entries)
if [ -f /root/.ssh/authorized_keys ]; then if [ -f /root/.ssh/authorized_keys ]; then
echo " • Removing SSH keys from authorized_keys..." echo " • Removing SSH keys from authorized_keys..."
# Remove any line containing "sensors -j" (forced command entries) # Create temporary file for safe atomic update
sed -i '/sensors -j/d' /root/.ssh/authorized_keys 2>/dev/null || true TMP_AUTH_KEYS=$(mktemp)
# Remove any line containing "pulse-monitor" or "Pulse monitoring" comments if [ -f "$TMP_AUTH_KEYS" ]; then
sed -i '/pulse-monitor/d' /root/.ssh/authorized_keys 2>/dev/null || true # Remove only lines with pulse-managed-key marker (preserves user keys)
sed -i '/Pulse monitoring/d' /root/.ssh/authorized_keys 2>/dev/null || true grep -vF '# pulse-managed-key' /root/.ssh/authorized_keys > "$TMP_AUTH_KEYS" 2>/dev/null || true
# Preserve permissions and atomically replace
chmod 600 "$TMP_AUTH_KEYS"
mv "$TMP_AUTH_KEYS" /root/.ssh/authorized_keys
fi
fi fi
# Remove LXC bind mounts from all container configs # Remove LXC bind mounts from all container configs
if [ -d /etc/pve/lxc ]; then if [ -d /etc/pve/lxc ]; then
echo " • Removing LXC bind mounts from container configs..." echo " • Removing LXC bind mounts from container configs..."
shopt -s nullglob
for conf in /etc/pve/lxc/*.conf; do for conf in /etc/pve/lxc/*.conf; do
if [ -f "$conf" ] && grep -q "pulse-sensor-proxy" "$conf"; then if [ -f "$conf" ] && grep -q "pulse-sensor-proxy" "$conf" 2>/dev/null; then
sed -i '/pulse-sensor-proxy/d' "$conf" sed -i '/pulse-sensor-proxy/d' "$conf" || true
fi fi
done done
shopt -u nullglob
fi fi
# Remove Pulse monitoring API tokens and user # Remove Pulse monitoring API tokens and user
echo " • Removing Pulse monitoring API tokens and user..." echo " • Removing Pulse monitoring API tokens and user..."
if command -v pveum &> /dev/null; then if command -v pveum &> /dev/null; then
# List all tokens for pulse-monitor@pam and remove them # List all tokens for pulse-monitor@pam and remove them (idempotent)
TOKEN_LIST=$(pveum user token list pulse-monitor@pam 2>/dev/null | awk 'NR>3 {print $2}' | grep -v '^$' || true) TOKEN_LIST=$(pveum user token list pulse-monitor@pam 2>/dev/null | awk 'NR>3 {print $2}' | grep -v '^$' || printf '')
if [ -n "$TOKEN_LIST" ]; then if [ -n "$TOKEN_LIST" ]; then
while IFS= read -r TOKEN; do while IFS= read -r TOKEN; do
if [ -n "$TOKEN" ]; then if [ -n "$TOKEN" ]; then
@ -3134,9 +3142,9 @@ if [[ $MAIN_ACTION =~ ^[Rr]$ ]]; then
fi fi
done <<< "$TOKEN_LIST" done <<< "$TOKEN_LIST"
fi fi
# Remove the user # Remove the user (idempotent)
pveum user delete pulse-monitor@pam 2>/dev/null || true pveum user delete pulse-monitor@pam 2>/dev/null || true
# Remove custom roles # Remove custom roles (idempotent)
pveum role delete PulseMonitor 2>/dev/null || true pveum role delete PulseMonitor 2>/dev/null || true
fi fi
@ -3417,7 +3425,7 @@ else
# SSH public key embedded from Pulse server # SSH public key embedded from Pulse server
SSH_PUBLIC_KEY="%s" SSH_PUBLIC_KEY="%s"
SSH_RESTRICTED_KEY_ENTRY="command=\"sensors -j\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty $SSH_PUBLIC_KEY" SSH_RESTRICTED_KEY_ENTRY="command=\"sensors -j\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty $SSH_PUBLIC_KEY # pulse-managed-key"
TEMPERATURE_ENABLED=false TEMPERATURE_ENABLED=false
# Check if SSH key is already configured and whether it needs upgrading # Check if SSH key is already configured and whether it needs upgrading
@ -3722,10 +3730,20 @@ if command -v pct >/dev/null 2>&1 && [ "$TEMPERATURE_ENABLED" = true ]; then
if [[ "$PULSE_IP" =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$ ]]; then if [[ "$PULSE_IP" =~ ^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
# Check all containers for matching IP # Check all containers for matching IP
for CTID in $(pct list | awk 'NR>1 {print $1}'); do for CTID in $(pct list | awk 'NR>1 {print $1}'); do
CT_IP=$(pct exec "$CTID" -- hostname -I 2>/dev/null | awk '{print $1}' || true) # Verify container is running before attempting connection
if ! pct status "$CTID" 2>/dev/null | grep -q "running"; then
continue
fi
# Get container IP (handles both IPv4 and IPv6)
CT_IP=$(pct exec "$CTID" -- hostname -I 2>/dev/null | awk '{print $1}' || printf '')
if [ "$CT_IP" = "$PULSE_IP" ]; then if [ "$CT_IP" = "$PULSE_IP" ]; then
PULSE_CTID="$CTID" # Validate with pct config to ensure it's the right container
break if pct config "$CTID" >/dev/null 2>&1; then
PULSE_CTID="$CTID"
break
fi
fi fi
done done
fi fi
@ -3761,32 +3779,51 @@ if command -v pct >/dev/null 2>&1 && [ "$TEMPERATURE_ENABLED" = true ]; then
if [[ $INSTALL_PROXY =~ ^[Yy]$|^$ ]]; then if [[ $INSTALL_PROXY =~ ^[Yy]$|^$ ]]; then
# Download installer script # Download installer script
PROXY_INSTALLER="/tmp/install-sensor-proxy-$$.sh" PROXY_INSTALLER="/tmp/install-sensor-proxy-$$.sh"
if curl -fsSL "https://github.com/rcourtman/Pulse/releases/latest/download/install-sensor-proxy.sh" -o "$PROXY_INSTALLER" 2>/dev/null; then if curl --fail --show-error --silent --location \
"https://github.com/rcourtman/Pulse/releases/latest/download/install-sensor-proxy.sh" \
-o "$PROXY_INSTALLER" 2>&1; then
chmod +x "$PROXY_INSTALLER" chmod +x "$PROXY_INSTALLER"
# Run installer (suppress verbose output) # Run installer (suppress verbose output)
if "$PROXY_INSTALLER" --ctid "$PULSE_CTID" --quiet 2>&1 | grep -E "✓|⚠️|ERROR" || "$PROXY_INSTALLER" --ctid "$PULSE_CTID" 2>&1 | grep -E "✓|⚠️|ERROR"; then if "$PROXY_INSTALLER" --ctid "$PULSE_CTID" --quiet 2>&1 | grep -E "✓|⚠️|ERROR" || "$PROXY_INSTALLER" --ctid "$PULSE_CTID" 2>&1 | grep -E "✓|⚠️|ERROR"; then
# Clean up old container-based SSH keys from nodes (silent) # Verify proxy health before removing legacy keys
CLEANUP_NODES="" PROXY_HEALTHY=false
if [ "$TEMPERATURE_ENABLED" = true ]; then if command -v systemctl >/dev/null 2>&1; then
CLEANUP_NODES="$(hostname)" if systemctl is-active --quiet pulse-sensor-proxy 2>/dev/null; then
fi PROXY_HEALTHY=true
if [ -n "${OTHER_NODES_LIST+x}" ] && [ ${#OTHER_NODES_LIST[@]} -gt 0 ]; then
CLEANUP_NODES="$CLEANUP_NODES ${OTHER_NODES_LIST[*]}"
fi
for NODE in $CLEANUP_NODES; do
if [ -n "$NODE" ] && [ -n "$SSH_PUBLIC_KEY" ]; then
ssh -o BatchMode=yes -o StrictHostKeyChecking=no -o ConnectTimeout=5 -o LogLevel=ERROR \
root@"$NODE" \
"sed -i '/$SSH_PUBLIC_KEY/d' /root/.ssh/authorized_keys 2>/dev/null || true" \
>/dev/null 2>&1
fi fi
done elif [ -x /usr/local/bin/pulse-sensor-proxy ]; then
# Binary exists and is executable
PROXY_HEALTHY=true
fi
echo "" if [ "$PROXY_HEALTHY" = true ]; then
echo "✓ Secure proxy architecture enabled" # Clean up old container-based SSH keys from nodes (silent)
echo " SSH keys are managed on the host for enhanced security" CLEANUP_NODES=""
if [ "$TEMPERATURE_ENABLED" = true ]; then
CLEANUP_NODES="$(hostname)"
fi
if [ -n "${OTHER_NODES_LIST+x}" ] && [ ${#OTHER_NODES_LIST[@]} -gt 0 ]; then
CLEANUP_NODES="$CLEANUP_NODES ${OTHER_NODES_LIST[*]}"
fi
for NODE in $CLEANUP_NODES; do
if [ -n "$NODE" ] && [ -n "$SSH_PUBLIC_KEY" ]; then
ssh -o BatchMode=yes -o StrictHostKeyChecking=no -o ConnectTimeout=5 -o LogLevel=ERROR \
root@"$NODE" \
"sed -i '/$SSH_PUBLIC_KEY/d' /root/.ssh/authorized_keys 2>/dev/null || true" \
>/dev/null 2>&1
fi
done
echo ""
echo "✓ Secure proxy architecture enabled"
echo " SSH keys are managed on the host for enhanced security"
else
echo ""
echo "⚠️ Proxy installed but not yet active - keeping existing SSH keys"
echo " Legacy keys will be removed automatically on next setup run"
fi
else else
echo "" echo ""
echo "⚠️ Installation incomplete - using standard configuration" echo "⚠️ Installation incomplete - using standard configuration"