From e4ca39cbec44114cc311afd7db02e1f1b47570df Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 29 Jul 2026 13:22:36 -0400 Subject: [PATCH] Bound implicit OAuth initialization waits --- src/lib/config.ts | 2 +- src/lib/coordination.ts | 10 ++- src/lib/persistent-oauth-client-provider.ts | 2 +- tests/integration/dead-backend.test.ts | 61 ++++++++++++-- tests/unit/implicit-oauth-timeouts.test.ts | 93 +++++++++++++++++++++ 5 files changed, 156 insertions(+), 12 deletions(-) create mode 100644 tests/unit/implicit-oauth-timeouts.test.ts diff --git a/src/lib/config.ts b/src/lib/config.ts index 34d90d6..399a954 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -59,7 +59,7 @@ export const CONFIG = { OAUTH_SCOPES: process.env.OAUTH_SCOPES || '', // Timeout Configuration (in milliseconds) - OAUTH_TIMEOUT: 30000, // 30 seconds + OAUTH_TIMEOUT: parsePositiveIntEnv(process.env.OAUTH_TIMEOUT_MS, 30000), // 30 seconds LOCK_TIMEOUT: 300000, // 5 minutes // WordPress request timeouts. Native fetch has no default timeout, so a diff --git a/src/lib/coordination.ts b/src/lib/coordination.ts index 0f7b38d..b9eb8cb 100644 --- a/src/lib/coordination.ts +++ b/src/lib/coordination.ts @@ -94,7 +94,7 @@ class LockfileManager { /** * Wait for lock to be released */ - async waitForRelease(timeout: number = CONFIG.LOCK_TIMEOUT): Promise { + async waitForRelease(timeout: number = CONFIG.OAUTH_TIMEOUT): Promise { const startTime = Date.now(); return new Promise((resolve, reject) => { @@ -114,13 +114,15 @@ class LockfileManager { return; } - if (Date.now() - startTime > timeout) { + const elapsed = Date.now() - startTime; + if (elapsed >= timeout) { reject(new OAuthError('Timeout waiting for auth lock', 'LOCK_TIMEOUT')); return; } - // Check again in 1 second - setTimeout(checkLock, 1000); + // Poll normally once per second, without allowing the final poll to + // extend the caller's operation budget. + setTimeout(checkLock, Math.min(1000, timeout - elapsed)); }; logger.info('Waiting for other instance to complete authentication...', 'COORDINATION'); diff --git a/src/lib/persistent-oauth-client-provider.ts b/src/lib/persistent-oauth-client-provider.ts index 759ad32..ea43adb 100644 --- a/src/lib/persistent-oauth-client-provider.ts +++ b/src/lib/persistent-oauth-client-provider.ts @@ -324,7 +324,7 @@ export class PersistentWPOAuthClientProvider { const timeout = setTimeout(() => { cleanup(); reject(new OAuthError('Authorization timeout', 'TIMEOUT')); - }, CONFIG.LOCK_TIMEOUT); + }, this.options.timeout); const cleanup = () => { this.events.removeAllListeners('oauth-success'); diff --git a/tests/integration/dead-backend.test.ts b/tests/integration/dead-backend.test.ts index 0e5bef4..e68bb8b 100644 --- a/tests/integration/dead-backend.test.ts +++ b/tests/integration/dead-backend.test.ts @@ -35,7 +35,11 @@ function collectMessages(proc: ChildProcess, count: number, ms = 15_000): Promis let buffer = ''; const timer = setTimeout(() => { cleanup(); - reject(new Error(`Timed out after ${ms}ms waiting for ${count} message(s), got ${messages.length}: ${JSON.stringify(messages)}`)); + reject( + new Error( + `Timed out after ${ms}ms waiting for ${count} message(s), got ${messages.length}: ${JSON.stringify(messages)}` + ) + ); }, ms); function onData(chunk: Buffer) { @@ -70,9 +74,10 @@ function collectMessages(proc: ChildProcess, count: number, ms = 15_000): Promis describe('dead backend integration', () => { let proxy: ChildProcess; - afterEach(() => { - if (proxy && !proxy.killed) { - proxy.kill(); + afterEach(async () => { + if (proxy && proxy.exitCode === null && proxy.signalCode === null) { + proxy.kill('SIGKILL'); + await once(proxy, 'exit'); } }); @@ -84,7 +89,7 @@ describe('dead backend integration', () => { ...process.env, WP_API_URL: 'http://192.0.2.1:1', JWT_TOKEN: 'test-dead-backend-token', - LOG_LEVEL: '0', // suppress logs on stderr + LOG_LEVEL: '0', // suppress logs on stderr NODE_ENV: 'test', }, stdio: ['pipe', 'pipe', 'pipe'], @@ -161,7 +166,9 @@ describe('dead backend integration', () => { // The response must be a clean MCP error, NOT a forwarded malformed request. // The init-ready gate should have caught this and returned an error. expect(toolsResponse.error).toBeDefined(); - expect(toolsResponse.error.message).toMatch(/WordPress connection failed during initialization/); + expect(toolsResponse.error.message).toMatch( + /WordPress connection failed during initialization/ + ); // The error must carry the underlying cause in `data` so the client can // explain why init failed, rather than a bare internal error (issue #61). @@ -169,6 +176,48 @@ describe('dead backend integration', () => { expect(toolsResponse.error.data.reason).toBe('failed'); }, 30_000); + it('returns the structured fallback when headless implicit OAuth has no callback', async () => { + const oauthTimeout = 100; + proxy = spawn('node', [PROXY_PATH], { + env: { + ...process.env, + WP_API_URL: 'http://192.0.2.1:1', + OAUTH_ENABLED: 'true', + OAUTH_FLOW_TYPE: 'implicit', + OAUTH_USE_PKCE: 'false', + OAUTH_AUTHORIZE_ENDPOINT: '/authorize', + OAUTH_TIMEOUT_MS: String(oauthTimeout), + OAUTH_CALLBACK_PORT: String(20_000 + Math.floor(Math.random() * 10_000)), + WP_MCP_CONFIG_DIR: join(process.cwd(), '.tmp-implicit-oauth-auth'), + LOG_LEVEL: '0', + NODE_ENV: 'test', + }, + stdio: ['pipe', 'pipe', 'pipe'], + }); + proxy.stderr!.resume(); + await new Promise(r => setTimeout(r, 200)); + + const startedAt = Date.now(); + send(proxy, { + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { + protocolVersion: '2025-06-18', + clientInfo: { name: 'headless-implicit-oauth-test', version: '1.0.0' }, + capabilities: {}, + }, + }); + + const [initResponse] = await collectMessages(proxy, 1, 2_000); + + expect(Date.now() - startedAt).toBeLessThan(2_000); + expect(initResponse.result.capabilities).toEqual({ + experimental: { connectionFailed: expect.any(Object) }, + }); + expect(initResponse.result.instructions).toMatch(/Connection Failed/i); + }, 5_000); + // Healthy-backend integration test omitted: unit tests cover the happy path. // A full test here needs a real or mocked WordPress endpoint in the child process. }); diff --git a/tests/unit/implicit-oauth-timeouts.test.ts b/tests/unit/implicit-oauth-timeouts.test.ts new file mode 100644 index 0000000..b2c6b6a --- /dev/null +++ b/tests/unit/implicit-oauth-timeouts.test.ts @@ -0,0 +1,93 @@ +/** + * Regression coverage for implicit OAuth waits. The operation budget must bound + * callers even though a lock may remain valid longer for coordination purposes. + */ + +import { jest } from '@jest/globals'; +import { EventEmitter } from 'node:events'; +import fs from 'node:fs'; +import path from 'node:path'; +import tmp from 'tmp'; +import { mockEnv } from '../utils/test-helpers.js'; + +describe('implicit OAuth timeout budgets', () => { + let tempDir: string; + let restoreEnv: () => void; + + beforeEach(() => { + tempDir = tmp.dirSync({ unsafeCleanup: true }).name; + restoreEnv = mockEnv({ + WP_MCP_CONFIG_DIR: tempDir, + OAUTH_TIMEOUT_MS: '100', + WP_API_URL: 'https://example.com', + }); + jest.resetModules(); + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + restoreEnv(); + }); + + it('times out an absent callback using the provider operation budget', async () => { + const { PersistentWPOAuthClientProvider } = await import( + '../../src/lib/persistent-oauth-client-provider.js' + ); + const provider = new PersistentWPOAuthClientProvider({ + serverUrl: 'https://example.com', + timeout: 50, + }); + const waiting = (provider as any).waitForAuthorizationResult(); + const result = waiting.catch((error: unknown) => error); + + await jest.advanceTimersByTimeAsync(49); + let settled = false; + void result.then(() => { + settled = true; + }); + await Promise.resolve(); + expect(settled).toBe(false); + + await jest.advanceTimersByTimeAsync(1); + await expect(result).resolves.toMatchObject({ code: 'TIMEOUT' }); + }); + + it('continues to accept a successful interactive callback before the deadline', async () => { + const { PersistentWPOAuthClientProvider } = await import( + '../../src/lib/persistent-oauth-client-provider.js' + ); + const provider = new PersistentWPOAuthClientProvider({ + serverUrl: 'https://example.com', + timeout: 50, + }); + const tokens = { access_token: 'interactive-token', token_type: 'Bearer' }; + const waiting = (provider as any).waitForAuthorizationResult(); + + (provider as any).events.emit('oauth-success', tokens); + + await expect(waiting).resolves.toEqual(tokens); + }); + + it('bounds a secondary process lock wait by the OAuth operation budget', async () => { + const { WPAuthCoordinator } = await import('../../src/lib/coordination.js'); + const { getConfigDir } = await import('../../src/lib/persistent-auth-config.js'); + const hash = 'implicit-oauth-timeout'; + fs.mkdirSync(getConfigDir(), { recursive: true }); + fs.writeFileSync( + path.join(getConfigDir(), `${hash}_auth.lock`), + JSON.stringify({ pid: process.pid, port: 0, timestamp: Date.now(), hostname: 'test' }) + ); + const coordinator = new WPAuthCoordinator( + hash, + 'https://example.com', + 7665, + new EventEmitter() + ); + const waiting = (coordinator as any).lockManager.waitForRelease(); + const result = waiting.catch((error: unknown) => error); + + await jest.advanceTimersByTimeAsync(100); + await expect(result).resolves.toMatchObject({ code: 'LOCK_TIMEOUT' }); + }); +});