From b6e56c5baa2ad995264faa80877543300bc10273 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Sun, 18 Oct 2026 18:11:01 +0100 Subject: [PATCH] Preserve PBS authentication cancellation while reading responses A cancelled login can consume an empty response before the transport reports cancellation. Preserve that context outcome for the owner and joined readers, reject late session publication and prevent a cancelled refusal from retrying another encoding. Keep live response validation and later recovery intact. --- ...ase-delivery-and-observation-2026-10-18.md | 4 +- .../v6/internal/subsystems/monitoring.md | 14 ++++ pkg/pbs/client.go | 20 +++-- pkg/pbs/client_construction_context_test.go | 73 +++++++++++++++++++ pkg/pbs/host_diagnostics_regression_test.go | 4 + pkg/pbs/session_response_admission_test.go | 2 +- 6 files changed, 109 insertions(+), 8 deletions(-) diff --git a/docs/release-control/v6/internal/records/maintenance-release-delivery-and-observation-2026-10-18.md b/docs/release-control/v6/internal/records/maintenance-release-delivery-and-observation-2026-10-18.md index bebc671790..7bce722bc9 100644 --- a/docs/release-control/v6/internal/records/maintenance-release-delivery-and-observation-2026-10-18.md +++ b/docs/release-control/v6/internal/records/maintenance-release-delivery-and-observation-2026-10-18.md @@ -39,7 +39,9 @@ The integrated source's [backend partition 0](https://github.com/rcourtman/Pulse At `6bf9e818e8c261012902da79ac2fc2e4a5b57acc`, all nine protected required checks passed, including both backend partitions, and five native platforms passed. The [Intel job](https://github.com/rcourtman/Pulse/actions/runs/39138980077/job/117709536519) passed the NAS preflight but exhausted the general-1 ten-minute Go process alarm while its active log-sink recovery control had run only 17 seconds. The remaining source blocker is aggregate native inventory execution. The canonical Unix runner now compiles and discovers one exact binary per phase, then executes every root check independently under the same ten-minute Go bound. Individual fixture and operation deadlines, full inventory acceptance and overall CI job deadlines remain unchanged. Exact-source native qualification is still required before landing. The required [script-smoke shard at 45a82ea03c](https://github.com/rcourtman/Pulse/actions/runs/39145479921/job/117728549560) exposed an existing script test entrypoint that still asserted the old grouped runner. The compiled-Go execution controls now live in that canonical script suite, shared by script-smoke discovery and the native Go wrapper. All eight controls passed through both worker entrypoints; fresh required and native checks must pass after this integration correction. -The [advisory benchmark comparison](https://github.com/rcourtman/Pulse/actions/runs/39138980114/job/117709718583) completed execution but reported three regressions above 10% in test-only legacy history-filter references. Their corresponding production variants were unchanged or below 4%. Both filter implementation and benchmark source are byte-identical between baseline `84a96940185cc202aef0451d658bc89b864710a0` and synthetic candidate `ed25833fa8318e493d560239f60e56ec5ffad540`. The retained paired samples and source identities require an owning disposition; unchanged source alone does not classify the timing observation. This advisory result does not replace the protected required checks or existing release gates. +The [backend partition 0 at 45a82ea03c](https://github.com/rcourtman/Pulse/actions/runs/39145479921/job/117729442632) exposed a PBS response-consumption race: construction stopped but sometimes classified an ended password exchange as a malformed session. A deterministic control at `266144de1fc00e699f0ed07c8df76fd6a00cacc4` reproduced all ten JSON/form cases, including late complete-session publication and a cancelled encoding refusal authorizing another attempt. The shared authentication boundary now preserves the owner's context error for joined readers, blocks fallback after cancellation and checks caller lifetime before publishing a session. Live response validation and independent later recovery remain. All 108 PBS root tests and five affected monitoring controls passed under the race detector with no failures or skips, including all ten deterministic response-cancellation cases. Fresh final-source required and native CI still govern source qualification. + +The [advisory benchmark comparison](https://github.com/rcourtman/Pulse/actions/runs/39138980114/job/117709718583) completed execution but reported three regressions above 10% in test-only legacy history-filter references. Their corresponding production variants were unchanged or below 4%. Both filter implementation and benchmark source are byte-identical between baseline `84a96940185cc202aef0451d658bc89b864710a0` and synthetic candidate `ed25833fa8318e493d560239f60e56ec5ffad540`. The retained paired samples and source identities require an owning disposition; unchanged source alone does not classify the timing observation. The later [45a82ea03c comparison](https://github.com/rcourtman/Pulse/actions/runs/39145479921/job/117728549510) passed against newer baseline `3d6949851f9370898390c690bec7e62631c51f3b`; it does not erase the earlier samples. This advisory result does not replace the protected required checks or existing release gates. These are source repairs and qualification evidence. Release inclusion, affected installed workflows and the manual reliability exit decision remain open. No target, lane state or priority changes with this work. diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 489ed526de..942bfdc5c2 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -4208,6 +4208,20 @@ named and default organizations. The custom event envelope stays unchanged. ## Current State +### PBS authentication keeps an ended response caller's outcome + +Initial and ordinary PBS authentication preserve the owning caller's context +error when cancellation races with response consumption. A cancelled JSON +refusal cannot authorize a form retry, and a complete response cannot replace +the retained session after its caller has ended. The pending owner's result +keeps that context class for joined readers. Live malformed or refused +responses retain their existing admission and HTTP-status behavior; an +independent later login can recover without resetting retained credentials. +`TestClientCancelledAuthResponseOwnsErrorAndSession` deterministically cancels +during response reading for JSON/form, complete and incomplete success, +encoding and credential refusal, and body-read failure. The real TLS +constructor cancellation/deadline and shared-renewal controls remain separate. + ### AI Discovery removed (2026-10-11) The [AI runtime retirement boundary](ai-runtime.md#ai-discovery-removed-2026-10-11) diff --git a/pkg/pbs/client.go b/pkg/pbs/client.go index d63556a9d4..e0271ef9e4 100644 --- a/pkg/pbs/client.go +++ b/pkg/pbs/client.go @@ -261,9 +261,15 @@ func (c *Client) refreshAuthentication(ctx context.Context, force bool) error { password := c.config.Password err := c.authenticateJSON(ctx, username, password) - if shouldFallbackToForm(err) { + if ctx.Err() == nil && shouldFallbackToForm(err) { err = c.authenticateForm(ctx, username, password) } + // A response can finish as the request is cancelled. Preserve the owning + // caller's outcome before sharing it with waiters; an empty or refused + // response from that ended exchange is not a new credential diagnosis. + if ctxErr := ctx.Err(); ctxErr != nil { + err = ctxErr + } c.authMu.Lock() pending.err = err c.authInFlight = nil @@ -299,7 +305,7 @@ func (c *Client) authenticateJSON(ctx context.Context, username, password string } defer resp.Body.Close() - return c.handleAuthResponse(resp) + return c.handleAuthResponse(ctx, resp) } func (c *Client) authenticateForm(ctx context.Context, username, password string) error { @@ -324,10 +330,10 @@ func (c *Client) authenticateForm(ctx context.Context, username, password string } defer resp.Body.Close() - return c.handleAuthResponse(resp) + return c.handleAuthResponse(ctx, resp) } -func (c *Client) handleAuthResponse(resp *http.Response) error { +func (c *Client) handleAuthResponse(ctx context.Context, resp *http.Response) error { if resp.StatusCode != http.StatusOK { body, err := readResponseBodyLimited(resp.Body) if err != nil { @@ -349,11 +355,13 @@ func (c *Client) handleAuthResponse(resp *http.Response) error { } c.authMu.Lock() + defer c.authMu.Unlock() + if err := ctx.Err(); err != nil { + return err + } c.auth.ticket = ticket c.auth.csrfToken = csrf c.auth.expiresAt = time.Now().Add(2 * time.Hour) // PBS tickets expire after 2 hours - c.authMu.Unlock() - return nil } diff --git a/pkg/pbs/client_construction_context_test.go b/pkg/pbs/client_construction_context_test.go index 237083f61b..63e6769543 100644 --- a/pkg/pbs/client_construction_context_test.go +++ b/pkg/pbs/client_construction_context_test.go @@ -9,6 +9,7 @@ import ( "io" "net/http" "net/http/httptest" + "reflect" "strings" "sync" "sync/atomic" @@ -154,3 +155,75 @@ func testClientConstructionHonoursContext(t *testing.T) { } } } + +// Cancel while consuming the response, after HTTP has already returned 200 or +// a refusal. This reproduces the constructor race without scheduler timing. +type cancellationAuthBody struct { + io.Reader + cancel context.CancelFunc +} + +func (r cancellationAuthBody) Read(p []byte) (int, error) { + r.cancel() + return r.Reader.Read(p) +} + +func testClientCancelledAuthResponseOwnsErrorAndSession(t *testing.T) { + const good = `{"data":{"ticket":"recovered-ticket","CSRFPreventionToken":"recovered-csrf"}}` + for _, encoding := range []string{"json", "form"} { + for _, outcome := range []struct { + name, body string + status int + readError bool + }{ + {"invalid-success", `{}`, http.StatusOK, false}, + {"complete-success", good, http.StatusOK, false}, + {"encoding-refusal", `{}`, http.StatusUnsupportedMediaType, false}, + {"credential-refusal", `{}`, http.StatusForbidden, false}, + {"body-read-failure", good, http.StatusOK, true}, + } { + t.Run(encoding+"/"+outcome.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + calls, recovering := 0, false + c := &Client{ + baseURL: "https://pbs.fixture.invalid/api2/json", + config: ClientConfig{Password: "fixture-password"}, + auth: auth{user: "fixture", realm: "pbs", ticket: "retained-ticket", + csrfToken: "retained-csrf", expiresAt: time.Now().Add(-time.Hour)}, + } + c.httpClient = &http.Client{Timeout: 5 * time.Second, Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + calls++ + if encoding == "form" && r.Header.Get("Content-Type") == "application/json" { + return &http.Response{StatusCode: http.StatusUnsupportedMediaType, Body: io.NopCloser(strings.NewReader(`{}`)), ContentLength: -1}, nil + } + if recovering { + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(good)), ContentLength: -1}, nil + } + var body io.Reader = strings.NewReader(outcome.body) + if outcome.readError { + body = sessionAdmissionReadError{body} + } + body = cancellationAuthBody{Reader: body, cancel: cancel} + return &http.Response{StatusCode: outcome.status, Body: io.NopCloser(body), ContentLength: -1}, nil + })} + before := c.authSnapshot() + err := c.authenticate(ctx) + if !errors.Is(err, context.Canceled) || !reflect.DeepEqual(c.authSnapshot(), before) { + t.Errorf("ended response lost caller or changed retained session: error=%v session_changed=%t", err, !reflect.DeepEqual(c.authSnapshot(), before)) + } + want := 1 + if encoding == "form" { + want = 2 + } + if calls != want || c.authInFlight != nil { + t.Errorf("cancellation replayed login or retained owner: calls=%d pending=%t", calls, c.authInFlight != nil) + } + recovering = true + if err := c.authenticate(context.Background()); err != nil || c.auth.ticket != "recovered-ticket" || c.auth.csrfToken != "recovered-csrf" || calls != 2*want { + t.Errorf("distinct later authentication did not recover coherently: error=%v calls=%d", err, calls) + } + }) + } + } +} diff --git a/pkg/pbs/host_diagnostics_regression_test.go b/pkg/pbs/host_diagnostics_regression_test.go index 6c1c0060e4..03c59df9db 100644 --- a/pkg/pbs/host_diagnostics_regression_test.go +++ b/pkg/pbs/host_diagnostics_regression_test.go @@ -83,3 +83,7 @@ func TestClientAcceptedHostDiagnosticsRemainUseful(t *testing.T) { func TestClientConstructionHonoursContext(t *testing.T) { testClientConstructionHonoursContext(t) } + +func TestClientCancelledAuthResponseOwnsErrorAndSession(t *testing.T) { + testClientCancelledAuthResponseOwnsErrorAndSession(t) +} diff --git a/pkg/pbs/session_response_admission_test.go b/pkg/pbs/session_response_admission_test.go index b7f7624485..c5232bc9e2 100644 --- a/pkg/pbs/session_response_admission_test.go +++ b/pkg/pbs/session_response_admission_test.go @@ -71,7 +71,7 @@ func TestClientSessionResponseAdmission(t *testing.T) { body = sessionAdmissionReadError{body} } resp := &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(body), ContentLength: -1} - err := c.handleAuthResponse(resp) + err := c.handleAuthResponse(context.Background(), resp) resp.Body.Close() if !tc.valid { if err == nil {