Merge commit '7508fc5edf68855b1c089fd92f1c5ccd05a16966' into maintainer-async/20261026T194603Z-delivery-trust/pulse

Change-source: pulse-maintainer

docs/release-control/v6/internal/subsystems/deployment-installability.md

@@ -759,6 +759,41 @@ Unix real-agent controls keep their independent two-second watchdog, with
 process-group cancellation and separate build/probe timings. These source
 fixtures do not establish signed native installation, persistence or recovery.
 
+### Legacy credential cleanup needs complete inventory and preserved keys
+
+Cluster cleanup must read the complete successful `pvesh` JSON response, validate
+all node identities and resolve every address before changing any SSH key file.
+Python receives its program through `-c`, not the response's stdin. Missing tools,
+empty/malformed inventory, failed reads or unresolved nodes return nonzero; they
+must not silently select local-only cleanup. An explicit `--local-only` remains
+available on each node without cluster discovery or SSH. Local keys are cleaned
+once and duplicate remote addresses once, with strict SSH host verification intact.
+
+Local key replacement uses an adjacent temporary file and preserves the original
+on read/comparison/metadata/rename failure. A no-match grep result is distinct
+from a read failure; removing an all-managed file may legitimately produce an
+empty file. Only the two existing Pulse comment suffixes are eligible, and
+unrelated keys and file metadata remain intact. Conditional helper invocation
+must propagate failure rather than print a successful key-removal claim.
+
+Opted-in API cleanup likewise requires a complete valid JSON token inventory.
+It does not reinterpret a failed or malformed response as empty or fall back to
+a guessed table. User deletion follows only successful removal of every listed
+token; unavailable tools and token/user removal failures remain nonzero through
+main, including when the caller disables shell errexit. Normal local service
+reload still runs after file cleanup. No new access is acquired or trust enrolled.
+
+`TestUninstallSensorProxyClusterInventoryUsesEveryValidatedNode`,
+`TestUninstallSensorProxyIncompleteInventoryNeverStartsKeyCleanup`,
+`TestUninstallSensorProxyMissingInventoryToolRequiresExplicitLocalOnly`,
+`TestUninstallSensorProxyKeyReplacementPreservesOriginalOnEveryFailure`,
+`TestUninstallSensorProxyLocalOnlyPropagatesKeyFailure`,
+`TestUninstallSensorProxyAPICleanupRequiresCompleteInventoryAndRemovals` and
+`TestUninstallSensorProxyMainCannotHideAPIRemovalFailure` execute closed shell
+fixtures through the repository's Go suite. Existing SSH trust and container
+state tests remain necessary. These are executable source/producer checks, not
+native Proxmox cleanup, installed recovery or reporter acceptance.
+
 ### Legacy sensor cleanup requires observed stopped targets
 
 The legacy sensor uninstaller must confirm each existing unit has stopped and

scripts/installtests/uninstall_sensor_proxy_test.go

@@ -252,7 +252,7 @@ main --quiet
 	if !ok || exitErr.ExitCode() != 23 {
 		t.Fatalf("main returned %v, want remote cleanup status 23:\n%s", err, out)
 	}
