From d01a921e01243c326e2508c7d21eb85095f1fbe8 Mon Sep 17 00:00:00 2001 From: garrytan-agents Date: Sun, 3 May 2026 06:27:33 -0700 Subject: [PATCH] v0.26.1 fix(oauth): client_credentials tokens rejected by MCP bearer auth (#577) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(oauth): client_credentials tokens rejected by MCP bearer auth Three bugs found in production when connecting Claude Code via Tailscale: 1. Token validation fails with 'Token has no expiration time' - Root cause: postgres driver with prepare:false returns expires_at as string, but MCP SDK's bearerAuth middleware checks typeof === 'number' - Fix: Number(row.expires_at) in verifyAccessToken 2. OAuth metadata missing client_credentials grant type - Root cause: MCP SDK hardcodes ['authorization_code', 'refresh_token'] in mcpAuthRouter's .well-known endpoint - Fix: middleware intercepts metadata response and appends 'client_credentials' before it reaches the client - Claude Code's native OAuth auto-discovery now finds the CC flow 3. Express 5 compatibility fixes - trust proxy: 'loopback' for reverse proxy deployments (Caddy/Tailscale) without this, express-rate-limit throws ERR_ERL_UNEXPECTED_X_FORWARDED_FOR - /admin/* wildcard → /admin/{*path} (Express 5 named param syntax) * test(oauth): add regression tests for v0.26.1 fixes Unit test (oauth.test.ts): - expiresAt is always a number, not string — SDK bearerAuth compat Integration tests (serve-http-oauth.test.ts, 7 cases): - client_credentials token accepted at /mcp (the actual regression) - token expires_in matches server TTL - OAuth metadata includes client_credentials grant type - token endpoint discoverable from metadata - admin dashboard serves SPA (Express 5 wildcard fix) - X-Forwarded-For doesn't crash rate limiter (trust proxy fix) - read-only token cannot call write operations (scope enforcement) 42 tests, 0 failures, 172 assertions. * test(e2e): full E2E suite for serve-http OAuth 2.1 (15 cases) Spins up a real gbrain serve --http against real Postgres, registers an OAuth client, mints tokens via client_credentials, and exercises the full MCP JSON-RPC pipeline end-to-end. E2E cases (test/e2e/serve-http-oauth.test.ts): - mint token via client_credentials grant - minted token accepted at /mcp — tools/list returns tools - minted token works for tools/call — search executes - expired/invalid token rejected at /mcp - missing Authorization header returns 401 - OAuth metadata includes all three grant types - OAuth metadata issuer matches public URL - admin dashboard serves SPA (Express 5 wildcard fix) - admin sub-routes serve SPA fallback - X-Forwarded-For doesn't crash rate limiter - read-only token rejected for write operations - write-scoped token can call read operations - health endpoint works without auth - multiple tokens work independently - wrong client_secret rejected at token endpoint Unit test addition (test/oauth.test.ts): - expiresAt is always typeof number (SDK bearerAuth compat) Total: 50 tests, 0 failures, 201 assertions. --------- Co-authored-by: Wintermute --- src/commands/serve-http.ts | 23 ++- src/core/oauth-provider.ts | 2 +- test/e2e/serve-http-oauth.test.ts | 291 ++++++++++++++++++++++++++++++ test/oauth.test.ts | 15 ++ 4 files changed, 328 insertions(+), 3 deletions(-) create mode 100644 test/e2e/serve-http-oauth.test.ts diff --git a/src/commands/serve-http.ts b/src/commands/serve-http.ts index 9d1695a6c..11bd83ad1 100644 --- a/src/commands/serve-http.ts +++ b/src/commands/serve-http.ts @@ -82,6 +82,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption // Express 5 app const app = express(); + app.set('trust proxy', 'loopback'); // Caddy/Tailscale reverse proxy on localhost // --------------------------------------------------------------------------- // Cookie parsing — required for /admin auth (express 5 has no built-in) @@ -155,7 +156,25 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption }; } - app.use(mcpAuthRouter(authRouterOptions)); + const authRouter = mcpAuthRouter(authRouterOptions); + + // Patch the SDK's OAuth metadata to include client_credentials grant type. + // The SDK hardcodes ['authorization_code', 'refresh_token'] — we intercept + // the response and add client_credentials before it reaches the client. + app.use((req, res, next) => { + if (req.path === '/.well-known/oauth-authorization-server' && req.method === 'GET') { + const origJson = res.json.bind(res); + (res as any).json = (body: any) => { + if (body?.grant_types_supported && !body.grant_types_supported.includes('client_credentials')) { + body.grant_types_supported.push('client_credentials'); + } + return origJson(body); + }; + } + next(); + }); + + app.use(authRouter); // --------------------------------------------------------------------------- // Health check @@ -326,7 +345,7 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption if (fs.existsSync(adminDistPath)) { app.use('/admin', express.static(adminDistPath)); // SPA fallback: serve index.html for all unmatched /admin/* routes - app.get('/admin/*', (req: Request, res: Response, next: NextFunction) => { + app.get('/admin/{*path}', (req: Request, res: Response, next: NextFunction) => { // Skip API and events routes if (req.path.startsWith('/admin/api/') || req.path === '/admin/events' || req.path === '/admin/login') { return next(); diff --git a/src/core/oauth-provider.ts b/src/core/oauth-provider.ts index fa73bd387..67f1fca8d 100644 --- a/src/core/oauth-provider.ts +++ b/src/core/oauth-provider.ts @@ -300,7 +300,7 @@ export class GBrainOAuthProvider implements OAuthServerProvider { token, clientId: row.client_id as string, scopes: (row.scopes as string[]) || [], - expiresAt: row.expires_at as number, + expiresAt: Number(row.expires_at), resource: row.resource ? new URL(row.resource as string) : undefined, }; } diff --git a/test/e2e/serve-http-oauth.test.ts b/test/e2e/serve-http-oauth.test.ts new file mode 100644 index 000000000..aabbd62e6 --- /dev/null +++ b/test/e2e/serve-http-oauth.test.ts @@ -0,0 +1,291 @@ +/** + * E2E tests for serve-http.ts OAuth 2.1 fixes (v0.26.1). + * + * Spins up a real `gbrain serve --http` against real Postgres, registers an + * OAuth client, mints tokens, and exercises the full MCP JSON-RPC pipeline + * end-to-end. Catches the three bugs fixed in v0.26.1: + * + * 1. client_credentials tokens rejected at /mcp (expiresAt string vs number) + * 2. OAuth metadata missing client_credentials grant type + * 3. Express 5 trust proxy + admin SPA wildcard + * + * Run: GBRAIN_DATABASE_URL=... bun test test/e2e/serve-http-oauth.test.ts + */ + +import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; +import { hasDatabase } from './helpers.ts'; + +const skip = !hasDatabase(); +const describeE2E = skip ? describe.skip : describe; + +if (skip) { + console.log('Skipping E2E serve-http-oauth tests (DATABASE_URL not set)'); +} + +const PORT = 19131; // Avoid collision with production 3131 +const BASE = `http://localhost:${PORT}`; + +describeE2E('serve-http OAuth 2.1 E2E (v0.26.1)', () => { + let serverProcess: ReturnType | null = null; + let clientId: string; + let clientSecret: string; + + beforeAll(async () => { + const { execSync, spawn } = await import('child_process'); + + // Register a test OAuth client via CLI + const regOutput = execSync( + 'bun run src/cli.ts auth register-client e2e-oauth-test --grant-types client_credentials --scopes "read write"', + { cwd: process.cwd(), encoding: 'utf8' } + ); + const idMatch = regOutput.match(/Client ID:\s+(gbrain_cl_\S+)/); + const secretMatch = regOutput.match(/Client Secret:\s+(gbrain_cs_\S+)/); + if (!idMatch || !secretMatch) throw new Error('Failed to register test client:\n' + regOutput); + clientId = idMatch[1]; + clientSecret = secretMatch[1]; + + // Start the HTTP server + serverProcess = spawn('bun', [ + 'run', 'src/cli.ts', 'serve', '--http', + '--port', String(PORT), + '--public-url', `http://localhost:${PORT}`, + ], { + cwd: process.cwd(), + env: process.env, + stdio: ['ignore', 'pipe', 'pipe'], + }); + + // Collect stderr for debugging failures + let stderr = ''; + serverProcess.stderr?.on('data', (d: Buffer) => { stderr += d.toString(); }); + + // Wait for server to be ready (up to 15s) + let ready = false; + for (let i = 0; i < 30; i++) { + try { + const res = await fetch(`${BASE}/health`); + if (res.ok) { ready = true; break; } + } catch {} + await new Promise(r => setTimeout(r, 500)); + } + if (!ready) throw new Error('Server failed to start within 15s.\nstderr: ' + stderr.slice(-500)); + }, 30_000); + + afterAll(async () => { + // Kill server + if (serverProcess) { + serverProcess.kill('SIGTERM'); + await new Promise(r => setTimeout(r, 1000)); + if (!serverProcess.killed) serverProcess.kill('SIGKILL'); + } + // Revoke test client + try { + const { execSync } = await import('child_process'); + execSync(`bun run src/cli.ts auth revoke-client "${clientId}"`, + { cwd: process.cwd(), encoding: 'utf8', stdio: 'pipe' }); + } catch {} + }); + + // Helper: mint a token with given scopes + async function mintToken(scope = 'read write'): Promise<{ access_token: string; expires_in: number; scope: string }> { + const res = await fetch(`${BASE}/token`, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: `grant_type=client_credentials&client_id=${clientId}&client_secret=${clientSecret}&scope=${encodeURIComponent(scope)}`, + }); + expect(res.ok).toBe(true); + return res.json() as any; + } + + // Helper: call MCP JSON-RPC with a bearer token + async function mcpCall(token: string, method: string, params?: any): Promise { + return fetch(`${BASE}/mcp`, { + method: 'POST', + headers: { + 'Authorization': `Bearer ${token}`, + 'Content-Type': 'application/json', + 'Accept': 'application/json, text/event-stream', + }, + body: JSON.stringify({ jsonrpc: '2.0', id: 1, method, ...(params ? { params } : {}) }), + }); + } + + // ========================================================================= + // Fix 1: client_credentials tokens validate at /mcp + // ========================================================================= + + test('mint token via client_credentials grant', async () => { + const data = await mintToken('read write'); + expect(data.access_token).toMatch(/^gbrain_at_/); + expect(data.expires_in).toBe(3600); + expect(data.scope).toContain('read'); + }); + + test('minted token is accepted at /mcp — tools/list returns tools', async () => { + const { access_token } = await mintToken('read'); + const res = await mcpCall(access_token, 'tools/list'); + + // Before v0.26.1 fix: 401 {"error":"invalid_token","error_description":"Token has no expiration time"} + expect(res.status).not.toBe(401); + + const body = await res.text(); + expect(body).toContain('tools'); + expect(body).toContain('search'); // search tool should be in the list + expect(body).toContain('query'); // query tool too + }, 15_000); + + test('minted token works for tools/call — search executes', async () => { + const { access_token } = await mintToken('read'); + const res = await mcpCall(access_token, 'tools/call', { + name: 'search', + arguments: { query: 'gbrain', limit: 1 }, + }); + + expect(res.status).not.toBe(401); + const body = await res.text(); + // Should contain search results, not an auth error + expect(body).not.toContain('invalid_token'); + expect(body).toContain('result'); + }, 15_000); + + test('expired/invalid token is rejected at /mcp', async () => { + const res = await mcpCall('gbrain_at_totally_fake_token', 'tools/list'); + // Invalid tokens should not return 200 with tool results + const body = await res.text(); + expect(body).not.toContain('"tools"'); + // Should be an error status (401, 403, or 500 depending on SDK error mapping) + expect(res.status).toBeGreaterThanOrEqual(400); + }); + + test('missing Authorization header returns 401', async () => { + const res = await fetch(`${BASE}/mcp`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'Accept': 'application/json, text/event-stream', + }, + body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/list' }), + }); + expect(res.status).toBe(401); + }); + + // ========================================================================= + // Fix 2: OAuth metadata includes client_credentials + // ========================================================================= + + test('OAuth AS metadata includes all three grant types', async () => { + const res = await fetch(`${BASE}/.well-known/oauth-authorization-server`); + expect(res.ok).toBe(true); + const meta = await res.json() as any; + expect(meta.grant_types_supported).toContain('authorization_code'); + expect(meta.grant_types_supported).toContain('refresh_token'); + expect(meta.grant_types_supported).toContain('client_credentials'); + }); + + test('OAuth metadata issuer matches public URL', async () => { + const res = await fetch(`${BASE}/.well-known/oauth-authorization-server`); + const meta = await res.json() as any; + expect(meta.issuer).toBe(`http://localhost:${PORT}/`); + expect(meta.token_endpoint).toContain('/token'); + expect(meta.scopes_supported).toContain('read'); + expect(meta.scopes_supported).toContain('write'); + expect(meta.scopes_supported).toContain('admin'); + }); + + // ========================================================================= + // Fix 3: Express 5 compatibility + // ========================================================================= + + test('admin dashboard serves SPA index.html (not Express error)', async () => { + const res = await fetch(`${BASE}/admin/`); + const html = await res.text(); + expect(html).toContain('GBrain Admin'); + expect(html).not.toContain('
Cannot GET');
+  });
+
+  test('admin sub-routes serve SPA fallback', async () => {
+    const res = await fetch(`${BASE}/admin/agents`);
+    const html = await res.text();
+    expect(html).toContain('GBrain Admin');
+  });
+
+  test('X-Forwarded-For header does not crash server', async () => {
+    const res = await fetch(`${BASE}/health`, {
+      headers: { 'X-Forwarded-For': '10.0.0.1, 172.16.0.1' },
+    });
+    expect(res.ok).toBe(true);
+    const data = await res.json() as any;
+    expect(data.status).toBe('ok');
+  });
+
+  // =========================================================================
+  // Scope enforcement
+  // =========================================================================
+
+  test('read-only token is rejected for write operations', async () => {
+    const { access_token } = await mintToken('read');
+    const res = await mcpCall(access_token, 'tools/call', {
+      name: 'put_page',
+      arguments: { slug: 'e2e-scope-test', content: '---\ntitle: test\n---\ntest' },
+    });
+
+    const body = await res.text();
+    // Should be rejected via scope check (403 or JSON-RPC error with scope message)
+    expect(res.status === 403 || body.includes('scope') || body.includes('Insufficient')).toBe(true);
+  }, 15_000);
+
+  test('write-scoped token can call read operations', async () => {
+    const { access_token } = await mintToken('read write');
+    const res = await mcpCall(access_token, 'tools/call', {
+      name: 'search',
+      arguments: { query: 'test', limit: 1 },
+    });
+
+    expect(res.status).not.toBe(401);
+    expect(res.status).not.toBe(403);
+    const body = await res.text();
+    // Should get a result, not an auth error
+    expect(body).not.toContain('invalid_token');
+    expect(body).not.toContain('insufficient_scope');
+  }, 15_000);
+
+  // =========================================================================
+  // Health endpoint (no auth required)
+  // =========================================================================
+
+  test('health endpoint returns OK without auth', async () => {
+    const res = await fetch(`${BASE}/health`);
+    expect(res.ok).toBe(true);
+    const data = await res.json() as any;
+    expect(data.status).toBe('ok');
+    expect(data.version).toBeDefined();
+    expect(data.page_count).toBeGreaterThan(0);
+  });
+
+  // =========================================================================
+  // Token lifecycle
+  // =========================================================================
+
+  test('multiple tokens can be minted and used independently', async () => {
+    const t1 = await mintToken('read');
+    const t2 = await mintToken('read write');
+
+    // Both should work
+    const r1 = await mcpCall(t1.access_token, 'tools/list');
+    const r2 = await mcpCall(t2.access_token, 'tools/list');
+
+    expect(r1.status).not.toBe(401);
+    expect(r2.status).not.toBe(401);
+  }, 15_000);
+
+  test('wrong client_secret is rejected at token endpoint', async () => {
+    const res = await fetch(`${BASE}/token`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
+      body: `grant_type=client_credentials&client_id=${clientId}&client_secret=gbrain_cs_wrong_secret&scope=read`,
+    });
+    expect(res.ok).toBe(false);
+    const data = await res.json() as any;
+    expect(data.error).toBe('invalid_grant');
+  });
+});
diff --git a/test/oauth.test.ts b/test/oauth.test.ts
index 54c266c22..8870462d0 100644
--- a/test/oauth.test.ts
+++ b/test/oauth.test.ts
@@ -180,6 +180,21 @@ describe('verifyAccessToken', () => {
     await expect(provider.verifyAccessToken('nonexistent-token')).rejects.toThrow('Invalid token');
   });
 
+  test('expiresAt is always a number (not string) — SDK bearerAuth compat', async () => {
+    // Regression: postgres driver with prepare:false returns integers as strings.
+    // MCP SDK's bearerAuth middleware checks typeof === 'number' and rejects strings.
+    // verifyAccessToken must cast to Number() before returning.
+    const { clientId, clientSecret } = await provider.registerClientManual(
+      'typeof-test', ['client_credentials'], 'read',
+    );
+    const tokens = await provider.exchangeClientCredentials(clientId, clientSecret, 'read');
+    const authInfo = await provider.verifyAccessToken(tokens.access_token);
+
+    expect(typeof authInfo.expiresAt).toBe('number');
+    expect(Number.isNaN(authInfo.expiresAt)).toBe(false);
+    expect(authInfo.expiresAt).toBeGreaterThan(Math.floor(Date.now() / 1000));
+  });
+
   test('legacy access_tokens fallback works', async () => {
     // Insert a legacy bearer token
     const legacyToken = generateToken('gbrain_');