diff --git a/completions/mcpctl.bash b/completions/mcpctl.bash index a889844..ed1b7e1 100644 --- a/completions/mcpctl.bash +++ b/completions/mcpctl.bash @@ -185,7 +185,7 @@ _mcpctl() { COMPREPLY=($(compgen -W "--data --force -h --help" -- "$cur")) ;; llm) - COMPREPLY=($(compgen -W "--type --model --url --tier --description --api-key-ref --extra --force -h --help" -- "$cur")) + COMPREPLY=($(compgen -W "--type --model --url --tier --description --api-key-ref --extra --force --skip-auth-check -h --help" -- "$cur")) ;; agent) COMPREPLY=($(compgen -W "--llm --project --description --system-prompt --system-prompt-file --proxy-model --default-temperature --default-top-p --default-top-k --default-max-tokens --default-seed --default-stop --default-extra --default-params-file --force -h --help" -- "$cur")) diff --git a/completions/mcpctl.fish b/completions/mcpctl.fish index ce00dbc..e02363f 100644 --- a/completions/mcpctl.fish +++ b/completions/mcpctl.fish @@ -331,6 +331,7 @@ complete -c mcpctl -n "__mcpctl_subcmd_active create llm" -l description -d 'Des complete -c mcpctl -n "__mcpctl_subcmd_active create llm" -l api-key-ref -d 'API key reference in SECRET/KEY form (e.g. anthropic-key/token)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create llm" -l extra -d 'Extra config key=value (repeat)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create llm" -l force -d 'Update if already exists' +complete -c mcpctl -n "__mcpctl_subcmd_active create llm" -l skip-auth-check -d 'Skip the upstream auth probe (for offline registration before infra exists)' # create agent options complete -c mcpctl -n "__mcpctl_subcmd_active create agent" -l llm -d 'Pinned Llm (see `mcpctl get llms`)' -x diff --git a/src/cli/src/commands/create.ts b/src/cli/src/commands/create.ts index 2a92112..5e5122a 100644 --- a/src/cli/src/commands/create.ts +++ b/src/cli/src/commands/create.ts @@ -264,6 +264,7 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { .option('--api-key-ref ', 'API key reference in SECRET/KEY form (e.g. anthropic-key/token)') .option('--extra ', 'Extra config key=value (repeat)', collect, []) .option('--force', 'Update if already exists') + .option('--skip-auth-check', 'Skip the upstream auth probe (for offline registration before infra exists)') .action(async (name: string, opts) => { const body: Record = { name, @@ -290,6 +291,11 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { } body.extraConfig = extra; } + // _skipAuthCheck is a side-channel field consumed (and stripped) by the + // mcpd route — it never makes it into the Llm row. mcpd defaults to + // running an auth probe at create/update time so wrong tokens fail fast + // with a 422 instead of silently 502'ing on first chat. + if (opts.skipAuthCheck === true) body._skipAuthCheck = true; try { const row = await client.post<{ id: string; name: string }>('/api/v1/llms', body); diff --git a/src/cli/src/index.ts b/src/cli/src/index.ts index a01ac82..6a0485f 100644 --- a/src/cli/src/index.ts +++ b/src/cli/src/index.ts @@ -40,14 +40,31 @@ export function createProgram(): Command { program.addCommand(createLoginCommand()); program.addCommand(createLogoutCommand()); - // Resolve target URL: --direct goes to mcpd, default goes to mcplocal + // Resolve target URL: --direct goes to mcpd, default goes to mcplocal. + // + // Commander's `program.opts()` returns the default values until + // `program.parse(argv)` runs — but commands (and ApiClient) need the + // resolved baseUrl at construction time. The chicken-and-egg meant + // `--direct` was previously a no-op for ApiClient: every command went to + // mcplocal regardless. Some commands accidentally still worked because + // mcplocal forwards plain `/api/v1/*` to mcpd, but flows that need direct + // SSE streaming (e.g. `mcpctl chat`) went to mcplocal:3200, which doesn't + // route them. + // + // Fix: peek at process.argv directly for the two global flags we need + // before Commander's full parse runs. const config = loadConfig(); const creds = loadCredentials(); - const opts = program.opts(); + const argv = process.argv; + const directFlag = argv.includes('--direct'); + const daemonUrlIdx = argv.indexOf('--daemon-url'); + const daemonUrlVal = daemonUrlIdx > -1 && daemonUrlIdx + 1 < argv.length + ? argv[daemonUrlIdx + 1] + : undefined; let baseUrl: string; - if (opts.daemonUrl) { - baseUrl = opts.daemonUrl as string; - } else if (opts.direct) { + if (daemonUrlVal !== undefined) { + baseUrl = daemonUrlVal; + } else if (directFlag) { baseUrl = config.mcpdUrl; } else { baseUrl = config.mcplocalUrl; diff --git a/src/mcpd/src/main.ts b/src/mcpd/src/main.ts index 499cf8b..4e9845c 100644 --- a/src/mcpd/src/main.ts +++ b/src/mcpd/src/main.ts @@ -415,8 +415,17 @@ async function main(): Promise { backends: secretBackendService, rotator: secretBackendRotator, }); - const llmService = new LlmService(llmRepo, secretService); const llmAdapters = new LlmAdapterRegistry(); + // LlmService takes the adapter registry so create()/update() can run an + // auth probe at registration time. Keeps registration honest: misconfigured + // tokens or wrong URLs surface as a 422 at create, not as a "fetch failed" + // 502 at first chat. Logger forwards inconclusive probes (network down, + // proxy doesn't expose /v1/models) to mcpd's structured log so operators + // can still see them without blocking registration. + const llmService = new LlmService(llmRepo, secretService, { + adapters: llmAdapters, + log: { warn: (msg) => app.log.warn(msg) }, + }); // AgentService + ChatService get fully wired below once projectService and // mcpProxyService are constructed (ChatService needs them via the // ChatToolDispatcher bridge). diff --git a/src/mcpd/src/routes/llms.ts b/src/mcpd/src/routes/llms.ts index 3e0bf79..7d34571 100644 --- a/src/mcpd/src/routes/llms.ts +++ b/src/mcpd/src/routes/llms.ts @@ -1,5 +1,6 @@ import type { FastifyInstance } from 'fastify'; import type { LlmService } from '../services/llm.service.js'; +import { LlmAuthVerificationError } from '../services/llm.service.js'; import { NotFoundError, ConflictError } from '../services/mcp-server.service.js'; export function registerLlmRoutes( @@ -31,7 +32,13 @@ export function registerLlmRoutes( app.post('/api/v1/llms', async (request, reply) => { try { - const row = await service.create(request.body); + // Body field `_skipAuthCheck`: opt-out for offline registration (e.g. + // wiring config before the upstream is reachable). Stripped from the + // body before validation. + const body = (request.body ?? {}) as Record; + const skipAuthCheck = body['_skipAuthCheck'] === true; + delete body['_skipAuthCheck']; + const row = await service.create(body, { skipAuthCheck }); reply.code(201); return row; } catch (err) { @@ -39,18 +46,29 @@ export function registerLlmRoutes( reply.code(409); return { error: err.message }; } + if (err instanceof LlmAuthVerificationError) { + reply.code(422); + return { error: err.message, status: err.status }; + } throw err; } }); app.put<{ Params: { id: string } }>('/api/v1/llms/:id', async (request, reply) => { try { - return await service.update(request.params.id, request.body); + const body = (request.body ?? {}) as Record; + const skipAuthCheck = body['_skipAuthCheck'] === true; + delete body['_skipAuthCheck']; + return await service.update(request.params.id, body, { skipAuthCheck }); } catch (err) { if (err instanceof NotFoundError) { reply.code(404); return { error: err.message }; } + if (err instanceof LlmAuthVerificationError) { + reply.code(422); + return { error: err.message, status: err.status }; + } throw err; } }); diff --git a/src/mcpd/src/services/llm.service.ts b/src/mcpd/src/services/llm.service.ts index 3a92410..6dd3932 100644 --- a/src/mcpd/src/services/llm.service.ts +++ b/src/mcpd/src/services/llm.service.ts @@ -13,6 +13,8 @@ import type { Llm } from '@prisma/client'; import type { ILlmRepository } from '../repositories/llm.repository.js'; import type { SecretService } from './secret.service.js'; +import type { LlmAdapterRegistry } from './llm/dispatcher.js'; +import type { InferContext } from './llm/types.js'; import { CreateLlmSchema, UpdateLlmSchema, @@ -21,6 +23,22 @@ import { } from '../validation/llm.schema.js'; import { NotFoundError, ConflictError } from './mcp-server.service.js'; +/** Dependencies for auth verification at create/update time. */ +export interface LlmServiceDeps { + /** Adapter registry to run the auth probe. Optional in tests / bootstrap. */ + adapters?: LlmAdapterRegistry; + /** Logger for unreachable/unexpected probe outcomes. */ + log?: { warn: (msg: string) => void }; +} + +/** Thrown when the auth probe fails decisively (401/403 from upstream). */ +export class LlmAuthVerificationError extends Error { + constructor(public readonly status: number, public readonly body: string, message: string) { + super(message); + this.name = 'LlmAuthVerificationError'; + } +} + /** Shape returned by API layer — merges DB row with a human-readable apiKeyRef. */ export interface LlmView { id: string; @@ -41,6 +59,7 @@ export class LlmService { constructor( private readonly repo: ILlmRepository, private readonly secrets: SecretService, + private readonly verifyDeps: LlmServiceDeps = {}, ) {} async list(): Promise { @@ -60,12 +79,29 @@ export class LlmService { return this.toView(row); } - async create(input: unknown): Promise { + async create(input: unknown, opts: { skipAuthCheck?: boolean } = {}): Promise { const data = CreateLlmSchema.parse(input); const existing = await this.repo.findByName(data.name); if (existing !== null) throw new ConflictError(`Llm already exists: ${data.name}`); const apiKeyFields = await this.resolveApiKeyRefToIds(data.apiKeyRef); + + // Auth probe: catch wrong tokens / wrong URLs at registration time, not + // at first chat. Skipped when there's no key (probe would be meaningless) + // or the caller explicitly opted out (e.g. wiring config before infra + // exists). The probe is also skipped when no adapters registry was + // injected — keeps tests + bootstrap simple. + if (!opts.skipAuthCheck && apiKeyFields.id !== null && this.verifyDeps.adapters !== undefined) { + await this.runAuthProbe({ + name: data.name, + type: data.type, + model: data.model, + url: data.url ?? '', + apiKeyRef: data.apiKeyRef ?? null, + extraConfig: data.extraConfig, + }); + } + const row = await this.repo.create({ name: data.name, type: data.type, @@ -80,9 +116,9 @@ export class LlmService { return this.toView(row); } - async update(id: string, input: unknown): Promise { + async update(id: string, input: unknown, opts: { skipAuthCheck?: boolean } = {}): Promise { const data = UpdateLlmSchema.parse(input); - await this.getById(id); + const before = await this.getById(id); const updateFields: Parameters[1] = {}; if (data.model !== undefined) updateFields.model = data.model; @@ -103,10 +139,93 @@ export class LlmService { } } + // Auth probe runs whenever any field that affects auth (apiKeyRef OR url) + // is changing, OR whenever the caller asks via skipAuthCheck=false. The + // probe uses the post-update view (new key + new url + same type/model). + const authAffectingChange = data.apiKeyRef !== undefined || data.url !== undefined; + const willHaveKey = data.apiKeyRef === null + ? false + : data.apiKeyRef !== undefined || before.apiKeyRef !== null; + if (authAffectingChange && !opts.skipAuthCheck && willHaveKey && this.verifyDeps.adapters !== undefined) { + await this.runAuthProbe({ + name: before.name, + type: before.type, + model: data.model ?? before.model, + url: data.url ?? before.url, + apiKeyRef: data.apiKeyRef === undefined ? before.apiKeyRef : data.apiKeyRef, + extraConfig: data.extraConfig ?? before.extraConfig, + }); + } + const row = await this.repo.update(id, updateFields); return this.toView(row); } + /** + * Run a cheap auth probe against the upstream provider. Throws + * `LlmAuthVerificationError` on a definitive auth failure (401/403). + * Logs and swallows transient network/unexpected errors — those are not + * fatal at registration time. + */ + private async runAuthProbe(snap: { + name: string; + type: string; + model: string; + url: string; + apiKeyRef: ApiKeyRef | null; + extraConfig: Record; + }): Promise { + if (snap.apiKeyRef === null) return; + if (this.verifyDeps.adapters === undefined) return; + let apiKey: string; + try { + const secret = await this.secrets.getByName(snap.apiKeyRef.name); + const data = await this.secrets.resolveData(secret); + const v = data[snap.apiKeyRef.key]; + if (v === undefined || v === '') { + throw new LlmAuthVerificationError(0, '', `Llm '${snap.name}' apiKeyRef points at empty secret data`); + } + apiKey = v; + } catch (err) { + if (err instanceof LlmAuthVerificationError) throw err; + // Secret resolution failure — bail with a clean error rather than + // letting it bubble as a generic 500. + throw new LlmAuthVerificationError(0, '', `Llm '${snap.name}' apiKeyRef could not be resolved: ${(err as Error).message}`); + } + let adapter; + try { + adapter = this.verifyDeps.adapters.get(snap.type); + } catch (err) { + // Provider type unsupported by the registry — that's a config error, + // surface it now. + throw new LlmAuthVerificationError(0, '', `Llm '${snap.name}' type '${snap.type}' has no adapter: ${(err as Error).message}`); + } + const ctx: InferContext = { + body: { model: snap.model, messages: [] }, + modelOverride: snap.model, + apiKey, + url: snap.url, + extraConfig: snap.extraConfig, + }; + const result = await adapter.verifyAuth(ctx); + if (result.ok) return; + if (result.reason === 'auth') { + throw new LlmAuthVerificationError( + result.status, + result.body, + `Llm '${snap.name}' auth check failed: ${snap.url || '(default URL)'} returned HTTP ${String(result.status)}. ` + + `Body: ${result.body.slice(0, 400)}`, + ); + } + // unreachable / unexpected — warn but allow registration. The user might + // be wiring config before the upstream is reachable, or hitting a + // proxy that doesn't expose /v1/models. + const reason = result.reason === 'unreachable' ? `unreachable (${result.error})` : `HTTP ${String(result.status)} (${result.body.slice(0, 200)})`; + this.verifyDeps.log?.warn( + `Llm '${snap.name}': auth probe inconclusive — ${reason}. Registration succeeded; first inference call will surface any real issue.`, + ); + } + async delete(id: string): Promise { await this.getById(id); await this.repo.delete(id); diff --git a/src/mcpd/src/services/llm/adapters/anthropic.ts b/src/mcpd/src/services/llm/adapters/anthropic.ts index 18c2fef..4f2f185 100644 --- a/src/mcpd/src/services/llm/adapters/anthropic.ts +++ b/src/mcpd/src/services/llm/adapters/anthropic.ts @@ -23,6 +23,7 @@ import type { StreamingChunk, AdapterDeps, OpenAiMessage, + VerifyAuthResult, } from '../types.js'; const DEFAULT_ANTHROPIC_URL = 'https://api.anthropic.com'; @@ -146,6 +147,40 @@ export class AnthropicAdapter implements LlmAdapter { yield { data: '[DONE]', done: true }; } + /** + * Anthropic doesn't expose a list-models or auth-only endpoint, so probe + * with the cheapest possible /v1/messages call (1 max_token, "ping" + * prompt). The point is to exercise the auth header, not to generate. + * Auth failures here are 401 with `{"type":"authentication_error"}` — + * caught and surfaced. Network failures bubble up as `unreachable`. + */ + async verifyAuth(ctx: InferContext): Promise { + const url = (ctx.url !== '' ? ctx.url : DEFAULT_ANTHROPIC_URL).replace(/\/+$/, ''); + let res: Response; + try { + res = await this.fetchImpl(`${url}/v1/messages`, { + method: 'POST', + headers: this.headers(ctx), + body: JSON.stringify({ + model: ctx.body.model !== '' ? ctx.body.model : ctx.modelOverride, + max_tokens: 1, + messages: [{ role: 'user', content: 'ping' }], + }), + }); + } catch (err) { + return { ok: false, reason: 'unreachable', error: (err as Error).message }; + } + if (res.ok) return { ok: true }; + const body = await res.text().catch(() => ''); + if (res.status === 401 || res.status === 403) { + return { ok: false, reason: 'auth', status: res.status, body }; + } + // 400s on a bad model name are still proof the auth worked. Report + // those as `unexpected` (warn) rather than `auth` (fail) so the user + // can register the Llm with a typo'd model and fix it later. + return { ok: false, reason: 'unexpected', status: res.status, body }; + } + private headers(ctx: InferContext): Record { return { 'Content-Type': 'application/json', diff --git a/src/mcpd/src/services/llm/adapters/openai-passthrough.ts b/src/mcpd/src/services/llm/adapters/openai-passthrough.ts index 8d9c2dd..ddad8e2 100644 --- a/src/mcpd/src/services/llm/adapters/openai-passthrough.ts +++ b/src/mcpd/src/services/llm/adapters/openai-passthrough.ts @@ -11,7 +11,7 @@ * - deepseek → https://api.deepseek.com * - vllm/ollama → must be configured; these have no canonical public URL. */ -import type { LlmAdapter, InferContext, NonStreamingResult, StreamingChunk, AdapterDeps } from '../types.js'; +import type { LlmAdapter, InferContext, NonStreamingResult, StreamingChunk, AdapterDeps, VerifyAuthResult } from '../types.js'; const DEFAULT_URLS: Record = { openai: 'https://api.openai.com', @@ -88,6 +88,40 @@ export class OpenAiPassthroughAdapter implements LlmAdapter { yield { data: '[DONE]', done: true }; } + /** + * Probe `GET /v1/models` with the configured auth header. OpenAI, + * vLLM, LiteLLM, DeepSeek, Ollama (in openai-compat mode) all expose this + * endpoint and it's gated by the same auth as chat/completions. Cheap (no + * generation), idempotent, and the response shape is a stable + * `{ data: [...] }` array. + */ + async verifyAuth(ctx: InferContext): Promise { + let url: string; + try { + url = this.endpointUrl(ctx.url); + } catch (err) { + return { ok: false, reason: 'unexpected', status: 0, body: (err as Error).message }; + } + let res: Response; + try { + res = await this.fetchImpl(`${url}/v1/models`, { + method: 'GET', + headers: this.headers(ctx), + }); + } catch (err) { + return { ok: false, reason: 'unreachable', error: (err as Error).message }; + } + if (res.ok) return { ok: true }; + const body = await res.text().catch(() => ''); + if (res.status === 401 || res.status === 403) { + return { ok: false, reason: 'auth', status: res.status, body }; + } + // Some providers don't expose /v1/models (e.g. a stripped LiteLLM proxy). + // 404 + non-OAI providers shouldn't hard-block registration — caller + // treats `unexpected` as a warning, not a failure. + return { ok: false, reason: 'unexpected', status: res.status, body }; + } + private endpointUrl(url: string): string { if (url !== '') return url.replace(/\/+$/, ''); const def = DEFAULT_URLS[this.kind]; diff --git a/src/mcpd/src/services/llm/types.ts b/src/mcpd/src/services/llm/types.ts index 2e78be7..fd75694 100644 --- a/src/mcpd/src/services/llm/types.ts +++ b/src/mcpd/src/services/llm/types.ts @@ -63,8 +63,30 @@ export interface LlmAdapter { * provider-native stream formats into OpenAI `chat.completion.chunk`s. */ stream(ctx: InferContext): AsyncGenerator; + /** + * Cheap auth probe used at Llm create/update time. Should pick the cheapest + * upstream call that exercises the auth header — typically a list-models + * endpoint or a 1-token messages call. + * + * Returns one of: + * - { ok: true } — auth succeeded + * - { ok: false, reason: 'auth', status, body } — upstream said no (401/403) + * - { ok: false, reason: 'unreachable', error } — network/DNS/timeout + * - { ok: false, reason: 'unexpected', status, body } — couldn't tell + * + * Callers (LlmService.create/update) throw on `auth`, warn-only on + * `unreachable`, and warn-only on `unexpected`. The point is to fail fast on + * provably wrong credentials at registration time. + */ + verifyAuth(ctx: InferContext): Promise; } +export type VerifyAuthResult = + | { ok: true } + | { ok: false; reason: 'auth'; status: number; body: string } + | { ok: false; reason: 'unreachable'; error: string } + | { ok: false; reason: 'unexpected'; status: number; body: string }; + export interface AdapterDeps { fetch?: typeof globalThis.fetch; } diff --git a/src/mcpd/tests/llm-adapters.test.ts b/src/mcpd/tests/llm-adapters.test.ts index e80991b..045ac88 100644 --- a/src/mcpd/tests/llm-adapters.test.ts +++ b/src/mcpd/tests/llm-adapters.test.ts @@ -208,3 +208,102 @@ describe('LlmAdapterRegistry', () => { expect(() => reg.get('bogus')).toThrow(UnsupportedProviderError); }); }); + +describe('verifyAuth — registration-time probe', () => { + it('OpenAI passthrough: 200 from /v1/models → ok', async () => { + const fetchImpl = mockFetch([ + { match: /\/v1\/models$/, status: 200, body: { data: [{ id: 'gpt-4o-mini' }] } }, + ]); + const adapter = new OpenAiPassthroughAdapter('openai', { fetch: fetchImpl as unknown as typeof fetch }); + const result = await adapter.verifyAuth(makeCtx({ url: 'http://lite:4000', apiKey: 'sk-good' })); + expect(result).toEqual({ ok: true }); + expect(fetchImpl).toHaveBeenCalledWith('http://lite:4000/v1/models', expect.objectContaining({ method: 'GET' })); + const callInit = fetchImpl.mock.calls[0][1] as RequestInit; + expect((callInit.headers as Record)['Authorization']).toBe('Bearer sk-good'); + }); + + it('OpenAI passthrough: 401 → reason=auth (caller throws)', async () => { + const fetchImpl = mockFetch([ + { match: /\/v1\/models$/, status: 401, text: '{"error":"invalid_api_key"}' }, + ]); + const adapter = new OpenAiPassthroughAdapter('openai', { fetch: fetchImpl as unknown as typeof fetch }); + const result = await adapter.verifyAuth(makeCtx({ url: 'http://lite:4000', apiKey: 'sk-bad' })); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe('auth'); + if (result.reason === 'auth') { + expect(result.status).toBe(401); + expect(result.body).toContain('invalid_api_key'); + } + } + }); + + it('OpenAI passthrough: 403 → reason=auth', async () => { + const fetchImpl = mockFetch([ + { match: /\/v1\/models$/, status: 403, text: 'forbidden' }, + ]); + const adapter = new OpenAiPassthroughAdapter('openai', { fetch: fetchImpl as unknown as typeof fetch }); + const result = await adapter.verifyAuth(makeCtx({ url: 'http://lite:4000', apiKey: 'k' })); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe('auth'); + }); + + it('OpenAI passthrough: 404 (proxy without /v1/models) → reason=unexpected (warn-only)', async () => { + const fetchImpl = mockFetch([ + { match: /\/v1\/models$/, status: 404, text: 'not found' }, + ]); + const adapter = new OpenAiPassthroughAdapter('openai', { fetch: fetchImpl as unknown as typeof fetch }); + const result = await adapter.verifyAuth(makeCtx({ url: 'http://lite:4000', apiKey: 'k' })); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe('unexpected'); + }); + + it('OpenAI passthrough: network error → reason=unreachable (warn-only)', async () => { + const fetchImpl = vi.fn(async () => { throw new Error('ECONNREFUSED 127.0.0.1:9999'); }); + const adapter = new OpenAiPassthroughAdapter('openai', { fetch: fetchImpl as unknown as typeof fetch }); + const result = await adapter.verifyAuth(makeCtx({ url: 'http://localhost:9999', apiKey: 'k' })); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe('unreachable'); + if (result.reason === 'unreachable') { + expect(result.error).toContain('ECONNREFUSED'); + } + } + }); + + it('Anthropic: 200 from /v1/messages probe → ok', async () => { + const fetchImpl = mockFetch([ + { match: /\/v1\/messages$/, status: 200, body: { id: 'msg_x', content: [{ type: 'text', text: 'pong' }] } }, + ]); + const adapter = new AnthropicAdapter({ fetch: fetchImpl as unknown as typeof fetch }); + const result = await adapter.verifyAuth(makeCtx({ url: 'https://api.anthropic.com', apiKey: 'sk-ant-good' })); + expect(result.ok).toBe(true); + const callInit = fetchImpl.mock.calls[0][1] as RequestInit; + expect((callInit.headers as Record)['x-api-key']).toBe('sk-ant-good'); + const reqBody = JSON.parse(callInit.body as string) as { max_tokens: number }; + expect(reqBody.max_tokens).toBe(1); + }); + + it('Anthropic: 401 → reason=auth', async () => { + const fetchImpl = mockFetch([ + { match: /\/v1\/messages$/, status: 401, text: '{"type":"authentication_error"}' }, + ]); + const adapter = new AnthropicAdapter({ fetch: fetchImpl as unknown as typeof fetch }); + const result = await adapter.verifyAuth(makeCtx({ apiKey: 'bad' })); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe('auth'); + }); + + it('Anthropic: 400 (typo\'d model) → reason=unexpected, NOT auth', async () => { + // Auth was fine; the request was rejected for a different reason. We + // don't want to block registration on bad model names — that error + // surfaces at chat time when the user actually picks a model. + const fetchImpl = mockFetch([ + { match: /\/v1\/messages$/, status: 400, text: '{"error":"model not found"}' }, + ]); + const adapter = new AnthropicAdapter({ fetch: fetchImpl as unknown as typeof fetch }); + const result = await adapter.verifyAuth(makeCtx({ apiKey: 'sk-ant-x', modelOverride: 'claude-fake' })); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.reason).toBe('unexpected'); + }); +}); diff --git a/src/mcpd/tests/llm-service.test.ts b/src/mcpd/tests/llm-service.test.ts index 78af3e3..5bfc983 100644 --- a/src/mcpd/tests/llm-service.test.ts +++ b/src/mcpd/tests/llm-service.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from 'vitest'; -import { LlmService } from '../src/services/llm.service.js'; +import { LlmService, LlmAuthVerificationError } from '../src/services/llm.service.js'; import type { ILlmRepository } from '../src/repositories/llm.repository.js'; import type { Llm, Secret } from '@prisma/client'; @@ -229,4 +229,125 @@ describe('LlmService', () => { name: 'x', type: 'openai', model: 'gpt-4', tier: 'warp-speed', })).rejects.toThrow(); }); + + // ── Auth verification at registration time ──────────────────────────── + // Catches misconfigured tokens / wrong URLs at create/update, not at + // first chat. The actual upstream-probe logic lives in each adapter's + // verifyAuth(); these tests exercise the service's reaction to the + // probe result. + + it('create: throws LlmAuthVerificationError when adapter probe returns reason=auth', async () => { + const repo = mockRepo(); + const sec = makeSecret({ id: 'sec-bad', name: 'bad-key' }); + const secrets = mockSecrets({ 'bad-key': sec }, { token: 'sk-bad' }); + const adapters = { + get: vi.fn(() => ({ + kind: 'openai', + verifyAuth: vi.fn(async () => ({ ok: false, reason: 'auth', status: 401, body: '{"error":"invalid_api_key"}' })), + })), + } as unknown as Parameters[2]['adapters']; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const svc = new LlmService(repo, secrets as any, { adapters }); + await expect(svc.create({ + name: 'wrong-key', type: 'openai', model: 'gpt-4o', + apiKeyRef: { name: 'bad-key', key: 'token' }, + })).rejects.toThrow(LlmAuthVerificationError); + // Repo.create should NOT have been called — no row written. + expect(repo.create).not.toHaveBeenCalled(); + }); + + it('create: warn-only when probe returns reason=unreachable (still creates row)', async () => { + const repo = mockRepo(); + const sec = makeSecret({ id: 'sec-x', name: 'k' }); + const secrets = mockSecrets({ k: sec }, { token: 'k' }); + const log = { warn: vi.fn() }; + const adapters = { + get: vi.fn(() => ({ + kind: 'openai', + verifyAuth: vi.fn(async () => ({ ok: false, reason: 'unreachable', error: 'ECONNREFUSED' })), + })), + } as unknown as Parameters[2]['adapters']; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const svc = new LlmService(repo, secrets as any, { adapters, log }); + const view = await svc.create({ + name: 'offline', type: 'openai', model: 'gpt-4o', + url: 'http://localhost:9999', + apiKeyRef: { name: 'k', key: 'token' }, + }); + expect(view.name).toBe('offline'); + expect(repo.create).toHaveBeenCalledOnce(); + expect(log.warn).toHaveBeenCalledWith(expect.stringContaining('unreachable')); + }); + + it('create: warn-only when probe returns reason=unexpected (404 from a stripped proxy)', async () => { + const repo = mockRepo(); + const sec = makeSecret({ id: 'sec-x', name: 'k' }); + const secrets = mockSecrets({ k: sec }, { token: 'k' }); + const log = { warn: vi.fn() }; + const adapters = { + get: vi.fn(() => ({ + kind: 'openai', + verifyAuth: vi.fn(async () => ({ ok: false, reason: 'unexpected', status: 404, body: 'not found' })), + })), + } as unknown as Parameters[2]['adapters']; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const svc = new LlmService(repo, secrets as any, { adapters, log }); + const view = await svc.create({ + name: 'stripped-proxy', type: 'openai', model: 'gpt-4o', + apiKeyRef: { name: 'k', key: 'token' }, + }); + expect(view.name).toBe('stripped-proxy'); + expect(log.warn).toHaveBeenCalledWith(expect.stringContaining('HTTP 404')); + }); + + it('create: skipAuthCheck=true bypasses the probe', async () => { + const repo = mockRepo(); + const sec = makeSecret({ id: 'sec-x', name: 'k' }); + const secrets = mockSecrets({ k: sec }, { token: 'k' }); + const verifyAuth = vi.fn(async () => ({ ok: false, reason: 'auth', status: 401, body: 'no' })); + const adapters = { + get: vi.fn(() => ({ kind: 'openai', verifyAuth })), + } as unknown as Parameters[2]['adapters']; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const svc = new LlmService(repo, secrets as any, { adapters }); + const view = await svc.create({ + name: 'offline-staging', type: 'openai', model: 'gpt-4o', + apiKeyRef: { name: 'k', key: 'token' }, + }, { skipAuthCheck: true }); + expect(view.name).toBe('offline-staging'); + expect(verifyAuth).not.toHaveBeenCalled(); + }); + + it('create: probe is skipped when no apiKeyRef (nothing to verify)', async () => { + const repo = mockRepo(); + const verifyAuth = vi.fn(); + const adapters = { + get: vi.fn(() => ({ kind: 'openai', verifyAuth })), + } as unknown as Parameters[2]['adapters']; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const svc = new LlmService(repo, mockSecrets({}) as any, { adapters }); + await svc.create({ name: 'no-key', type: 'ollama', model: 'llama3', url: 'http://localhost:11434' }); + expect(verifyAuth).not.toHaveBeenCalled(); + }); + + it('update: probes only when apiKeyRef or url changes', async () => { + const existing = makeLlm({ id: 'llm-up', name: 'up', apiKeySecretId: 'sec-x', apiKeySecretKey: 'token' }); + const repo = mockRepo([existing]); + const sec = makeSecret({ id: 'sec-x', name: 'k' }); + const secrets = mockSecrets({ k: sec }, { token: 'k' }); + const verifyAuth = vi.fn(async () => ({ ok: true })); + const adapters = { + get: vi.fn(() => ({ kind: 'openai', verifyAuth })), + } as unknown as Parameters[2]['adapters']; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const svc = new LlmService(repo, secrets as any, { adapters }); + + // Description-only update — no probe. + await svc.update('llm-up', { description: 'new' }); + expect(verifyAuth).not.toHaveBeenCalled(); + + // URL change — probe runs. + await svc.update('llm-up', { url: 'http://new-host:4000' }); + expect(verifyAuth).toHaveBeenCalledOnce(); + }); });