diff --git a/docs/AGENT_SECURITY.md b/docs/AGENT_SECURITY.md index 0089c2d94d..a6b28114dc 100644 --- a/docs/AGENT_SECURITY.md +++ b/docs/AGENT_SECURITY.md @@ -62,6 +62,13 @@ where you need data that Proxmox cannot provide through its API, or where the data lives inside a guest/container rather than at the Proxmox node layer. The Settings Proxmox setup flow uses this API inventory path as the default; the host telemetry agent path is for the full-telemetry cases above. +Generated API Inventory setup still needs a one-time privileged shell on the +Proxmox host so it can create the `pulse-monitor` account, token, and ACLs, but +steady-state monitoring uses the Proxmox API rather than a root Pulse agent. +For PVE, the generated script creates a privilege-separated API token and +mirrors the generated read/monitoring ACLs onto both the service user and the +token. For PBS, the generated script grants the `Audit` ACL to both the service +user and token. Running `pulse-agent` as a custom non-root systemd user is possible by editing the service unit, but it is not a supported full-telemetry mode today. Expect diff --git a/docs/UNIFIED_AGENT.md b/docs/UNIFIED_AGENT.md index 71d4be3a24..abf0487471 100644 --- a/docs/UNIFIED_AGENT.md +++ b/docs/UNIFIED_AGENT.md @@ -378,7 +378,10 @@ If local PVE backups aren't appearing in Pulse after setting up via `--enable-pr 1. **Check permissions**: The API token needs `PVEDatastoreAdmin` on `/storage`: ```bash pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin + pveum aclmod /storage -token 'pulse-monitor@pve!' -role PVEDatastoreAdmin ``` + Replace `pulse-monitor@pve!` with the full token ID shown in Pulse. + Privilege-separated PVE tokens need the storage ACL on the token as well as the service user. 2. **Re-run setup**: Delete the node in Pulse Settings and re-run the agent with `--enable-proxmox`. Recent versions grant this permission automatically. diff --git a/docs/UPGRADE_v5.md b/docs/UPGRADE_v5.md index 7695c4102b..2a9273cc99 100644 --- a/docs/UPGRADE_v5.md +++ b/docs/UPGRADE_v5.md @@ -85,7 +85,11 @@ This can happen if: **Quick fix** (run on each Proxmox host): ```bash pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin +pveum aclmod /storage -token 'pulse-monitor@pve!' -role PVEDatastoreAdmin ``` +Replace `pulse-monitor@pve!` with the full token ID shown in Pulse, +for example `pulse-monitor@pve!pulse-example`. Privilege-separated PVE tokens +need the storage ACL on the token as well as the service user. **Alternative** (re-run setup): 1. Delete the node from Pulse Settings diff --git a/docs/release-control/v6/internal/records/agent-lifecycle-proxmox-setup-permission-proof-2026-05-05.md b/docs/release-control/v6/internal/records/agent-lifecycle-proxmox-setup-permission-proof-2026-05-05.md new file mode 100644 index 0000000000..fbc3ccfc85 --- /dev/null +++ b/docs/release-control/v6/internal/records/agent-lifecycle-proxmox-setup-permission-proof-2026-05-05.md @@ -0,0 +1,48 @@ +# Agent Lifecycle Proxmox Setup Permission Proof + +- Date: `2026-05-05` +- Lane: `L16` +- Trigger: Follow-up to Proxmox API-first onboarding and root-agent hardening + +## Context + +The Proxmox API Inventory path should be the default for ordinary PVE/PBS +inventory, but that only holds if the generated setup script creates credentials +that are narrow in steady state. PVE API tokens with privilege separation need +their own ACLs, and those token ACLs remain bounded by the associated user. + +## Outcome + +Generated Proxmox setup scripts now pin the permission model directly: + +- PVE setup creates the `pulse-monitor@pve!pulse-*` token with privilege + separation enabled. +- PVE setup applies `PVEAuditor`, optional `PulseMonitor`, and optional + `/storage` `PVEDatastoreAdmin` grants to both `pulse-monitor@pve` and the + concrete generated token id. +- PBS setup proof now asserts the generated script grants only `Audit` to both + `pulse-monitor@pbs` and the concrete generated token id. +- PVE backup permission warnings and manual repair docs now include the token + `/storage` ACL as well as the service-user ACL, so operators do not apply a + user-only repair that privilege-separated tokens still cannot use. +- Operator-facing security docs and release-control contracts now record that + API Inventory is a one-time privileged setup action followed by API-token + monitoring, not a root Pulse agent deployment. + +## Proof Commands + +Run as part of this slice: + +- `go test ./internal/api -run 'TestPVESetupScript|TestPBSSetupScript|TestHandleSetupScript|TestRenderSetupScript' -count=1` +- `go test ./internal/monitoring -run TestPVEBackupPermissionWarning -count=1` +- `go test ./internal/api -count=1` +- `go test ./internal/monitoring -count=1` +- `python3 scripts/release_control/status_audit.py --pretty` +- `python3 scripts/release_control/contract_audit.py --pretty` +- `python3 scripts/release_control/registry_audit.py --pretty` + +## Follow-Up Guard + +Future generated Proxmox setup changes must keep PVE token ACLs and user ACLs +paired. A user-only ACL is not enough for privilege-separated PVE tokens, and a +shared/unprivileged token is not the canonical API Inventory posture. diff --git a/docs/release-control/v6/internal/status.json b/docs/release-control/v6/internal/status.json index e070cc63e7..421af4a695 100644 --- a/docs/release-control/v6/internal/status.json +++ b/docs/release-control/v6/internal/status.json @@ -3140,6 +3140,16 @@ "path": "internal/monitoring/metrics_history.go", "kind": "file" }, + { + "repo": "pulse", + "path": "internal/monitoring/monitor_backup_poll_test.go", + "kind": "file" + }, + { + "repo": "pulse", + "path": "internal/monitoring/monitor_backups.go", + "kind": "file" + }, { "repo": "pulse", "path": "internal/monitoring/poll_providers.go", @@ -3350,6 +3360,11 @@ "agent-lifecycle" ], "evidence": [ + { + "repo": "pulse", + "path": "docs/AGENT_SECURITY.md", + "kind": "file" + }, { "repo": "pulse", "path": "docs/release-control/v6/internal/records/agent-lifecycle-agent-privilege-docs-2026-05-01.md", @@ -3365,6 +3380,11 @@ "path": "docs/release-control/v6/internal/records/agent-lifecycle-proxmox-api-first-onboarding-2026-05-05.md", "kind": "file" }, + { + "repo": "pulse", + "path": "docs/release-control/v6/internal/records/agent-lifecycle-proxmox-setup-permission-proof-2026-05-05.md", + "kind": "file" + }, { "repo": "pulse", "path": "docs/release-control/v6/internal/records/agent-lifecycle-root-agent-hardening-2026-05-05.md", @@ -3380,6 +3400,16 @@ "path": "docs/release-control/v6/internal/subsystems/agent-lifecycle.md", "kind": "file" }, + { + "repo": "pulse", + "path": "docs/UNIFIED_AGENT.md", + "kind": "file" + }, + { + "repo": "pulse", + "path": "docs/UPGRADE_v5.md", + "kind": "file" + }, { "repo": "pulse", "path": "frontend-modern/src/api/agentProfiles.ts", @@ -3440,11 +3470,31 @@ "path": "internal/agentupdate/update.go", "kind": "file" }, + { + "repo": "pulse", + "path": "internal/api/agent_install_command_shared_test.go", + "kind": "file" + }, + { + "repo": "pulse", + "path": "internal/api/config_handlers_setup_script_test.go", + "kind": "file" + }, { "repo": "pulse", "path": "internal/api/config_setup_handlers.go", "kind": "file" }, + { + "repo": "pulse", + "path": "internal/api/contract_test.go", + "kind": "file" + }, + { + "repo": "pulse", + "path": "internal/api/setup_script_render.go", + "kind": "file" + }, { "repo": "pulse", "path": "internal/api/unified_agent.go", diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 441a9ddb72..1c89af7bb3 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -777,6 +777,11 @@ Inventory path: the UI may recommend a root Pulse Agent only as the Host Telemetry Agent path for temperatures, SMART, local storage detail, agent-driven operations, or other node-local telemetry the Proxmox API cannot provide. +Generated API Inventory scripts remain a one-time privileged setup action, but +their steady-state credential must be a narrowly scoped Proxmox API token: PVE +setup must use a privilege-separated token and mirror generated ACLs onto both +the service user and token, while PBS setup must keep `Audit` grants on both +the service user and token. Generated TrueNAS CORE rc.d service scripts must give `/usr/sbin/daemon -r` a supervisor pidfile with `-P`, keep the child pid in a separate diagnostic diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index db8aca2211..cbf79ec59d 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -183,6 +183,14 @@ product API routes free of maintainer commercial analytics. That same canonical payload contract also owns strict-TLS truth for that selected host: `/api/auto-register` may only persist `VerifySSL=true` when Pulse actually captured a certificate fingerprint for the selected candidate, and it must not pretend public-CA verification is safe after every candidate fingerprint probe failed. That same contract now owns stale-marker verification as well: setup-token-authenticated `checkRegistration` requests may omit token completion fields and must answer `{registered:boolean}` from canonical candidate-host matching so runtime repair can distinguish real registrations from stale local marker files without rotating tokens first. That same shared setup contract also owns teardown symmetry for script-managed Proxmox nodes: `/api/auto-unregister` must accept the canonical `type`, normalized `host`, explicit `serverName`, optional canonical `tokenId`, request-body `authToken`, and `source:"script"` payload, and it must answer the same canonical success envelope on both real removals and idempotent no-op reruns so browser/runtime callers do not invent a second uninstall vocabulary. + That same setup-script payload contract owns the generated Proxmox + permission shape. PVE scripts must create `pulse-monitor@pve!pulse-*` + tokens with privilege separation enabled, then apply the generated + `PVEAuditor`, optional `PulseMonitor`, and optional `/storage` + `PVEDatastoreAdmin` ACLs to both `pulse-monitor@pve` and the concrete + token id. PBS scripts must grant `Audit` to both `pulse-monitor@pbs` and + the concrete token id. Browser/runtime setup callers must not fork this + into token-shared, token-unprivileged, or user-only ACL variants. 37. `internal/api/enterprise_extension_rbac_admin.go` shared with `organization-settings`: RBAC admin extension endpoints are both an organization settings control surface and a canonical API payload contract boundary. 38. `internal/api/licensing_bridge.go` shared with `cloud-paid`: commercial licensing bridge handlers carry both API payload contract and cloud-paid entitlement boundary ownership. 39. `internal/api/licensing_handlers.go` shared with `cloud-paid`: commercial licensing handlers carry both API payload contract and cloud-paid entitlement boundary ownership. diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index b578e73e8b..557cbe14ac 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -879,6 +879,11 @@ instance and guest type, including template VMIDs that are intentionally excluded from normal workload resources. Backup alert evaluation may then receive that scoped signal from monitoring, but alert code must not infer PVE orphan readiness from recovery rollups alone. +That same Proxmox backup boundary also owns permission-repair guidance for PVE +backup visibility failures. When storage content reads fail with authorization +errors, the monitoring warning must tell operators to grant `/storage` +`PVEDatastoreAdmin` to both the `pulse-monitor@pve` service user and the +configured privilege-separated token when that token id is known. That same monitoring-owned host-agent ingest boundary now also owns vendor-managed NAS RAID normalization. `internal/monitoring/monitor_agents.go` must filter vendor-managed system arrays through the shared diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 029ef2ff78..418b9a6112 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -66,6 +66,12 @@ state. ## Extension Points +Generated Proxmox setup-script changes in `internal/api/` that affect backup +visibility permissions are storage/recovery-adjacent: optional PVE `/storage` +grants must remain effective for privilege-separated tokens by assigning the +same `PVEDatastoreAdmin` role to both the service user and the concrete token +id. + Storage/recovery auth-adjacent changes may consume SSO-authenticated sessions, but they must not reinterpret SAML or multi-provider SSO availability as a storage/recovery entitlement; that provider-route and license truth belongs to @@ -2353,6 +2359,10 @@ names stay bound to the canonical Pulse endpoint across setup/bootstrap surfaces, so adjacent setup and recovery-linked flows may not derive token scope from request-local `Host` fallbacks and accidentally fork monitor-token identity for the same Pulse instance. +That same shared dependency also assumes generated PVE setup scripts keep +backup visibility permissions effective on privilege-separated tokens: optional +`/storage` `PVEDatastoreAdmin` grants must be mirrored to the service user and +the concrete token id, not left as a user-only ACL that the token cannot use. That same shared dependency also assumes `/api/setup-script` stays on one canonical artifact contract: manual setup downloads must ship as `text/x-shellscript` attachments with deterministic `pulse-setup-*.sh` diff --git a/internal/api/agent_install_command_shared_test.go b/internal/api/agent_install_command_shared_test.go index 1554a41081..9b820c5d0d 100644 --- a/internal/api/agent_install_command_shared_test.go +++ b/internal/api/agent_install_command_shared_test.go @@ -207,7 +207,11 @@ func TestRenderSetupScript_UsesSharedInstallArtifactShape(t *testing.T) { SetupToken: "setup-token-123", TokenName: buildPulseMonitorTokenName("https://pulse.example"), TokenMatchPrefix: buildPulseMonitorTokenName("https://pulse.example"), - StoragePerms: "\npveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin", + StoragePerms: ` +pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin +if [ "$TOKEN_CREATED" = true ]; then + pveum aclmod /storage -token "$PULSE_TOKEN_ID" -role PVEDatastoreAdmin +fi`, SensorsPublicKey: "ssh-ed25519 AAAATEST pulse@test", Artifact: artifact, }) @@ -216,6 +220,7 @@ func TestRenderSetupScript_UsesSharedInstallArtifactShape(t *testing.T) { require.Contains(t, script, `PULSE_BOOTSTRAP_COMMAND_WITH_ENV='curl -fsSL '"'"'https://pulse.example/api/setup-script?backup_perms=true&host=https%3A%2F%2Fpve1.local%3A8006&pulse_url=https%3A%2F%2Fpulse.example&type=pve'"'"' | `) require.Contains(t, script, `PULSE_SETUP_TOKEN="${PULSE_SETUP_TOKEN:-setup-token-123}"`) require.Contains(t, script, `pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin`) + require.Contains(t, script, `pveum aclmod /storage -token "$PULSE_TOKEN_ID" -role PVEDatastoreAdmin`) } func TestInstallBaseURLRequiresInsecure(t *testing.T) { diff --git a/internal/api/config_handlers_setup_script_test.go b/internal/api/config_handlers_setup_script_test.go index cc38a1ba03..97d56d7542 100644 --- a/internal/api/config_handlers_setup_script_test.go +++ b/internal/api/config_handlers_setup_script_test.go @@ -292,16 +292,24 @@ func TestPVESetupScript_ConfiguresPulseMonitorRoleSafely(t *testing.T) { script := rr.Body.String() - // Privileges must be comma-separated for pveum, and role updates should be non-destructive. + // Privileges must be comma-separated for pveum, and generated tokens must stay privilege-separated. wantSnippets := []string{ + `TOKEN_OUTPUT=$(pveum user token add pulse-monitor@pve "$TOKEN_NAME" --privsep 1 2>&1)`, + `pveum aclmod / -user pulse-monitor@pve -role PVEAuditor`, + `pveum aclmod / -token "$PULSE_TOKEN_ID" -role PVEAuditor`, `PRIV_STRING="$(IFS=,; echo "${EXTRA_PRIVS[*]}")"`, `pveum role modify PulseMonitor -privs "$PRIV_STRING" 2>/dev/null || pveum role add PulseMonitor -privs "$PRIV_STRING" 2>/dev/null`, + `pveum aclmod / -user pulse-monitor@pve -role PulseMonitor`, + `pveum aclmod / -token "$PULSE_TOKEN_ID" -role PulseMonitor`, } for _, snippet := range wantSnippets { if !containsString(script, snippet) { t.Fatalf("expected generated script to contain:\n%s\n\nGot (first 500 chars):\n%s", snippet, truncate(script, 500)) } } + if containsString(script, `--privsep 0`) { + t.Fatalf("expected generated PVE token to remain privilege-separated, got: %s", truncate(script, 900)) + } } func TestPVESetupScript_UsesFailFastRetryGuidance(t *testing.T) { @@ -383,6 +391,9 @@ func TestPVESetupScript_PreservesEncodedRerunURLAndBackupPerms(t *testing.T) { if !containsString(script, `pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin`) { t.Fatalf("expected backup permission role setup to remain enabled, got: %s", truncate(script, 900)) } + if !containsString(script, `pveum aclmod /storage -token "$PULSE_TOKEN_ID" -role PVEDatastoreAdmin`) { + t.Fatalf("expected backup permission role setup to apply to the privilege-separated token, got: %s", truncate(script, 900)) + } } func TestPVESetupScript_RemovesDiscoveredOldTokensFromBothUsers(t *testing.T) { @@ -1010,6 +1021,26 @@ func TestHandleSetupScript_PBSTypeGeneratesScript(t *testing.T) { }) } + wantACLs := []string{ + `proxmox-backup-manager acl update / Audit --auth-id pulse-monitor@pbs`, + `proxmox-backup-manager acl update / Audit --auth-id "$PULSE_TOKEN_ID"`, + } + for _, snippet := range wantACLs { + if !containsString(script, snippet) { + t.Fatalf("expected generated PBS script to contain exact Audit ACL:\n%s\n\nGot (first 900 chars):\n%s", snippet, truncate(script, 900)) + } + } + for _, forbidden := range []string{ + `Admin --auth-id pulse-monitor@pbs`, + `Admin --auth-id "$PULSE_TOKEN_ID"`, + `DatastoreAdmin`, + `PVEDatastoreAdmin`, + } { + if containsString(script, forbidden) { + t.Fatalf("expected generated PBS setup to avoid admin ACL %q, got: %s", forbidden, truncate(script, 900)) + } + } + // Verify PBS script does NOT contain PVE-specific content if containsString(script, "pveum user add") { t.Error("PBS script should not contain PVE commands like 'pveum user add'") diff --git a/internal/api/config_setup_handlers.go b/internal/api/config_setup_handlers.go index 68c677f8cf..aaa3e49cb4 100644 --- a/internal/api/config_setup_handlers.go +++ b/internal/api/config_setup_handlers.go @@ -136,7 +136,11 @@ func (h *ConfigHandlers) handleSetupScript(w http.ResponseWriter, r *http.Reques storagePerms := "" if backupPerms { - storagePerms = "\npveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin" + storagePerms = ` +pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin +if [ "$TOKEN_CREATED" = true ]; then + pveum aclmod /storage -token "$PULSE_TOKEN_ID" -role PVEDatastoreAdmin +fi` } script := renderSetupScript(serverType, setupScriptRenderContext{ diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index c413efdca2..7e1bf6a187 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -72,6 +72,87 @@ func TestPatrolRemediationCommercialCopyUsesSafeRemediationWording(t *testing.T) } } +func TestContract_ProxmoxSetupScriptUsesPrivilegeSeparatedTokenACLs(t *testing.T) { + storagePerms := ` +pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin +if [ "$TOKEN_CREATED" = true ]; then + pveum aclmod /storage -token "$PULSE_TOKEN_ID" -role PVEDatastoreAdmin +fi` + pveArtifact := buildSetupScriptInstallArtifact( + "https://pulse.example", + "pve", + "https://pve.example:8006", + "https://pulse.example", + true, + "setup-token-123", + 0, + ) + pveScript := renderSetupScript("pve", setupScriptRenderContext{ + ServerName: "pve-example", + PulseURL: "https://pulse.example", + ServerHost: "https://pve.example:8006", + SetupToken: "setup-token-123", + TokenName: "pulse-example", + TokenMatchPrefix: "pulse-example", + StoragePerms: storagePerms, + SensorsPublicKey: "ssh-ed25519 AAAATEST pulse@test", + Artifact: pveArtifact, + }) + + for _, required := range []string{ + `TOKEN_OUTPUT=$(pveum user token add pulse-monitor@pve "$TOKEN_NAME" --privsep 1 2>&1)`, + `pveum aclmod / -user pulse-monitor@pve -role PVEAuditor`, + `pveum aclmod / -token "$PULSE_TOKEN_ID" -role PVEAuditor`, + `pveum aclmod / -user pulse-monitor@pve -role PulseMonitor`, + `pveum aclmod / -token "$PULSE_TOKEN_ID" -role PulseMonitor`, + `pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin`, + `pveum aclmod /storage -token "$PULSE_TOKEN_ID" -role PVEDatastoreAdmin`, + } { + if !strings.Contains(pveScript, required) { + t.Fatalf("PVE setup script must contain %q", required) + } + } + if strings.Contains(pveScript, "--privsep 0") { + t.Fatal("PVE setup script must not create full-privilege API tokens") + } + + pbsArtifact := buildSetupScriptInstallArtifact( + "https://pulse.example", + "pbs", + "https://pbs.example:8007", + "https://pulse.example", + false, + "setup-token-123", + 0, + ) + pbsScript := renderSetupScript("pbs", setupScriptRenderContext{ + ServerName: "pbs-example", + PulseURL: "https://pulse.example", + ServerHost: "https://pbs.example:8007", + SetupToken: "setup-token-123", + TokenName: "pulse-example", + TokenMatchPrefix: "pulse-example", + Artifact: pbsArtifact, + }) + + for _, required := range []string{ + `proxmox-backup-manager acl update / Audit --auth-id pulse-monitor@pbs`, + `proxmox-backup-manager acl update / Audit --auth-id "$PULSE_TOKEN_ID"`, + } { + if !strings.Contains(pbsScript, required) { + t.Fatalf("PBS setup script must contain %q", required) + } + } + for _, forbidden := range []string{ + `Admin --auth-id pulse-monitor@pbs`, + `Admin --auth-id "$PULSE_TOKEN_ID"`, + } { + if strings.Contains(pbsScript, forbidden) { + t.Fatalf("PBS setup script must not grant admin ACL %q", forbidden) + } + } +} + func TestContract_HostedMagicLinkStablePrincipalProof(t *testing.T) { source, err := os.ReadFile(filepath.Clean("magic_link_handlers.go")) if err != nil { diff --git a/internal/api/setup_script_render.go b/internal/api/setup_script_render.go index 1db23957bb..0ca494efd9 100644 --- a/internal/api/setup_script_render.go +++ b/internal/api/setup_script_render.go @@ -479,8 +479,8 @@ if pveum user token list pulse-monitor@pve 2>/dev/null | awk 'NR>3 {print $2}' | fi fi -# Create token and capture value (shown once by Proxmox) -TOKEN_OUTPUT=$(pveum user token add pulse-monitor@pve "$TOKEN_NAME" --privsep 0 2>&1) +# Create a privilege-separated token and capture value (shown once by Proxmox) +TOKEN_OUTPUT=$(pveum user token add pulse-monitor@pve "$TOKEN_NAME" --privsep 1 2>&1) TOKEN_CREATE_RC=$? if [ "$TOKEN_CREATE_RC" -ne 0 ]; then echo "❌ Failed to create token '$TOKEN_NAME'" @@ -517,7 +517,10 @@ fi # Set up permissions echo "Setting up permissions..." -pveum aclmod / -user pulse-monitor@pve -role PVEAuditor%s +pveum aclmod / -user pulse-monitor@pve -role PVEAuditor +if [ "$TOKEN_CREATED" = true ]; then + pveum aclmod / -token "$PULSE_TOKEN_ID" -role PVEAuditor +fi%s # Detect Proxmox version and apply appropriate permissions # Method 1: Try to check if VM.Monitor exists (reliable for PVE 8 and below) @@ -578,6 +581,9 @@ if [ ${#EXTRA_PRIVS[@]} -gt 0 ]; then # Prefer modify (non-destructive) in case PulseMonitor already exists. if pveum role modify PulseMonitor -privs "$PRIV_STRING" 2>/dev/null || pveum role add PulseMonitor -privs "$PRIV_STRING" 2>/dev/null; then pveum aclmod / -user pulse-monitor@pve -role PulseMonitor + if [ "$TOKEN_CREATED" = true ]; then + pveum aclmod / -token "$PULSE_TOKEN_ID" -role PulseMonitor + fi echo " • Applied privileges: $PRIV_STRING" else echo " • Failed to configure PulseMonitor role with: $PRIV_STRING" diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index 0e364dbb17..492ce01564 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -73,6 +73,21 @@ func TestNoGetStateResourceArrayRegression(t *testing.T) { } } +func TestPVEBackupPermissionWarningsPreserveTokenACLRepair(t *testing.T) { + warning := pveBackupPermissionWarning(&config.PVEInstance{ + TokenName: "pulse-monitor@pve!pulse-example", + }) + + for _, snippet := range []string{ + "pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin", + "pveum aclmod /storage -token 'pulse-monitor@pve!pulse-example' -role PVEDatastoreAdmin", + } { + if !strings.Contains(warning, snippet) { + t.Fatalf("expected warning to contain %q, got %q", snippet, warning) + } + } +} + func TestGetStateRefreshesLiveAlertSnapshots(t *testing.T) { data, err := os.ReadFile("monitor.go") if err != nil { diff --git a/internal/monitoring/monitor_backup_poll_test.go b/internal/monitoring/monitor_backup_poll_test.go index 60c52a579a..3728549ebb 100644 --- a/internal/monitoring/monitor_backup_poll_test.go +++ b/internal/monitoring/monitor_backup_poll_test.go @@ -8,6 +8,32 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/config" ) +func TestPVEBackupPermissionWarningIncludesTokenACLWhenAvailable(t *testing.T) { + warning := pveBackupPermissionWarning(&config.PVEInstance{ + TokenName: "pulse-monitor@pve!pulse-example", + }) + + for _, snippet := range []string{ + "pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin", + "pveum aclmod /storage -token 'pulse-monitor@pve!pulse-example' -role PVEDatastoreAdmin", + } { + if !strings.Contains(warning, snippet) { + t.Fatalf("expected warning to contain %q, got %q", snippet, warning) + } + } +} + +func TestPVEBackupPermissionWarningFallsBackForManualTokens(t *testing.T) { + warning := pveBackupPermissionWarning(&config.PVEInstance{}) + + if !strings.Contains(warning, "pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin") { + t.Fatalf("expected user ACL guidance, got %q", warning) + } + if !strings.Contains(warning, "if using a privilege-separated API token") { + t.Fatalf("expected privilege-separated token fallback guidance, got %q", warning) + } +} + func TestShouldRunBackupPoll(t *testing.T) { now := time.Now() last := now.Add(-5 * time.Minute) diff --git a/internal/monitoring/monitor_backups.go b/internal/monitoring/monitor_backups.go index 4649e53374..7484dac5ce 100644 --- a/internal/monitoring/monitor_backups.go +++ b/internal/monitoring/monitor_backups.go @@ -87,6 +87,22 @@ func (m *Monitor) updatePVEBackupTemplateSubjectsFromClusterResources(instanceNa m.updatePVEBackupTemplateSubjectsForType(instanceName, "lxc", lxcTemplates) } +func quotePVEACLTokenID(tokenID string) string { + return "'" + strings.ReplaceAll(tokenID, "'", `'"'"'`) + "'" +} + +func pveBackupPermissionWarning(instanceCfg *config.PVEInstance) string { + warning := "Missing PVEDatastoreAdmin permission on /storage. Run: pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin" + if instanceCfg == nil { + return warning + "; if using a privilege-separated API token, also grant PVEDatastoreAdmin on /storage to that token." + } + tokenID := strings.TrimSpace(instanceCfg.TokenName) + if tokenID == "" || !strings.Contains(tokenID, "!") { + return warning + "; if using a privilege-separated API token, also grant PVEDatastoreAdmin on /storage to that token." + } + return warning + " && pveum aclmod /storage -token " + quotePVEACLTokenID(tokenID) + " -role PVEDatastoreAdmin" +} + func (m *Monitor) backupInventoryScopeForAlerts() *alerts.BackupInventoryScope { if m == nil { return nil @@ -204,8 +220,9 @@ func (m *Monitor) pollStorageBackupsWithNodes(ctx context.Context, instanceName if strings.Contains(errStr, "403") || strings.Contains(errStr, "401") || strings.Contains(errStr, "permission") || strings.Contains(errStr, "forbidden") { hadPermissionError = true + warning := pveBackupPermissionWarning(m.getInstanceConfig(instanceName)) m.mu.Lock() - m.backupPermissionWarnings[instanceName] = "Missing PVEDatastoreAdmin permission on /storage. Run: pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin" + m.backupPermissionWarnings[instanceName] = warning m.mu.Unlock() log.Warn(). Str("instance", instanceName).