From e318cf850370f4bbeebd568edec51b7fe63a9311 Mon Sep 17 00:00:00 2001 From: Trynax Date: Sun, 30 Aug 2026 07:45:54 +0100 Subject: [PATCH 1/2] fix(oauth): enforce hard polling deadlines --- src/client.ts | 8 ++- src/commands/oauth.ts | 122 +++++++++++++++++++++++++++++++++++------- 2 files changed, 110 insertions(+), 20 deletions(-) diff --git a/src/client.ts b/src/client.ts index 93599a3..6422956 100644 --- a/src/client.ts +++ b/src/client.ts @@ -683,7 +683,11 @@ export async function initiateOAuth( ); } -export async function listOAuthBindings(jwtToken: string, apiHost: string) { +export async function listOAuthBindings( + jwtToken: string, + apiHost: string, + signal?: AbortSignal, +) { return request>( `${scheme(apiHost)}://${apiHost}/api/oauth/bindings`, - { method: 'GET', headers: jwtHeaders(jwtToken) }, + { method: 'GET', headers: jwtHeaders(jwtToken), signal }, DEFAULT_TIMEOUT_MS, IDEMPOTENT_RETRIES, ); diff --git a/src/commands/oauth.ts b/src/commands/oauth.ts index 411c094..71fd0ce 100644 --- a/src/commands/oauth.ts +++ b/src/commands/oauth.ts @@ -24,6 +24,7 @@ import { initiateOAuth, listOAuthBindings, deleteOAuthBinding, + isRetryableRequestError, } from '../client.ts'; import type { OAuthProvider, ScopeDefinition } from '../client.ts'; import { output, err } from '../format.ts'; @@ -67,6 +68,69 @@ function bindingChangedAfter( return changedAt >= startedAtMs; } +const POLL_DEADLINE = Symbol('oauth poll deadline'); + +/** Wait for an interval without sleeping past the polling deadline. */ +function waitForPollInterval(ms: number, signal: AbortSignal): Promise { + return new Promise((resolve) => { + if (signal.aborted) { + resolve(false); + return; + } + + let timer: ReturnType | undefined; + const cleanup = () => signal.removeEventListener('abort', onAbort); + const onAbort = () => { + if (timer !== undefined) clearTimeout(timer); + cleanup(); + resolve(false); + }; + + timer = setTimeout(() => { + cleanup(); + resolve(true); + }, Math.max(0, ms)); + signal.addEventListener('abort', onAbort, { once: true }); + if (signal.aborted) onAbort(); + }); +} + +/** Resolve when an operation finishes or when the poll deadline aborts it. */ +function resolveOnPollAbort( + operation: Promise, + signal: AbortSignal, +): Promise { + return new Promise((resolve, reject) => { + let settled = false; + const cleanup = () => signal.removeEventListener('abort', onAbort); + const onAbort = () => { + if (settled) return; + settled = true; + cleanup(); + resolve(POLL_DEADLINE); + }; + const resolveOperation = (value: T) => { + if (settled) return; + settled = true; + cleanup(); + resolve(value); + }; + const rejectOperation = (error: unknown) => { + if (settled) return; + settled = true; + cleanup(); + reject(error); + }; + + signal.addEventListener('abort', onAbort, { once: true }); + operation.then(resolveOperation, rejectOperation); + if (signal.aborted) { + onAbort(); + return; + } + }); +} + export async function pollForBinding( apiKeyId: string, providerId: string, @@ -79,28 +143,50 @@ export async function pollForBinding( const deadline = Date.now() + timeoutMs; const isTTY = process.stdout.isTTY; const startedAtMs = startedAt.getTime() - 5000; + const controller = new AbortController(); + const deadlineTimer = setTimeout( + () => controller.abort(), + Math.max(0, deadline - Date.now()), + ); - while (Date.now() < deadline) { - await new Promise((r) => setTimeout(r, intervalMs)); + try { + while (Date.now() < deadline) { + const remaining = deadline - Date.now(); + const intervalElapsed = await waitForPollInterval( + Math.min(Math.max(0, intervalMs), remaining), + controller.signal, + ); + if (!intervalElapsed || Date.now() >= deadline || controller.signal.aborted) break; - try { - const bindings = await listOAuthBindings(jwtToken, XAPI_API_HOST); - const match = Array.isArray(bindings) - ? bindings.find((b) => - b.apiKeyId === apiKeyId && - b.providerId === providerId && - bindingChangedAfter(b, startedAtMs, existingBindingIds) - ) - : null; - if (match) return match; - } catch { - // transient error — keep polling - } + try { + const bindings = await resolveOnPollAbort( + listOAuthBindings(jwtToken, XAPI_API_HOST, controller.signal), + controller.signal, + ); + if (bindings === POLL_DEADLINE || Date.now() >= deadline) break; + + const match = Array.isArray(bindings) + ? bindings.find((b) => + b.apiKeyId === apiKeyId && + b.providerId === providerId && + bindingChangedAfter(b, startedAtMs, existingBindingIds) + ) + : null; + if (match) return match; + } catch (e) { + if (controller.signal.aborted || Date.now() >= deadline) break; + if (!isRetryableRequestError(e)) throw e; + // Transient errors are retried until the deadline. + } - if (isTTY) { - const remaining = Math.ceil((deadline - Date.now()) / 1000); - process.stdout.write(`\r Waiting for authorization... (${remaining}s remaining) `); + if (isTTY) { + const remaining = Math.ceil((deadline - Date.now()) / 1000); + process.stdout.write(`\r Waiting for authorization... (${remaining}s remaining) `); + } } + } finally { + clearTimeout(deadlineTimer); + controller.abort(); } if (process.stdout.isTTY) process.stdout.write('\n'); From ffc24382280795781ee3ae05339521db3b580c05 Mon Sep 17 00:00:00 2001 From: Trynax Date: Sun, 30 Aug 2026 07:45:54 +0100 Subject: [PATCH 2/2] test(oauth): cover polling deadline behavior --- src/tests/oauth.test.ts | 140 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/src/tests/oauth.test.ts b/src/tests/oauth.test.ts index a6504f1..71ec7ce 100644 --- a/src/tests/oauth.test.ts +++ b/src/tests/oauth.test.ts @@ -212,6 +212,146 @@ describe('oauth commands', () => { }); describe('pollForBinding', () => { + it('does not start a poll after the deadline', async () => { + const listBindingsSpy = spyOn(client, 'listOAuthBindings').mockResolvedValue([] as any); + + try { + const binding = await pollForBinding( + MOCK_KEY_ID, + 'prov-1', + MOCK_JWT, + new Date(), + new Set(), + 0, + 10, + ); + + expect(binding).toBeNull(); + expect(listBindingsSpy).not.toHaveBeenCalled(); + } finally { + listBindingsSpy.mockRestore(); + } + }); + + it('cancels an in-flight poll when the deadline expires', async () => { + let aborted = false; + const listBindingsSpy = spyOn(client, 'listOAuthBindings').mockImplementation( + async (_jwtToken, _apiHost, signal) => { + await new Promise((_resolve, reject) => { + if (signal?.aborted) { + aborted = true; + reject(new DOMException('aborted', 'AbortError')); + return; + } + signal?.addEventListener('abort', () => { + aborted = true; + reject(new DOMException('aborted', 'AbortError')); + }, { once: true }); + }); + return [] as any; + }, + ); + + try { + const binding = await pollForBinding( + MOCK_KEY_ID, + 'prov-1', + MOCK_JWT, + new Date(), + new Set(), + 20, + 1, + ); + + expect(binding).toBeNull(); + expect(aborted).toBe(true); + expect(listBindingsSpy).toHaveBeenCalledWith( + MOCK_JWT, + expect.any(String), + expect.any(AbortSignal), + ); + } finally { + listBindingsSpy.mockRestore(); + } + }); + + it('does not accept a binding returned after the deadline', async () => { + const listBindingsSpy = spyOn(client, 'listOAuthBindings').mockImplementation( + async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + return [mockBindings[0]] as any; + }, + ); + + try { + const binding = await pollForBinding( + MOCK_KEY_ID, + 'prov-1', + MOCK_JWT, + new Date(), + new Set(), + 10, + 1, + ); + + expect(binding).toBeNull(); + expect(listBindingsSpy).toHaveBeenCalledTimes(1); + } finally { + listBindingsSpy.mockRestore(); + } + }); + + it('surfaces non-retryable polling errors immediately', async () => { + const listBindingsSpy = spyOn(client, 'listOAuthBindings') + .mockRejectedValue(new client.HttpError(401, 'unauthorized')); + + try { + await expect( + pollForBinding( + MOCK_KEY_ID, + 'prov-1', + MOCK_JWT, + new Date(), + new Set(), + 100, + 1, + ), + ).rejects.toThrow('HTTP 401'); + expect(listBindingsSpy).toHaveBeenCalledTimes(1); + } finally { + listBindingsSpy.mockRestore(); + } + }); + + it('continues polling after transient errors', async () => { + const listBindingsSpy = spyOn(client, 'listOAuthBindings') + .mockRejectedValueOnce(new TypeError('fetch failed')) + .mockResolvedValueOnce([ + { + ...mockBindings[0], + createdAt: '2026-06-03T00:00:01.000Z', + updatedAt: '2026-06-03T00:00:01.000Z', + }, + ] as any); + + try { + const binding = await pollForBinding( + MOCK_KEY_ID, + 'prov-1', + MOCK_JWT, + new Date('2026-06-03T00:00:00.000Z'), + new Set(), + 100, + 1, + ); + + expect(binding?.id).toBe('bind-uuid-1'); + expect(listBindingsSpy).toHaveBeenCalledTimes(2); + } finally { + listBindingsSpy.mockRestore(); + } + }); + it('ignores existing bindings from before the current authorization', async () => { const listBindingsSpy = spyOn(client, 'listOAuthBindings') .mockResolvedValueOnce([