-	if !strings.Contains(string(out), "Local cleanup completed") {
+	if !strings.Contains(string(out), "Local file cleanup completed") {
 		t.Fatalf("remote trust failure did not explain local cleanup state:\n%s", out)
 	}
 	content, readErr := os.ReadFile(marker)
@@ -309,7 +309,7 @@ func TestUninstallSensorProxyScriptRemovesTempFootprintAndManagedKeys(t *testing
 		t.Fatalf("write authorized_keys: %v", err)
 	}
 
-	cmd := exec.Command("bash", repoFile("scripts", "uninstall-sensor-proxy.sh"), "--uninstall", "--purge", "--quiet")
+	cmd := exec.Command("bash", repoFile("scripts", "uninstall-sensor-proxy.sh"), "--uninstall", "--purge", "--quiet", "--local-only")
 	cmd.Env = append(os.Environ(),
 		"PATH="+binDir+string(os.PathListSeparator)+os.Getenv("PATH"),
 		"PULSE_SENSOR_PROXY_BINARY_PATH="+binaryPath,
@@ -345,3 +345,246 @@ func TestUninstallSensorProxyScriptRemovesTempFootprintAndManagedKeys(t *testing
 		t.Fatalf("non-Pulse SSH key was not preserved:\n%s", got)
 	}
 }
+
+// Only closed API/resolver/key-writer doubles are used. No live pvesh, pveum,
+// SSH connection, privileged path or container lifecycle is reached here.
+const proxyInventoryFixture = `source "$1"
+pvesh() {
+    [[ "$*" == 'get /nodes --output-format json' ]] || return 90
+    cat "$FIXTURE_DIR/inventory"
+    [[ "$SCENARIO" != read_failure ]]
+}
+getent() {
+    [[ "$1" == ahostsv4 || "$1" == hosts ]] || return 90
+    case "$2" in
+        pve-a) echo '192.0.2.10 STREAM pve-a' ;;
+        pve-b)
+            [[ "$SCENARIO" != unresolved ]] || return 2
+            if [[ "$SCENARIO" == resolver_failure ]]; then echo '192.0.2.11 STREAM pve-b'; return 2; fi
+            if [[ "$SCENARIO" == invalid_address ]]; then echo 'not-an-address pve-b'; else echo '192.0.2.11 STREAM pve-b'; fi ;;
+        pve-v6) [[ "$1" == hosts ]] || return 2; echo '2001:db8::12 pve-v6' ;;
+        *) return 2 ;;
+    esac
+}
+is_local_host() { [[ "$1" == 192.0.2.10 ]]; }
+cleanup_local_authorized_keys() { printf 'local\n' >>"$FIXTURE_DIR/actions"; }
+cleanup_remote_authorized_keys() { printf 'remote %s\n' "$1" >>"$FIXTURE_DIR/actions"; }
+if cleanup_cluster_authorized_keys; then exit 0; else exit $?; fi
+`
+
+func TestUninstallSensorProxyClusterInventoryUsesEveryValidatedNode(t *testing.T) {
+	dir := t.TempDir()
+	if err := os.WriteFile(filepath.Join(dir, "inventory"), []byte(`[{"node":"pve-b"},{"node":"pve-a"},{"node":"pve-b"},{"node":"pve-v6"}]`), 0600); err != nil {
+		t.Fatal(err)
+	}
+	out, code := runSensorProxySafetyShell(t, repoFile("scripts", "uninstall-sensor-proxy.sh"), dir, "success", proxyInventoryFixture)
+	if code != 0 {
+		t.Fatalf("exit %d: %s", code, out)
+	}
+	got, err := os.ReadFile(filepath.Join(dir, "actions"))
+	if err != nil || string(got) != "local\nremote 192.0.2.11\nremote 2001:db8::12\n" {
+		t.Fatalf("incomplete or duplicated cleanup: %q, %v", got, err)
+	}
+}
+
+func TestUninstallSensorProxyIncompleteInventoryNeverStartsKeyCleanup(t *testing.T) {
+	for _, tc := range []struct{ name, inventory string }{
+		{"empty", `[]`}, {"object", `{}`}, {"null", `null`}, {"malformed", `[{`},
+		{"missing", `[{"node":"pve-a"},{}]`}, {"wrong_type", `[{"node":7}]`},
+		{"unsafe", `[{"node":"pve-a"},{"node":"-oProxyCommand=bad"}]`},
+		{"multiline", `[{"node":"pve-a\npve-b"}]`}, {"non_object", `["pve-a"]`},
+		{"read_failure", `[{"node":"pve-a"}]`},
+		{"unresolved", `[{"node":"pve-a"},{"node":"pve-b"}]`},
+		{"invalid_address", `[{"node":"pve-a"},{"node":"pve-b"}]`},
+		{"resolver_failure", `[{"node":"pve-a"},{"node":"pve-b"}]`},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			dir := t.TempDir()
+			if err := os.WriteFile(filepath.Join(dir, "inventory"), []byte(tc.inventory), 0600); err != nil {
+				t.Fatal(err)
+			}
+			out, code := runSensorProxySafetyShell(t, repoFile("scripts", "uninstall-sensor-proxy.sh"), dir, tc.name, proxyInventoryFixture)
+			if code == 0 || !strings.Contains(out, "[WARN]") {
+				t.Fatalf("incomplete inventory accepted: exit %d: %s", code, out)
+			}
+			if _, err := os.Stat(filepath.Join(dir, "actions")); !os.IsNotExist(err) {
+				t.Fatalf("keys changed before inventory validation: %v", err)
+			}
+		})
+	}
+}
+
+func TestUninstallSensorProxyMissingInventoryToolRequiresExplicitLocalOnly(t *testing.T) {
+	for _, tool := range []string{"pvesh", "python3"} {
+		t.Run(tool, func(t *testing.T) {
+			dir := t.TempDir()
+			body := `source "$1"
+command() {
+    if [[ "$1" == -v && "$2" == "$SCENARIO" ]]; then return 1; fi
+    builtin command "$@"
+}
+cleanup_local_authorized_keys() { echo local >>"$FIXTURE_DIR/actions"; }
+cleanup_remote_authorized_keys() { echo remote >>"$FIXTURE_DIR/actions"; }
+if cleanup_cluster_authorized_keys; then exit 0; else exit $?; fi
+`
+			out, code := runSensorProxySafetyShell(t, repoFile("scripts", "uninstall-sensor-proxy.sh"), dir, tool, body)
+			if code == 0 || !strings.Contains(out, "--local-only") {
+				t.Fatalf("missing tool accepted: exit %d: %s", code, out)
+			}
+			if _, err := os.Stat(filepath.Join(dir, "actions")); !os.IsNotExist(err) {
+				t.Fatalf("implicit local fallback changed keys: %v", err)
+			}
+		})
+	}
+}
+
+func TestUninstallSensorProxyKeyReplacementPreservesOriginalOnEveryFailure(t *testing.T) {
+	const original = "ssh-ed25519 AAAAlegacy pulse # pulse-managed-key\nssh-ed25519 AAAAkeep user-key\n"
+	for _, phase := range []string{"mktemp", "grep", "cmp", "chmod", "chown", "mv", "success", "all_managed", "unchanged", "missing"} {
+		t.Run(phase, func(t *testing.T) {
+			dir := t.TempDir()
+			keys := original
+			if phase == "all_managed" {
+				keys = "ssh-ed25519 AAAAlegacy pulse # pulse-proxy-key\n"
+			} else if phase == "unchanged" {
+				keys = "ssh-ed25519 AAAAkeep user-key\n"
+			}
+			path := filepath.Join(dir, "authorized_keys")
+			if phase != "missing" {
+				if err := os.WriteFile(path, []byte(keys), 0640); err != nil {
+					t.Fatal(err)
+				}
+			}
+			body := `source "$1"
+AUTHORIZED_KEYS_PATH="$FIXTURE_DIR/authorized_keys"
+mktemp() { [[ "$SCENARIO" != mktemp ]] || return 9; command mktemp "$@"; }
+grep() { command grep "$@"; local code=$?; [[ "$SCENARIO" != grep ]] || return 2; return "$code"; }
+cmp() { [[ "$SCENARIO" != cmp ]] || return 2; command cmp "$@"; }
+chmod() { [[ "$SCENARIO" != chmod ]] || return 9; command chmod "$@"; }
+chown() { [[ "$SCENARIO" != chown ]] || return 9; command chown "$@"; }
+mv() { [[ "$SCENARIO" != mv ]] || return 9; command mv "$@"; }
+# Conditional invocation deliberately disables bash errexit in the helpers.
+if cleanup_local_authorized_keys; then exit 0; else exit $?; fi
+`
+			out, code := runSensorProxySafetyShell(t, repoFile("scripts", "uninstall-sensor-proxy.sh"), dir, phase, body)
+			success := phase == "success" || phase == "all_managed" || phase == "unchanged" || phase == "missing"
+			if (code == 0) != success || strings.Contains(out, "[ OK ]") != success {
+				t.Fatalf("replacement status mismatch: exit %d: %s", code, out)
+			}
+			got, err := os.ReadFile(path)
+			if phase == "missing" {
+				if !os.IsNotExist(err) {
+					t.Fatalf("missing keys created: %v", err)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatal(err)
+			}
+			want := keys
+			if phase == "success" {
+				want = "ssh-ed25519 AAAAkeep user-key\n"
+			} else if phase == "all_managed" {
+				want = ""
+			}
+			if string(got) != want {
+				t.Fatalf("keys corrupted: %q, want %q", got, want)
+			}
+			info, err := os.Stat(path)
+			if err != nil || info.Mode().Perm() != 0640 {
+				t.Fatalf("key permissions changed: %v, %v", info, err)
+			}
+			leftovers, err := filepath.Glob(path + ".pulse-cleanup.*")
+			if err != nil || len(leftovers) != 0 {
+				t.Fatalf("replacement temp files remain: %v, %v", leftovers, err)
+			}
+		})
+	}
+}
+
+func TestUninstallSensorProxyLocalOnlyPropagatesKeyFailure(t *testing.T) {
+	body := `source "$1"
+LOCAL_ONLY=true
+cleanup_local_authorized_keys() { return 23; }
+if cleanup_cluster_authorized_keys; then exit 0; else exit $?; fi
+`
+	out, code := runSensorProxySafetyShell(t, repoFile("scripts", "uninstall-sensor-proxy.sh"), t.TempDir(), "failure", body)
+	if code != 23 {
+		t.Fatalf("local-only failure hidden: exit %d: %s", code, out)
+	}
+}
+
+func TestUninstallSensorProxyAPICleanupRequiresCompleteInventoryAndRemovals(t *testing.T) {
+	for _, tc := range []struct {
+		name, inventory, actions string
+		success                  bool
+	}{
+		{"success", `[{"tokenid":"alpha"},{"tokenid":"beta"}]`, "list\nremove alpha\nremove beta\ndelete\n", true},
+		{"empty", `[]`, "list\ndelete\n", true},
+		{"opt_out", `[]`, "", true},
+		{"malformed", `{`, "list\n", false},
+		{"missing_id", `[{"tokenid":"alpha"},{}]`, "list\n", false},
+		{"unsafe_id", `[{"tokenid":"alpha"},{"tokenid":"--other"}]`, "list\n", false},
+		{"read_failure", `[{"tokenid":"alpha"}]`, "list\n", false},
+		{"remove_failure", `[{"tokenid":"alpha"},{"tokenid":"beta"}]`, "list\nremove alpha\nremove beta\n", false},
+		{"delete_failure", `[]`, "list\ndelete\n", false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			dir := t.TempDir()
+			if err := os.WriteFile(filepath.Join(dir, "inventory"), []byte(tc.inventory), 0600); err != nil {
+				t.Fatal(err)
+			}
+			body := `source "$1"
+REMOVE_PROXMOX_ACCESS=true
+[[ "$SCENARIO" != opt_out ]] || REMOVE_PROXMOX_ACCESS=false
+pveum() {
+    case "$*" in
+        'user token list pulse-monitor@pam --output-format json')
+            echo list >>"$FIXTURE_DIR/actions"
+            cat "$FIXTURE_DIR/inventory"
+            [[ "$SCENARIO" != read_failure ]] ;;
+        'user token remove pulse-monitor@pam alpha'|'user token remove pulse-monitor@pam beta')
+            echo "remove $5" >>"$FIXTURE_DIR/actions"
+            [[ "$SCENARIO" != remove_failure || "$5" != alpha ]] ;;
+        'user delete pulse-monitor@pam')
+            echo delete >>"$FIXTURE_DIR/actions"
+            [[ "$SCENARIO" != delete_failure ]] ;;
+        *) return 90 ;;
+    esac
+}
+if remove_proxmox_access; then exit 0; else exit $?; fi
+`
+			out, code := runSensorProxySafetyShell(t, repoFile("scripts", "uninstall-sensor-proxy.sh"), dir, tc.name, body)
+			if (code == 0) != tc.success {
+				t.Fatalf("API status mismatch: exit %d: %s", code, out)
+			}
+			got, err := os.ReadFile(filepath.Join(dir, "actions"))
+			if tc.name == "opt_out" && os.IsNotExist(err) {
+				return
+			}
+			if err != nil || string(got) != tc.actions {
+				t.Fatalf("unexpected API mutation: %q, %v; want %q", got, err, tc.actions)
+			}
+		})
+	}
+}
+
+func TestUninstallSensorProxyMainCannotHideAPIRemovalFailure(t *testing.T) {
+	body := `source "$1"
+disable_legacy_units() { :; }
+cleanup_cluster_authorized_keys() { :; }
+cleanup_stale_sensor_proxy_mounts() { :; }
+remove_legacy_files() { :; }
+remove_proxmox_access() { return 23; }
+systemctl_if_available() { echo reload >"$FIXTURE_DIR/actions"; }
+if main --remove-proxmox-access; then exit 0; else exit $?; fi
+`
+	dir := t.TempDir()
+	out, code := runSensorProxySafetyShell(t, repoFile("scripts", "uninstall-sensor-proxy.sh"), dir, "failure", body)
+	if code != 23 || strings.Contains(out, "cleanup complete") {
+		t.Fatalf("main hid API failure: exit %d: %s", code, out)
+	}
+	if got, err := os.ReadFile(filepath.Join(dir, "actions")); err != nil || string(got) != "reload\n" {
+		t.Fatalf("file cleanup reload skipped: %q, %v", got, err)
+	}
+}

