From 3ef095fb7160fa4689893f599081a4950f9e9a6f Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sun, 27 Sep 2026 14:00:42 +0200 Subject: [PATCH] agent: a guest outside the agent's ACL is not a known guest (R-689, v0.136.0 regression) PVE answers 403 permission denied, not "does not exist", for a vmid outside the felhom pool; v0.136.0 turned that into a lookup failure and the local tier read UNKNOWN every evaluation (measured on demo-hp). Such an archive is skipped. Red-proofed; verified read-only on demo-hp with the pre-release binary. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- internal/backup/backup_test.go | 4 ++++ internal/backup/r689_golden_test.go | 19 +++++++++++++++++++ internal/backup/runner.go | 8 ++++++-- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index 503f569..0553194 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -23,6 +23,7 @@ type fakeBackupAPI struct { waitErr error cfg proxmox.GuestConfig goneGuests map[int]bool // R-689: vmids whose config lookup answers "does not exist" + aclGuests map[int]bool // R-689: vmids outside the token's ACL — PVE answers 403 "permission denied" cfgErr error content []proxmox.StorageContent contentErr error @@ -46,6 +47,9 @@ func (f *fakeBackupAPI) WaitTask(_ context.Context, _ string, _ proxmox.WaitOpti return proxmox.TaskStatus{Status: "stopped", ExitStatus: "OK"}, f.waitErr } func (f *fakeBackupAPI) GuestConfig(_ context.Context, vmid int) (proxmox.GuestConfig, error) { + if f.aclGuests[vmid] { + return proxmox.GuestConfig{}, fmt.Errorf("proxmox: GET /nodes/n/lxc/%d/config -> HTTP 403: permission denied at /vms/%d (missing privilege VM.Audit)", vmid, vmid) + } if f.goneGuests[vmid] { // R-689: PVE's answer for a deleted guest return proxmox.GuestConfig{}, fmt.Errorf("proxmox: GET /nodes/n/lxc/%d/config -> HTTP 500: Configuration file 'nodes/n/lxc/%d.conf' does not exist", vmid, vmid) } diff --git a/internal/backup/r689_golden_test.go b/internal/backup/r689_golden_test.go index 7a15aa4..233013e 100644 --- a/internal/backup/r689_golden_test.go +++ b/internal/backup/r689_golden_test.go @@ -91,3 +91,22 @@ func TestR689_AGuestLookupFailureIsUnknownNotEmpty(t *testing.T) { t.Fatal("a failed guest lookup read as a clean answer") } } + +// v0.137.0 — THE MEASURED ANSWER: the agent's token sees only its pool, so for the deleted guest PVE says 403 +// "permission denied at /vms/9100", not "does not exist" (demo-hp, right after v0.136.0 — the local tier read +// UNKNOWN). Such a guest is not one this agent manages: its archive is skipped, the tier is not an error. +// +// COMPANION RED-PROOF (REPORT.md): drop the "permission denied" case — the pick errors. +func TestR689_AGuestOutsideTheAgentsACLIsNotAKnownGuest(t *testing.T) { + const day = int64(86400) + now := int64(1790476000) + api := &fakeBackupAPI{aclGuests: map[int]bool{9100: true}, content: []proxmox.StorageContent{ + {VolID: "local:backup/vzdump-lxc-9100-2026_08_21-17_59_15.tar.zst", Content: "backup", VMID: 9100, Size: 656970239, CTime: now - 37*day}, + {VolID: "local:backup/vzdump-lxc-9201-2026_09_27-04_35_47.tar.zst", Content: "backup", VMID: 9201, Size: 8 << 30, CTime: now - 7*3600}, + }} + r := NewBackupRunner(api, "local", proxmox.ModeSnapshot, "", "keep-last=1", quiet()) + got, _, err := r.PickSettledRestoreCandidateOn(context.Background(), "local", time.Unix(now-day, 0).UTC()) + if err != nil || got != "" { + t.Fatalf("picked %q err=%v — want nothing and no error (the only settled archive is not ours)", got, err) + } +} diff --git a/internal/backup/runner.go b/internal/backup/runner.go index 7210487..2d5f4e4 100644 --- a/internal/backup/runner.go +++ b/internal/backup/runner.go @@ -381,14 +381,18 @@ func (r *BackupRunner) PickSettledRestoreCandidateOn(ctx context.Context, target switch { case err == nil: known[e.VMID] = true - case strings.Contains(err.Error(), "does not exist"): + case strings.Contains(err.Error(), "does not exist"), strings.Contains(err.Error(), "permission denied"): + // v0.137.0: PVE answers 403 "permission denied at /vms/" — not "does not exist" — for a guest + // outside the agent's ACL (the `felhom` pool). Measured on demo-hp after v0.136.0: the deleted + // guest 9100's archive made the local tier UNKNOWN every evaluation. A guest the agent cannot + // read is not one it manages; its archive is not a candidate. known[e.VMID] = false default: return "", time.Time{}, fmt.Errorf("checking whether guest %d still exists: %w", e.VMID, err) } } if !known[e.VMID] { - r.noteNotAGuestBackupOnce(e, fmt.Sprintf("guest %d no longer exists on this node", e.VMID)) + r.noteNotAGuestBackupOnce(e, fmt.Sprintf("guest %d does not exist on this node or is not one this agent manages", e.VMID)) continue } if !notAfter.IsZero() && e.CTime > notAfter.Unix() {