diff --git a/src/adapters/providers/discord.ts b/src/adapters/providers/discord.ts index 7372980..7e49acd 100644 --- a/src/adapters/providers/discord.ts +++ b/src/adapters/providers/discord.ts @@ -25,6 +25,7 @@ export const discordManifest: ProviderManifest = { // there is no scope list to ask for here. scopes: [], auth: { type: 'api_key', in: 'header', name: 'authorization', prefix: 'Bot ' }, + validate: { request: 'GET /users/@me' }, pagination: { // Discord sends no cursor back. The caller asks for what came before the // oldest id it holds, so the cursor is a message id read off the page. diff --git a/src/adapters/providers/manifest.ts b/src/adapters/providers/manifest.ts index c2a47db..820af57 100644 --- a/src/adapters/providers/manifest.ts +++ b/src/adapters/providers/manifest.ts @@ -141,6 +141,8 @@ export type ProviderManifest = { baseUrl: string scopes: string[] auth: AuthScheme + /** Optional request made before an api key is stored in the vault. */ + validate?: { request: `${Method} /${string}` } /** Merged under the headers the executor sets, never over them. */ headers?: Record pagination?: Pagination @@ -176,6 +178,54 @@ export function manifestProvider(manifest: ProviderManifest): ProviderAdapter { credential: manifest.auth.type === 'api_key' ? 'api_key' : 'oauth', listTools: () => manifest.tools.map(toolDef), + ...(manifest.validate + ? { + async validateKey(ctx: AdapterContext): Promise { + const space = manifest.validate!.request.indexOf(' ') + const method = manifest.validate!.request.slice(0, space) + const path = manifest.validate!.request.slice(space + 1) + let res: Response + try { + res = await followRedirects( + ctx.fetch, + fail, + manifest.auth, + withKeyInQuery(`${manifest.baseUrl}${path}`, manifest.auth, ctx.accessToken ?? ''), + { + method, + headers: { + ...manifest.headers, + ...authHeader(manifest.auth, ctx.accessToken ?? ''), + 'x-request-id': ctx.requestId, + }, + }, + ) + } catch { + throw fail('upstream_error', `${manifest.prefix} validation request failed`) + } + // invalid_arguments, not reauth_required: the key being refused + // arrived in this request, so it is a bad argument rather than a + // stored credential that went stale. + if (res.status === 401) { + throw fail('invalid_arguments', `${manifest.prefix} refused the api key`) + } + if (res.status === 403) { + // A bare 403 does not mean a bad key for every vendor, which is + // the whole reason errors.forbidden exists. + throw fail( + manifest.errors?.forbidden ?? 'invalid_arguments', + `${manifest.prefix} refused the api key`, + ) + } + if (!res.ok) { + throw fail('upstream_error', `${manifest.prefix} validation request failed`) + } + // Nothing reads this body, and an unread one holds the socket. + await res.body?.cancel() + }, + } + : {}), + async callTool(ctx: AdapterContext, name: string, rawArgs: unknown): Promise { const tool = byName.get(name) if (!tool) throw fail('tool_not_found', `${manifest.prefix} has no tool ${name}`) diff --git a/src/adapters/providers/registry.ts b/src/adapters/providers/registry.ts index b8f3aac..df2872f 100644 --- a/src/adapters/providers/registry.ts +++ b/src/adapters/providers/registry.ts @@ -53,6 +53,7 @@ export interface ProviderAdapter { */ credential?: 'oauth' | 'api_key' | 'none' listTools(): ToolDef[] + validateKey?(ctx: AdapterContext): Promise callTool(ctx: AdapterContext, tool: string, args: unknown): Promise mapError(err: unknown): GatewayError } diff --git a/src/application/connections.ts b/src/application/connections.ts index 410abd1..c75bdb2 100644 --- a/src/application/connections.ts +++ b/src/application/connections.ts @@ -21,6 +21,7 @@ export type ConnectionDeps = { states: StateStore grants: GrantStore enablement: EnablementStore + fetch?: typeof fetch exchange?: typeof exchangeCode } @@ -136,6 +137,15 @@ export async function setApiKey( const key = input.key.trim() if (!key) throw new GatewayError('invalid_arguments', 'api_key must not be empty') + if (adapter.validateKey) { + await adapter.validateKey({ + workspaceId: input.workspaceId, + requestId: `key-validation-${input.prefix}`, + accessToken: key, + fetch: deps.fetch ?? fetch, + }) + } + await deps.grants.save(input.workspaceId, adapter.grantId, { accessToken: key, refreshToken: null, diff --git a/test/connections.test.ts b/test/connections.test.ts index f1d76e1..115f51f 100644 --- a/test/connections.test.ts +++ b/test/connections.test.ts @@ -3,6 +3,7 @@ import { beginConnection, completeConnection, disconnect, + setApiKey, type ConnectionDeps, } from '../src/application/connections.ts' import { createRegistry, type ProviderAdapter } from '../src/adapters/providers/registry.ts' @@ -185,3 +186,25 @@ describe('a grant shared by several providers', () => { expect(new URL(url).searchParams.get('scope')).toBe('cal.read') }) }) + +describe('api key validation', () => { + it('does not save or enable a provider when validation refuses the key', async () => { + const base = fakeProvider() + const rejected: ProviderAdapter = { + ...base, + id: 'keyed', + prefix: 'keyed', + credential: 'api_key', + validateKey: async () => { + throw new Error('refused') + }, + } + const shared = deps({ registry: createRegistry([rejected]) }) + + await expect(setApiKey(shared, { workspaceId, prefix: 'keyed', key: 'wrong' })).rejects.toThrow( + 'refused', + ) + expect(await shared.grants.load(workspaceId, 'keyed')).toBeNull() + expect(await shared.enablement.enabledPrefixes(workspaceId)).not.toContain('keyed') + }) +}) diff --git a/test/helpers/server.ts b/test/helpers/server.ts index fcd3b82..737ee05 100644 --- a/test/helpers/server.ts +++ b/test/helpers/server.ts @@ -21,6 +21,13 @@ export const EPHEMERAL_FLOOR = 49152 let nextPort = 34000 +/** Keep validation probes in tests offline unless a test explicitly overrides fetch. */ +const offlineFetch = (async () => + new Response('{}', { + status: 200, + headers: { 'content-type': 'application/json' }, + })) as typeof fetch + /** * `listen(0)` draws from the same ephemeral range every other process on the * machine binds at random, so a suite that uses it occasionally addresses a @@ -88,6 +95,10 @@ export async function startTestServer( config: testConfig, registry: bootRegistry(), ...opts.overrides, + connectionOverrides: { + fetch: offlineFetch, + ...opts.overrides?.connectionOverrides, + }, }) const server = await listenBelowEphemeral(app) open.push(server) diff --git a/test/providers/manifest.test.ts b/test/providers/manifest.test.ts index fb3b0a7..a3e3fb8 100644 --- a/test/providers/manifest.test.ts +++ b/test/providers/manifest.test.ts @@ -64,6 +64,55 @@ function withTools(tools: ProviderManifest['tools'], over: Partial { + it('validates an api key without storing or exposing it', async () => { + const validator = withTools([], { validate: { request: 'GET /users/@me' }, auth: { type: 'api_key', in: 'header', name: 'authorization', prefix: 'Bot ' } }) + const upstream = fakeUpstream([{ match: /users\/@me/, body: { id: 'bot-1' } }]) + + await validator.validateKey?.({ ...ctx(upstream), accessToken: 'secret-token' }) + + expect(upstream.calls[0]?.url).toBe('https://api.demo.test/users/@me') + expect((upstream.calls[0]?.init?.headers as Record).authorization).toBe('Bot secret-token') + }) + + it('rejects an api key when its validation request fails', async () => { + const validator = withTools([], { validate: { request: 'GET /users/@me' } }) + const upstream = fakeUpstream([{ match: /users\/@me/, status: 401, body: { message: 'bad token' } }]) + + await expect(validator.validateKey?.({ ...ctx(upstream), accessToken: 'wrong' })).rejects.toMatchObject({ + code: 'invalid_arguments', + }) + }) + + it('puts query API keys on the validation request', async () => { + const validator = withTools([], { + validate: { request: 'GET /users/@me' }, + auth: { type: 'api_key', in: 'query', name: 'api_key' }, + }) + const upstream = fakeUpstream([{ match: /users\/@me\?api_key=secret-token/, body: { id: 'bot-1' } }]) + + await validator.validateKey?.({ ...ctx(upstream), accessToken: 'secret-token' }) + + expect(upstream.calls[0]?.url).toContain('api_key=secret-token') + }) + + it('reports validation service failures as upstream errors', async () => { + const validator = withTools([], { validate: { request: 'GET /users/@me' } }) + const upstream = fakeUpstream([{ match: /users\/@me/, status: 503, body: { message: 'busy' } }]) + + await expect(validator.validateKey?.({ ...ctx(upstream), accessToken: 'secret-token' })).rejects.toMatchObject({ + code: 'upstream_error', + }) + }) + + it('reports validation network failures as upstream errors', async () => { + const validator = withTools([], { validate: { request: 'GET /users/@me' } }) + const unreachable = { ...ctx(fakeUpstream([])), fetch: (async () => { throw new Error('offline') }) as typeof fetch } + + await expect(validator.validateKey?.({ ...unreachable, accessToken: 'secret-token' })).rejects.toMatchObject({ + code: 'upstream_error', + }) + }) + it('fills path placeholders and escapes every segment', async () => { const upstream = fakeUpstream([{ match: /boxes/, body: itemsPage(1) }]) await demo.callTool(ctx(upstream), 'list_things', { box: '../../admin' })