From 1f185692c02b606f379614e791fa5a695238a5e6 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Thu, 25 Jun 2026 02:01:14 +0800 Subject: [PATCH 1/2] fix(mcp): make remote /api/v1/mcp data plane work for external agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remote MCP HTTP surface (ADR-0036) was non-functional for external agents: every call 401'd with a valid key, standard MCP clients couldn't connect, and writes threw "ql.insert is not a function". All three are the same family — the MCP route resolves data services differently from REST. - resolveExecutionContext getQl: resolve objectql from the per-request kernel directly (kernel.getServiceAsync) instead of the scoped resolveService, which in the multi-env runtime hands back an instance blind to the env's sys_api_key rows so the api-key never resolved (→ 401). REST already resolves identity this way (rest-server), so REST + MCP no longer drift. - extractApiKey: accept `Authorization: Bearer osk_` (prefix-gated). Remote MCP clients (Claude Desktop / Cursor / Claude Code) send the key as a Bearer per the MCP spec; a better-auth session Bearer never carries the osk_ prefix, so the session path is unaffected. - callData create/update/delete: route through protocol.createData/updateData/ deleteData (mirroring the read paths and REST) instead of calling ql.insert on context.dataDriver, which in the multi-env runtime is a raw db driver with no ORM methods. Validated on a local isolated stack: tools/list 401->200 (X-API-Key AND Bearer), create_record succeeds, sys_* and no-auth stay fail-closed, REST unaffected. Unit: mcp 44/44, core api-key 10/10, runtime mcp/exec-ctx/keys 41/41. Co-Authored-By: Claude Opus 4.8 --- packages/core/src/security/api-key.test.ts | 14 ++++++- packages/core/src/security/api-key.ts | 21 +++++++--- packages/runtime/src/http-dispatcher.ts | 39 +++++++++++++++++-- .../resolve-execution-context.test.ts | 15 +++++-- 4 files changed, 74 insertions(+), 15 deletions(-) diff --git a/packages/core/src/security/api-key.test.ts b/packages/core/src/security/api-key.test.ts index 108bfc1dd5..2689e2a31f 100644 --- a/packages/core/src/security/api-key.test.ts +++ b/packages/core/src/security/api-key.test.ts @@ -39,10 +39,22 @@ describe('core api-key primitives', () => { expect(k.raw.startsWith(k.prefix)).toBe(true); }); - it('extractApiKey: x-api-key / ApiKey scheme, not Bearer', () => { + it('extractApiKey: x-api-key / ApiKey, and Bearer only for osk_-prefixed keys', () => { expect(extractApiKey({ 'x-api-key': 'k' })).toBe('k'); expect(extractApiKey({ authorization: 'ApiKey k' })).toBe('k'); + // A bare Bearer (e.g. a better-auth session token) is NOT an api-key. expect(extractApiKey({ authorization: 'Bearer k' })).toBeUndefined(); + // A Bearer carrying the api-key prefix IS accepted — remote MCP clients + // (Claude Desktop / Cursor / Claude Code) send the key this way. + expect(extractApiKey({ authorization: 'Bearer osk_abc123' })).toBe('osk_abc123'); + expect(extractApiKey({ authorization: 'bearer osk_abc123' })).toBe('osk_abc123'); // scheme is case-insensitive + }); + + it('resolveApiKeyPrincipal resolves a Bearer osk_ key (remote MCP client path)', async () => { + const raw = 'osk_' + 'b'.repeat(24); + const ql = makeQl([{ key: hashApiKey(raw), revoked: false, user_id: 'u1' }]); + const p = await resolveApiKeyPrincipal(ql, { authorization: `Bearer ${raw}` }); + expect(p?.userId).toBe('u1'); }); it('parseScopes + isExpired basics', () => { diff --git a/packages/core/src/security/api-key.ts b/packages/core/src/security/api-key.ts index 1b8255ebdb..6c2d20b8a7 100644 --- a/packages/core/src/security/api-key.ts +++ b/packages/core/src/security/api-key.ts @@ -69,18 +69,27 @@ export function generateApiKey(prefix: string = API_KEY_PREFIX): GeneratedApiKey } /** - * Extract an API key from request headers. Accepts `X-API-Key: ` or - * `Authorization: ApiKey ` (case-insensitive scheme). Bearer tokens are - * deliberately NOT treated as API keys — those flow through the session path. + * Extract an API key from request headers. Accepts, in order: + * - `X-API-Key: ` + * - `Authorization: ApiKey ` (case-insensitive scheme) + * - `Authorization: Bearer ` ONLY when `` carries the ObjectStack + * api-key prefix (`osk_`). Remote MCP clients (Claude Desktop / Cursor / + * Claude Code) authenticate to `/api/v1/mcp` with the key as a Bearer per the + * MCP spec, so rejecting Bearer outright made every standard MCP client fail. + * A better-auth *session* token never starts with `osk_`, so a session Bearer + * still falls through to the session path — this can't shadow it. */ export function extractApiKey(headers: any): string | undefined { const x = readHeader(headers, 'x-api-key'); if (x && x.trim()) return x.trim(); const auth = readHeader(headers, 'authorization'); if (!auth) return undefined; - const m = auth.match(/^ApiKey\s+(.+)$/i); - const token = m?.[1]?.trim(); - return token || undefined; + const apiKeyScheme = auth.match(/^ApiKey\s+(.+)$/i); + if (apiKeyScheme?.[1]?.trim()) return apiKeyScheme[1].trim(); + // Bearer is accepted only for prefixed api-keys (never for session tokens). + const bearer = auth.match(/^Bearer\s+(.+)$/i)?.[1]?.trim(); + if (bearer && bearer.startsWith(API_KEY_PREFIX)) return bearer; + return undefined; } /** Parse a `scopes` value that may be a JSON-string textarea or a real array. */ diff --git a/packages/runtime/src/http-dispatcher.ts b/packages/runtime/src/http-dispatcher.ts index 9ee43993bb..58a41f4c46 100644 --- a/packages/runtime/src/http-dispatcher.ts +++ b/packages/runtime/src/http-dispatcher.ts @@ -287,7 +287,16 @@ export class HttpDispatcher { }; if (action === 'create') { - if (ql) { + // Prefer the protocol service (validations + RLS + audit), mirroring + // the read paths below. The MCP bridge passes `context.dataDriver` as + // `ql`, which in the multi-env runtime is a RAW db driver with no ORM + // `insert` — so going straight to `ql.insert` broke MCP create_record + // ("ql.insert is not a function") while REST (which uses `createData`) + // worked. Routing writes through the protocol keeps them aligned. + if (protocol && typeof protocol.createData === 'function') { + return await protocol.createData({ object: params.object, data: params.data, ...(scopeId ? { environmentId: scopeId } : {}), context: executionContext }); + } + if (ql && typeof ql.insert === 'function') { const res = await ql.insert(params.object, params.data, qlOpts); const record = { ...params.data, ...res }; return { object: params.object, id: record.id, record }; @@ -310,7 +319,10 @@ export class HttpDispatcher { } if (action === 'update') { - if (ql && params.id) { + if (protocol && typeof protocol.updateData === 'function') { + return await protocol.updateData({ object: params.object, id: params.id, data: params.data, ...(scopeId ? { environmentId: scopeId } : {}), context: executionContext }); + } + if (ql && params.id && typeof ql.update === 'function') { let all = await ql.find(params.object, findOpts({ where: { id: params.id }, limit: 1 })); if (all && (all as any).value) all = (all as any).value; if (!all) all = []; @@ -323,7 +335,10 @@ export class HttpDispatcher { } if (action === 'delete') { - if (ql) { + if (protocol && typeof protocol.deleteData === 'function') { + return await protocol.deleteData({ object: params.object, id: params.id, ...(scopeId ? { environmentId: scopeId } : {}), context: executionContext }); + } + if (ql && typeof ql.delete === 'function') { await ql.delete(params.object, findOpts({ where: { id: params.id } })); return { object: params.object, id: params.id, deleted: true }; } @@ -3192,7 +3207,23 @@ export class HttpDispatcher { try { context.executionContext = await resolveExecutionContext({ getService: (n: string) => this.resolveService(n, context.environmentId), - getQl: () => Promise.resolve(this.getObjectQLService(context.environmentId)), + // Resolve ObjectQL from the per-request kernel DIRECTLY. The scoped + // `resolveService('objectql', envId)` factory can return a different + // instance that doesn't see THIS env's rows (the gotcha + // `handleActions` works around) — which made the api-key lookup miss + // `sys_api_key` on the MCP path and reject valid keys with 401, while + // REST accepted them (rest-server resolves identity via + // `kernel.getServiceAsync('objectql')`). Resolving off `this.kernel` + // keeps REST + MCP identity resolution aligned; falls back to the + // scoped path when the kernel can't hand back an objectql directly. + getQl: async () => { + const k: any = this.kernel; + if (k && typeof k.getServiceAsync === 'function') { + const ql = await k.getServiceAsync('objectql').catch(() => undefined); + if (ql && (ql.registry || typeof ql.find === 'function')) return ql; + } + return this.getObjectQLService(context.environmentId); + }, request: context.request, }); } catch { diff --git a/packages/runtime/src/security/resolve-execution-context.test.ts b/packages/runtime/src/security/resolve-execution-context.test.ts index 381ef4c32a..ffcd98280e 100644 --- a/packages/runtime/src/security/resolve-execution-context.test.ts +++ b/packages/runtime/src/security/resolve-execution-context.test.ts @@ -131,14 +131,21 @@ describe('resolveExecutionContext — API key verify path', () => { expect(ctx.permissions).toEqual([]); }); - it('ignores Bearer tokens on the API-key path (no key resolution)', async () => { + it('resolves a Bearer api-key (osk_ prefix) but not a bare session Bearer', async () => { const raw = 'osk_valid'; const rows = [{ id: 'k1', key: hashApiKey(raw), revoked: false, user_id: 'u1' }]; - // Bearer is a session token, not an API key — must not resolve here. - const ctx = await resolveExecutionContext( + // A Bearer carrying the osk_ api-key prefix DOES resolve — remote MCP clients + // (Claude Desktop / Cursor / Claude Code) send the key as a Bearer. + const keyed = await resolveExecutionContext( makeOpts(rows, { authorization: `Bearer ${raw}` }), ); - expect(ctx.userId).toBeUndefined(); + expect(keyed.userId).toBe('u1'); + // A bare Bearer (a better-auth session token) is NOT an api-key — no key + // resolution here; it falls through to the session path. + const session = await resolveExecutionContext( + makeOpts(rows, { authorization: 'Bearer some-session-token' }), + ); + expect(session.userId).toBeUndefined(); }); }); From 59a0e3cdc4888b3fe168d86a8796154e5acd8f19 Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Thu, 25 Jun 2026 02:10:23 +0800 Subject: [PATCH 2/2] harden api-key scheme regexes against polynomial backtracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL flagged `^(ApiKey|Bearer)\s+(.+)$` as polynomial-ReDoS: `\s+` and `.+` both match whitespace, so an all-whitespace tail backtracks O(n²). Anchor the capture to a non-whitespace first char (`\s+(\S.*)`) — linear, same behaviour for every valid header. Clears the pre-existing ApiKey alert too. Co-Authored-By: Claude Opus 4.8 --- packages/core/src/security/api-key.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/security/api-key.ts b/packages/core/src/security/api-key.ts index 6c2d20b8a7..fa0f327eed 100644 --- a/packages/core/src/security/api-key.ts +++ b/packages/core/src/security/api-key.ts @@ -84,10 +84,10 @@ export function extractApiKey(headers: any): string | undefined { if (x && x.trim()) return x.trim(); const auth = readHeader(headers, 'authorization'); if (!auth) return undefined; - const apiKeyScheme = auth.match(/^ApiKey\s+(.+)$/i); + const apiKeyScheme = auth.match(/^ApiKey\s+(\S.*)$/i); if (apiKeyScheme?.[1]?.trim()) return apiKeyScheme[1].trim(); // Bearer is accepted only for prefixed api-keys (never for session tokens). - const bearer = auth.match(/^Bearer\s+(.+)$/i)?.[1]?.trim(); + const bearer = auth.match(/^Bearer\s+(\S.*)$/i)?.[1]?.trim(); if (bearer && bearer.startsWith(API_KEY_PREFIX)) return bearer; return undefined; }