From 41e2a6a751f42fbbf1f98b57fa20e6f4995d7869 Mon Sep 17 00:00:00 2001 From: 4gray Date: Mon, 3 Aug 2026 22:23:05 +0200 Subject: [PATCH] fix(stalker): stop discovery when an abandoned auth outlives its drain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cancellation is cooperative — the PWA fetch takes no signal and the Electron main process runs its request to completion — so the bounded drain is a hope, not a guarantee. An attempt still unsettled after its 65 s budget plus the 15 s drain may yet land a get_profile, which adopts the MAC's token portal-side; advancing staked the next candidate's freshly issued session on that never happening. The rejection now reports it and the candidate loop stops, preferring a retryable "could not confirm" over a session that looks established and dies later. Co-Authored-By: Claude Fable 5 --- docs/architecture/stalker-portal.md | 12 +++++ .../stalker-portal-discovery.service.spec.ts | 47 +++++++++++++++++++ .../lib/stalker-portal-discovery.service.ts | 41 ++++++++++++++-- 3 files changed, 97 insertions(+), 3 deletions(-) diff --git a/docs/architecture/stalker-portal.md b/docs/architecture/stalker-portal.md index 6522278d8..bc9b50bd0 100644 --- a/docs/architecture/stalker-portal.md +++ b/docs/architecture/stalker-portal.md @@ -382,6 +382,18 @@ dispatched — which the check cannot cover, but sequencing can: discovery the next candidate, instead of racing it. The request cannot be un-sent, but nothing forces us to have a competing session in flight while it lands. +**A drain that times out stops discovery.** Draining is bounded, and the bound +has to mean something: an attempt still unsettled after its own 65 s budget +plus the 15 s drain is one no transport can recall — the PWA `fetch()` takes no +signal at all, and the Electron main process runs its HTTP request to +completion. Advancing anyway would stake the next candidate's freshly issued +session on that request never landing. So the rejection carries +`abandonedInFlight` and the candidate loop returns instead of probing on, +preferring an honest "could not confirm this portal" the user can retry over a +session that looks established and dies later. It costs nothing in the normal +case: an aborted attempt settles as soon as its in-flight request errors out, +so the drain returns at once and the loop continues. + The budget itself covers the longest real flow: a status-2 portal costs four sequential requests (handshake, profile, `do_auth`, profile retry) and the Electron transport allows each 15 s, so a two-request budget would have failed diff --git a/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.spec.ts b/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.spec.ts index 44bc89939..a03d54a1d 100644 --- a/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.spec.ts +++ b/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.spec.ts @@ -453,4 +453,51 @@ describe('StalkerPortalDiscoveryService', () => { jest.useRealTimers(); } }); + + it('stops discovery when the drain deadline expires with the attempt still live', async () => { + jest.useFakeTimers(); + try { + // Cancellation is cooperative: neither transport can pull a + // request off the wire. An attempt still unsettled 80 s in may + // yet land its `get_profile`, which adopts the MAC's token + // portal-side — so probing on would stake the next candidate's + // freshly issued session on a request nobody can recall. + mockProbes({ + 'http://hung.example/portal.php': { + resolve: 'Authorization failed.', + }, + 'http://hung.example/server/load.php': { + resolve: 'Authorization failed.', + }, + }); + + // Never settles — not even after the drain. + authenticate + .mockImplementationOnce(() => new Promise(() => undefined)) + .mockResolvedValue({ token: 'SECOND' }); + + const discovery = service.discover('http://hung.example/c', MAC); + for (let i = 0; i < 6; i += 1) { + await Promise.resolve(); + } + + jest.advanceTimersByTime(65_000); + for (let i = 0; i < 6; i += 1) { + await Promise.resolve(); + } + jest.advanceTimersByTime(15_000); + for (let i = 0; i < 10; i += 1) { + await Promise.resolve(); + } + + await expect(discovery).resolves.toMatchObject({ + status: 'auth-rejected', + abandonedInFlight: true, + }); + // The second candidate was never authenticated. + expect(authenticate).toHaveBeenCalledTimes(1); + } finally { + jest.useRealTimers(); + } + }); }); diff --git a/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.ts b/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.ts index 5a028efc5..009c1c8e5 100644 --- a/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.ts +++ b/libs/portal/stalker/data-access/src/lib/stalker-portal-discovery.service.ts @@ -45,6 +45,16 @@ export interface StalkerPortalDiscoveryRejection { status: 'auth-rejected'; portalUrl: string; error?: unknown; + /** + * The abandoned attempt was STILL in flight when the drain deadline + * expired, so discovery must not advance. Cancellation is cooperative — + * neither transport can pull a request off the wire (the PWA `fetch()` + * takes no signal, and the Electron main process runs its HTTP request to + * completion) — so an attempt this far past its deadline may still land a + * `get_profile`, which adopts the MAC's token portal-side and would + * invalidate the session a later candidate had just established. + */ + abandonedInFlight?: boolean; } /** No candidate answered like a Stalker portal (host down or not a portal). */ @@ -156,6 +166,9 @@ export class StalkerPortalDiscoveryService { if (outcome.status === 'resolved') { return outcome; } + if (outcome.abandonedInFlight) { + return authRejection ?? outcome; + } authRejection = authRejection ?? outcome; continue; } @@ -201,6 +214,11 @@ export class StalkerPortalDiscoveryService { if (outcome.status === 'resolved') { return outcome; } + // An attempt still on the wire outranks further probing: + // see `abandonedInFlight`. + if (outcome.abandonedInFlight) { + return authRejection ?? outcome; + } // The endpoint is real but refused our credentials; // remember the first such endpoint in case no later // candidate resolves. @@ -268,16 +286,33 @@ export class StalkerPortalDiscoveryService { // wire: its `get_profile` adopts the MAC's token portal-side, so // racing it is exactly what invalidates the next candidate's // freshly issued session. - await Promise.race([ - pending.catch(() => undefined), - new Promise((resolve) => + // + // Draining is the normal case and usually returns at once — an + // aborted attempt settles as soon as its in-flight request errors + // out. The deadline exists for the attempt that does not settle, + // and reaching it is reported rather than swallowed: continuing + // would stake a working candidate's session on a request nobody + // can recall. + const DRAINED = Symbol('drained'); + const outcome = await Promise.race([ + pending.then( + () => DRAINED, + () => DRAINED + ), + new Promise((resolve) => setTimeout(resolve, ABANDONED_DRAIN_MS) ), ]); + if (outcome !== DRAINED) { + this.logger.warn( + 'Abandoned Stalker authentication is still in flight after the drain deadline; stopping discovery rather than racing it' + ); + } return { status: 'auth-rejected', portalUrl: candidate, error, + ...(outcome === DRAINED ? {} : { abandonedInFlight: true }), }; } }