From 3c57c2d20728f9e711ae2930e282b853eb218730 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:10:01 +0100 Subject: [PATCH] Defer guest commands after incomplete HTTP error responses The HTTP error wrapper hid truncated response bodies from guest admission. Preserve the bounded read failure as a private wrapping type so error statuses also cool down the whole guest. Cover both 200 and 500 truncation, and retain complete permission refusals plus subsequent healthy reads. Contract-Neutral: Completes the existing full-response/incomplete-body guest safety contract; error text and ordinary API errors retain compatibility, with no new interface or surface. Change-source: pulse-maintainer --- pkg/proxmox/client.go | 11 ++++++-- pkg/proxmox/guest_agent_guard.go | 5 ++++ pkg/proxmox/guest_agent_guard_test.go | 38 ++++++++++++++++++++++++++- 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/pkg/proxmox/client.go b/pkg/proxmox/client.go index 1b3dcee276..dec68ba461 100644 --- a/pkg/proxmox/client.go +++ b/pkg/proxmox/client.go @@ -52,13 +52,20 @@ func classifyAPIErrorLog(status int, path string) (apiErrorLogLevel, string) { return apiErrorLogNone, "" } +// Preserve an incomplete body through the HTTP error-response wrapper too. +// An error status alone is not a completed refusal if its body was truncated. +type responseBodyReadError struct{ cause error } + +func (e *responseBodyReadError) Error() string { return e.cause.Error() } +func (e *responseBodyReadError) Unwrap() error { return e.cause } + func readResponseBodyLimited(r io.Reader) ([]byte, error) { body, err := io.ReadAll(io.LimitReader(r, maxResponseBodyBytes+1)) if err != nil { - return nil, err + return nil, &responseBodyReadError{cause: err} } if int64(len(body)) > maxResponseBodyBytes { - return nil, fmt.Errorf("response body exceeds %d bytes", maxResponseBodyBytes) + return nil, &responseBodyReadError{cause: fmt.Errorf("response body exceeds %d bytes", maxResponseBodyBytes)} } return body, nil } diff --git a/pkg/proxmox/guest_agent_guard.go b/pkg/proxmox/guest_agent_guard.go index 1f8df97621..fce6be56b2 100644 --- a/pkg/proxmox/guest_agent_guard.go +++ b/pkg/proxmox/guest_agent_guard.go @@ -176,6 +176,11 @@ func (c *Client) getGuestAgent(ctx context.Context, path, node string, vmid int) // own their usual session recovery; this guest admission is single-attempt. resp, err := c.requestWithRetry(ctx, http.MethodGet, path, nil, true) if err != nil { + var incomplete *responseBodyReadError + if errors.As(err, &incomplete) { + uncertain = true + return nil, &guestAgentDeferredError{reason: "agent-response-incomplete", cause: err} + } // Explicit refusals/unsupported commands remain ordinary errors. An // uncertain completion blocks every command, not just this method. lower := strings.ToLower(err.Error()) diff --git a/pkg/proxmox/guest_agent_guard_test.go b/pkg/proxmox/guest_agent_guard_test.go index 62d77db187..7fcdc823d4 100644 --- a/pkg/proxmox/guest_agent_guard_test.go +++ b/pkg/proxmox/guest_agent_guard_test.go @@ -101,6 +101,14 @@ func TestGuestAgentLockAppearingInFlightDiscardsPayload(t *testing.T) { } func TestGuestAgentIncompleteBodyBlocksWholeVM(t *testing.T) { + for _, status := range []int{http.StatusOK, http.StatusInternalServerError} { + t.Run(fmt.Sprintf("status-%d", status), func(t *testing.T) { + testGuestAgentIncompleteBodyBlocksWholeVM(t, status) + }) + } +} + +func testGuestAgentIncompleteBodyBlocksWholeVM(t *testing.T, status int) { var calls atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.HasSuffix(r.URL.Path, "/config") { @@ -109,12 +117,13 @@ func TestGuestAgentIncompleteBodyBlocksWholeVM(t *testing.T) { } calls.Add(1) w.Header().Set("Content-Length", "1000") + w.WriteHeader(status) fmt.Fprint(w, `{"data":{"result":[]}}`) })) defer server.Close() c := backupTestClient(t, server.URL) if _, err := c.GetVMFSInfo(context.Background(), "node", 105); GuestAgentDeferredReason(err) != "agent-response-incomplete" { - t.Fatalf("incomplete body not deferred: %v", err) + t.Errorf("incomplete body not deferred: %v", err) } for name, read := range backupAgentReads() { t.Run(name, func(t *testing.T) { @@ -128,6 +137,33 @@ func TestGuestAgentIncompleteBodyBlocksWholeVM(t *testing.T) { } } +func TestGuestAgentDeferralDoesNotHideCompletedRefusal(t *testing.T) { + var calls atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/config") { + fmt.Fprint(w, `{"data":{}}`) + return + } + calls.Add(1) + if strings.HasSuffix(r.URL.Path, "/get-fsinfo") { + http.Error(w, "permission denied", http.StatusForbidden) + return + } + backupAgentPayload(w, r) + })) + defer server.Close() + c := backupTestClient(t, server.URL) + if _, err := c.GetVMFSInfo(context.Background(), "node", 105); err == nil || errors.Is(err, ErrGuestAgentDeferred) || !strings.Contains(err.Error(), "403") { + t.Fatalf("completed refusal hidden or lost: %v", err) + } + if _, err := c.GetVMAgentInfo(context.Background(), "node", 105); err != nil { + t.Fatalf("completed refusal poisoned other healthy reads: %v", err) + } + if calls.Load() != 2 { + t.Fatalf("refused/healthy command count = %d", calls.Load()) + } +} + func TestGuestAgentClusterDoesNotReplayUncertainCall(t *testing.T) { var calls atomic.Int32 handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {