From a090a6775613903e2d7dc05838fb9cbfaa67fa27 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sat, 3 Oct 2026 14:21:03 +0100 Subject: [PATCH] Keep guest-agent admission single-attempt for password sessions The generic password-session client retries once after a 401; that implicit retry can bypass a fresh lock check inside a guest admission. Disable it only for QGA commands, retain the refusal, and prove healthy guest reads plus unchanged ordinary API session recovery. Contract-Neutral: Completes the documented no-replay guest admission rule without adding a contract or surface; adds password refusal and healthy recovery controls. Change-source: pulse-maintainer --- pkg/proxmox/guest_agent_guard.go | 5 ++- pkg/proxmox/guest_agent_guard_test.go | 52 +++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/pkg/proxmox/guest_agent_guard.go b/pkg/proxmox/guest_agent_guard.go index d273d7ae02..1f8df97621 100644 --- a/pkg/proxmox/guest_agent_guard.go +++ b/pkg/proxmox/guest_agent_guard.go @@ -171,7 +171,10 @@ func (c *Client) getGuestAgent(ctx context.Context, path, node string, vmid int) if err := c.verifyGuestAgentUnlocked(ctx, node, vmid); err != nil { return nil, err } - resp, err := c.request(ctx, http.MethodGet, path, nil) + // A password-session 401 must not replay this command after re-authentication: + // the operation lock could change between attempts. Ordinary API reads still + // own their usual session recovery; this guest admission is single-attempt. + resp, err := c.requestWithRetry(ctx, http.MethodGet, path, nil, true) if err != nil { // Explicit refusals/unsupported commands remain ordinary errors. An // uncertain completion blocks every command, not just this method. diff --git a/pkg/proxmox/guest_agent_guard_test.go b/pkg/proxmox/guest_agent_guard_test.go index bf27f30353..62d77db187 100644 --- a/pkg/proxmox/guest_agent_guard_test.go +++ b/pkg/proxmox/guest_agent_guard_test.go @@ -189,3 +189,55 @@ func TestGuestAgentCooldownExpiresWithoutSleepOrReplay(t *testing.T) { t.Fatal("healthy completed operation retained estate state") } } + +func TestGuestAgentPasswordSessionDoesNotReplayRefusedCommand(t *testing.T) { + for _, refused := range []bool{false, true} { + t.Run(fmt.Sprintf("refused-%t", refused), func(t *testing.T) { + var authCalls, agentCalls, nodeCalls atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/access/ticket"): + authCalls.Add(1) + fmt.Fprint(w, `{"data":{"ticket":"fixture-ticket","CSRFPreventionToken":"fixture-csrf"}}`) + case strings.HasSuffix(r.URL.Path, "/config"): + fmt.Fprint(w, `{"data":{}}`) + case strings.Contains(r.URL.Path, "/agent/"): + if agentCalls.Add(1) == 1 && refused { + http.Error(w, "session refused", http.StatusUnauthorized) + return + } + backupAgentPayload(w, r) + case strings.HasSuffix(r.URL.Path, "/nodes"): + if nodeCalls.Add(1) == 1 { + http.Error(w, "expired ordinary API session", http.StatusUnauthorized) + return + } + fmt.Fprint(w, `{"data":[]}`) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + client, err := NewClient(ClientConfig{Host: server.URL, User: "fixture@pam", Password: "fixture-password", Timeout: time.Second}) + if err != nil { + t.Fatal(err) + } + _, err = client.GetVMFSInfo(context.Background(), "node", 105) + if refused && (err == nil || !strings.Contains(err.Error(), "401")) { + t.Fatalf("guest refusal lost: %v", err) + } + if !refused && err != nil { + t.Fatal(err) + } + if agentCalls.Load() != 1 || authCalls.Load() != 1 { + t.Fatalf("guest replay: commands=%d authentications=%d", agentCalls.Load(), authCalls.Load()) + } + if _, err := client.GetNodes(context.Background()); err != nil { + t.Fatalf("ordinary API session recovery broken: %v", err) + } + if agentCalls.Load() != 1 || nodeCalls.Load() != 2 || authCalls.Load() != 2 { + t.Fatalf("ordinary recovery replayed guest or did not recover: guest=%d nodes=%d auth=%d", agentCalls.Load(), nodeCalls.Load(), authCalls.Load()) + } + }) + } +}