mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 04:33:17 +00:00
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
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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())
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user