scripts/uninstall-sensor-proxy.sh

@@ -138,27 +138,42 @@ disable_legacy_units() {
 remove_managed_keys_from_authorized_keys_file() {
     local file="$1"
     local tmp_file=""
+    local status=0
 
     if [[ ! -f "$file" ]]; then
         return 0
     fi
 
-    tmp_file=$(mktemp)
-    grep -v -E '# pulse-(managed|proxy)-key$' "$file" >"$tmp_file" 2>/dev/null || true
-    if cmp -s "$tmp_file" "$file"; then
-        rm -f "$tmp_file"
+    # Keep replacement on the same filesystem. A failed read, metadata copy or
+    # rename must leave the original keys intact, even under conditional calls.
+    tmp_file=$(mktemp "${file}.pulse-cleanup.XXXXXX") || return 1
+    grep -v -E '# pulse-(managed|proxy)-key$' "$file" >"$tmp_file" 2>/dev/null || status=$?
+    if (( status > 1 )); then
+        rm -f -- "$tmp_file"
+        return 1
+    fi
+    status=0
+    cmp -s "$tmp_file" "$file" || status=$?
+    if (( status == 0 )); then
+        rm -f -- "$tmp_file" || return 1
         return 0
     fi
-
-    chmod --reference="$file" "$tmp_file" 2>/dev/null || chmod 600 "$tmp_file" 2>/dev/null || true
-    chown --reference="$file" "$tmp_file" 2>/dev/null || true
-    mv "$tmp_file" "$file"
+    if (( status > 1 )) ||
+        ! chmod --reference="$file" "$tmp_file" ||
+        ! chown --reference="$file" "$tmp_file" ||
+        ! mv -- "$tmp_file" "$file"; then
+        rm -f -- "$tmp_file"
+        return 1
+    fi
 }
 
 cleanup_local_authorized_keys() {
     local auth_file=""
-    auth_file=$(resolve_path "$AUTHORIZED_KEYS_PATH")
-    remove_managed_keys_from_authorized_keys_file "$auth_file"
+    auth_file=$(resolve_path "$AUTHORIZED_KEYS_PATH") || return 1
+    if ! remove_managed_keys_from_authorized_keys_file "$auth_file"; then
+        print_warn "Local legacy SSH key cleanup failed; original keys were preserved"
+        return 1
+    fi
     print_success "Removed legacy Pulse SSH key entries from ${auth_file}"
 }
 
@@ -183,45 +198,61 @@ is_local_host() {
     [[ "$target" == "127.0.0.1" || "$target" == "localhost" ]]
 }
 
+# The program is passed with -c so stdin remains the complete API response.
+# Validate every row before emitting any identities; a partial list is unsafe.
+sensor_proxy_inventory() {
+    python3 -c '
+import json, re, sys
+field = sys.argv[1]
+try:
+    rows = json.load(sys.stdin)
+    if not isinstance(rows, list) or (field == "node" and not rows):
+        raise ValueError()
+    values = []
+    for row in rows:
+        value = row.get(field) if isinstance(row, dict) else None
+        if not isinstance(value, str) or not re.fullmatch(r"[A-Za-z0-9_][A-Za-z0-9_.-]{0,252}", value):
+            raise ValueError()
+        values.append(value)
+except (ValueError, TypeError):
+    sys.exit(1)
+for value in values:
+    print(value)
+' "$1"
+}
+
 discover_cluster_node_addresses() {
+    local nodes=""
     local node=""
     local node_ip=""
+    local addresses=""
 
-    if command -v pvesh >/dev/null 2>&1 && command -v python3 >/dev/null 2>&1; then
-        while IFS= read -r node; do
-            [[ -z "$node" ]] && continue
-            node_ip=$(getent ahostsv4 "$node" 2>/dev/null | awk 'NR==1 {print $1}') || true
-            if [[ -z "$node_ip" ]]; then
-                node_ip=$(getent hosts "$node" 2>/dev/null | awk 'NR==1 {print $1}') || true
-            fi
-            [[ -n "$node_ip" ]] && printf '%s\n' "$node_ip"
-        done < <(
-            pvesh get /nodes --output-format json 2>/dev/null | python3 - <<'PY'
-import json
-import sys
+    if ! command -v pvesh >/dev/null 2>&1 || ! command -v python3 >/dev/null 2>&1; then
+        print_warn "Cluster inventory requires pvesh and python3; use --local-only once on each node for local cleanup"
+        return 1
+    fi
+    if ! nodes=$(pvesh get /nodes --output-format json 2>/dev/null | sensor_proxy_inventory node); then
+        print_warn "Cluster inventory is unavailable or invalid; no SSH key cleanup attempted"
+        return 1
+    fi
+    while IFS= read -r node; do
+        node_ip=$(getent ahostsv4 "$node" 2>/dev/null | awk 'NR==1 {print $1}') || node_ip=""
+        if [[ -z "$node_ip" ]]; then
+            node_ip=$(getent hosts "$node" 2>/dev/null | awk 'NR==1 {print $1}') || node_ip=""
+        fi
+        if [[ -z "$node_ip" ]] || ! node_ip=$(python3 -c '
+import ipaddress, sys
 try:
-    data = json.load(sys.stdin)
-except Exception:
-    sys.exit(0)
-if isinstance(data, list):
-    for item in data:
-        node = item.get("node")
-        if node:
-            print(node)
-PY
-        )
-        return 0
-    fi
-
-    if command -v pvecm >/dev/null 2>&1; then
-        pvecm status 2>/dev/null | awk '
-            /0x[0-9a-f]+/ {
-                for (i = 1; i <= NF; i++) {
-                    if ($i ~ /^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$/) print $i
-                }
-            }
-        '
-    fi
+    print(ipaddress.ip_address(sys.argv[1]))
+except ValueError:
+    sys.exit(1)
+' "$node_ip"); then
+            print_warn "A cluster node has no valid resolved address; no SSH key cleanup attempted"
+            return 1
+        fi
+        addresses+="${node_ip}"$'\n'
+    done <<<"$nodes"
+    printf '%s' "$addresses"
 }
 
 cleanup_remote_authorized_keys() {
@@ -261,27 +292,27 @@ fi'
 
 cleanup_cluster_authorized_keys() {
     local host=""
-    local saw_nodes=false
+    local hosts=""
     local status=0
 
     if [[ "$LOCAL_ONLY" == "true" ]]; then
         cleanup_local_authorized_keys
-        return 0
+        return $?
     fi
 
+    # Capture the discovery status, not a process substitution whose failure
+    # would otherwise look like an empty, successful single-host installation.
+    if ! hosts=$(discover_cluster_node_addresses) || [[ -z "$hosts" ]]; then
+        return 1
+    fi
+    hosts=$(printf '%s\n' "$hosts" | sort -u) || return 1
+    cleanup_local_authorized_keys || status=1
     while IFS= read -r host; do
         [[ -z "$host" ]] && continue
-        saw_nodes=true
-        if is_local_host "$host"; then
-            cleanup_local_authorized_keys
-        else
+        if ! is_local_host "$host"; then
             cleanup_remote_authorized_keys "$host" || status=1
         fi
-    done < <(discover_cluster_node_addresses | sort -u)
-
-    if [[ "$saw_nodes" != "true" ]]; then
-        cleanup_local_authorized_keys
-    fi
+    done <<<"$hosts"
     return "$status"
 }
 
@@ -469,58 +500,37 @@ remove_legacy_files() {
 remove_proxmox_access() {
     local token_ids=""
     local token_id=""
+    local status=0
 
     if [[ "$REMOVE_PROXMOX_ACCESS" != "true" ]]; then
         return 0
     fi
-    if ! command -v pveum >/dev/null 2>&1; then
-        print_warn "pveum is not available; skipping pulse-monitor@pam cleanup"
-        return 0
+    if ! command -v pveum >/dev/null 2>&1 || ! command -v python3 >/dev/null 2>&1; then
+        print_warn "API access cleanup requires pveum and python3; cleanup is incomplete"
+        return 1
     fi
-
-    if command -v python3 >/dev/null 2>&1; then
-        token_ids=$(pveum user token list pulse-monitor@pam --output-format json 2>/dev/null | python3 - <<'PY'
-import json
-import sys
-try:
-    data = json.load(sys.stdin)
-except Exception:
-    sys.exit(0)
-if isinstance(data, list):
-    for item in data:
-        token_id = item.get("tokenid")
-        if token_id:
-            print(token_id)
-PY
-        ) || true
+    if ! token_ids=$(pveum user token list pulse-monitor@pam --output-format json 2>/dev/null | sensor_proxy_inventory tokenid); then
+        print_warn "API token inventory is unavailable or invalid; API access was left unchanged"
+        return 1
     fi
-
-    if [[ -z "$token_ids" ]]; then
-        token_ids=$(pveum user token list pulse-monitor@pam 2>/dev/null | awk '
-            {
-                line=$0
-                gsub(/\r/, "", line)
-                gsub(/│/, "|", line)
-                if (index(line, "|") == 0) next
-                n = split(line, parts, "|")
-                if (n < 3) next
-                name = parts[2]
-                gsub(/^[[:space:]]+|[[:space:]]+$/, "", name)
-                if (name != "" && name != "tokenid") print name
-            }') || true
-    fi
-
     while IFS= read -r token_id; do
         [[ -z "$token_id" ]] && continue
-        pveum user token remove pulse-monitor@pam "$token_id" >/dev/null 2>&1 || print_warn "Failed to remove pulse-monitor@pam token ${token_id}"
+        if ! pveum user token remove pulse-monitor@pam "$token_id" >/dev/null 2>&1; then
+            print_warn "A legacy API token could not be removed; API user deletion will not be attempted"
+            status=1
+        fi
     done <<<"$token_ids"
-
-    pveum user delete pulse-monitor@pam >/dev/null 2>&1 || print_warn "pulse-monitor@pam was not removed; it may already be absent or still in use"
+    (( status == 0 )) || return "$status"
+    if ! pveum user delete pulse-monitor@pam >/dev/null 2>&1; then
+        print_warn "Legacy API user removal is unconfirmed; cleanup is incomplete"
+        return 1
+    fi
 }
 
 main() {
     local cluster_key_cleanup_status=0
     local container_cleanup_status=0
+    local api_cleanup_status=0
 
     while [[ $# -gt 0 ]]; do
         case "$1" in
@@ -585,15 +595,16 @@ main() {
         return "$container_cleanup_status"
     fi
     remove_legacy_files
-    remove_proxmox_access
+    remove_proxmox_access || api_cleanup_status=$?
     if ! systemctl_if_available daemon-reload; then
         print_warn "Cannot reload service configuration; cleanup is incomplete"
         return 1
     fi
     if (( cluster_key_cleanup_status != 0 )); then
-        print_warn "Local cleanup completed, but one or more remote cluster nodes still require trusted, local cleanup"
+        print_warn "Local file cleanup completed, but cluster SSH key cleanup is incomplete; use --local-only on each node or repair trusted cluster inventory"
         return "$cluster_key_cleanup_status"
     fi
+    (( api_cleanup_status == 0 )) || return "$api_cleanup_status"
     print_success "Legacy pulse-sensor-proxy cleanup complete"
 }