diff --git a/src/main/cli/providerModelAdminRoutes.ts b/src/main/cli/providerModelAdminRoutes.ts index 853ace6d7..d0945e55e 100644 --- a/src/main/cli/providerModelAdminRoutes.ts +++ b/src/main/cli/providerModelAdminRoutes.ts @@ -1,6 +1,5 @@ import { randomUUID } from 'node:crypto' import { - PublicModelConfigSchema, PublicProviderSummarySchema, modelsGetPublicConfigRoute, modelsSetPublicConfigRoute, @@ -15,7 +14,12 @@ import type { LLM_PROVIDER } from '@shared/types/provider' import type { ProviderRuntime } from '@/provider' import type { ProviderQueryScheduler } from '@/provider/providerService' import type { ProviderSettingsPort } from '@/provider/settings' -import { createRouteMap, type DeepchatRouteMap, type RouteCaller } from '@/routes/routeRegistry' +import { + createRouteMap, + projectJsonRouteOutput, + type DeepchatRouteMap, + type RouteCaller +} from '@/routes/routeRegistry' import { CliRequestError } from './errors' type PublicProviderSettings = Pick< @@ -257,10 +261,8 @@ export function createCliProviderModelAdminRoutes( requireCliCaller(context.caller) const input = modelsGetPublicConfigRoute.input.parse(rawInput) requireModel(input.providerId, input.modelId) - return modelsGetPublicConfigRoute.output.parse({ - config: PublicModelConfigSchema.parse( - dependencies.providerSettings.getModelConfig(input.modelId, input.providerId) - ) + return projectJsonRouteOutput(modelsGetPublicConfigRoute.output, { + config: dependencies.providerSettings.getModelConfig(input.modelId, input.providerId) }) } ], @@ -277,10 +279,9 @@ export function createCliProviderModelAdminRoutes( input.config ) ) - const config = PublicModelConfigSchema.parse( - dependencies.providerSettings.getModelConfig(input.modelId, input.providerId) - ) - return modelsSetPublicConfigRoute.output.parse({ config }) + return projectJsonRouteOutput(modelsSetPublicConfigRoute.output, { + config: dependencies.providerSettings.getModelConfig(input.modelId, input.providerId) + }) } ] ]) diff --git a/src/main/cli/server.ts b/src/main/cli/server.ts index 475e9de78..0c8d49158 100644 --- a/src/main/cli/server.ts +++ b/src/main/cli/server.ts @@ -873,17 +873,25 @@ export class CliServer { rawOutput: unknown, routeMethod: string ): JsonValue { - const parsedOutput = entry.contract.output.safeParse(rawOutput) - const parsedResult = parsedOutput.success - ? JsonValueSchema.safeParse(parsedOutput.data) - : { success: false as const } - if (!parsedOutput.success || !parsedResult.success) { - this.log.error('[CLI] Route returned invalid output', { method: routeMethod }) + const fail = (stage: 'route' | 'json', error: z.ZodError): never => { + this.log.error('[CLI] Route returned invalid output', { + method: routeMethod, + stage, + issueCount: error.issues.length, + issueCodes: Array.from(new Set(error.issues.slice(0, 16).map((issue) => issue.code))) + }) throw new CliRequestError('internal_error', 'Route returned an invalid result', { httpStatus: 500 }) } - return parsedResult.data as JsonValue + + const parsedOutput = entry.contract.output.safeParse(rawOutput) + if (!parsedOutput.success) return fail('route', parsedOutput.error) + + const parsedResult = JsonValueSchema.safeParse(parsedOutput.data) + if (!parsedResult.success) return fail('json', parsedResult.error) + + return parsedResult.data } private async dispatchStreamResponse( diff --git a/src/main/provider/routes.ts b/src/main/provider/routes.ts index 7a437277d..1f49cba05 100644 --- a/src/main/provider/routes.ts +++ b/src/main/provider/routes.ts @@ -66,6 +66,7 @@ import { } from '@shared/contracts/routes' import { createRouteMap, + projectJsonRouteOutput, requireRendererCaller, type DeepchatRouteMap } from '@/routes/routeRegistry' @@ -519,7 +520,7 @@ export function createProviderRoutes(deps: { modelsListRuntimeRoute.name, async (rawInput) => { const input = modelsListRuntimeRoute.input.parse(rawInput) - return modelsListRuntimeRoute.output.parse({ + return projectJsonRouteOutput(modelsListRuntimeRoute.output, { models: await providerRuntime.getModelList(input.providerId) }) } diff --git a/src/main/routes/routeRegistry.ts b/src/main/routes/routeRegistry.ts index 495dec014..0891d84ed 100644 --- a/src/main/routes/routeRegistry.ts +++ b/src/main/routes/routeRegistry.ts @@ -1,5 +1,6 @@ import type { DeepchatRouteName } from '@shared/contracts/routes' import type { LocalControlScope } from '@shared/contracts/localControl' +import type { z } from 'zod' export type RendererRouteCaller = Readonly<{ kind: 'renderer' @@ -68,6 +69,19 @@ export type DeepchatRouteHandler = (rawInput: unknown, context: RouteContext) => export type DeepchatRouteMap = ReadonlyMap +export function projectJsonRouteOutput( + outputSchema: OutputSchema, + rawOutput: unknown +): z.output { + // Project through the public contract before serialization can observe unknown/private fields. + const publicOutput = outputSchema.parse(rawOutput) + const serializedOutput = JSON.stringify(publicOutput) + if (serializedOutput === undefined) { + throw new TypeError('Route output cannot be represented as JSON') + } + return outputSchema.parse(JSON.parse(serializedOutput)) +} + export function createRouteMap( entries: ReadonlyArray ): DeepchatRouteMap { diff --git a/test/main/cli/providerModelAdminRoutes.test.ts b/test/main/cli/providerModelAdminRoutes.test.ts index 6531e8c5f..d31000060 100644 --- a/test/main/cli/providerModelAdminRoutes.test.ts +++ b/test/main/cli/providerModelAdminRoutes.test.ts @@ -7,6 +7,7 @@ import { providersTestPublicConnectionRoute, providersUpdatePublicRoute } from '@shared/contracts/routes' +import { JsonValueSchema } from '@shared/contracts/json' import type { LLM_PROVIDER, ModelConfig } from '@shared/types/provider' import { createCliProviderModelAdminRoutes } from '@/cli/providerModelAdminRoutes' import type { CliRouteCaller, RouteContext } from '@/routes/routeRegistry' @@ -255,7 +256,7 @@ describe('CLI provider administration routes', () => { ).toBe(true) }) - it('uses strict public model config input and strips main-owned identity fields', async () => { + it('returns sparse public model configs as redacted JSON values', async () => { const provider: LLM_PROVIDER = { id: 'provider-1', name: 'Provider', @@ -291,17 +292,62 @@ describe('CLI provider administration routes', () => { }).success ).toBe(false) - harness.modelConfigs.set(`${provider.id}:model-1`, { + const sparseStoredConfig = { ...config, + temperature: undefined, + topP: undefined, + imageGeneration: { + size: undefined, + quality: 'high' + }, + videoGeneration: { + seconds: undefined, + watermark: false, + inputReference: { data: 'reference-image', mimeType: undefined }, + references: [ + { + type: 'image', + url: 'https://example.com/reference.png', + data: undefined, + mimeType: undefined + } + ] + }, + tts: { voice: undefined, speed: 1 }, conversationId: 'private-session', - ownedBy: 'internal-owner' - } as ModelConfig) + ownedBy: 'internal-owner', + futureSecret: 'secret' + } as ModelConfig & { futureSecret: string } + const publicSparseConfig = { + ...config, + imageGeneration: { quality: 'high' as const }, + videoGeneration: { + watermark: false, + inputReference: { data: 'reference-image' }, + references: [{ type: 'image' as const, url: 'https://example.com/reference.png' }] + }, + tts: { speed: 1 } + } + harness.modelConfigs.set(`${provider.id}:model-1`, sparseStoredConfig) const result = await harness.invoke(modelsGetPublicConfigRoute.name, { providerId: provider.id, modelId: 'model-1' }) - expect(result).toEqual({ config }) + expect(result).toEqual({ config: publicSparseConfig }) + expect(JsonValueSchema.safeParse(result).success).toBe(true) expect(JSON.stringify(result)).not.toContain('private-session') + expect(JSON.stringify(result)).not.toContain('futureSecret') + + harness.setModelConfig.mockImplementationOnce((modelId, providerId) => { + harness.modelConfigs.set(`${providerId}:${modelId}`, sparseStoredConfig) + }) + const setResult = await harness.invoke(modelsSetPublicConfigRoute.name, { + providerId: provider.id, + modelId: 'model-1', + config + }) + expect(setResult).toEqual({ config: publicSparseConfig }) + expect(JsonValueSchema.safeParse(setResult).success).toBe(true) }) it('normalizes mutation storage failures without exposing their details', async () => { diff --git a/test/main/cli/server.test.ts b/test/main/cli/server.test.ts index 03bbec94a..86981587c 100644 --- a/test/main/cli/server.test.ts +++ b/test/main/cli/server.test.ts @@ -3,8 +3,10 @@ import { request as httpRequest } from 'node:http' import { mkdtemp, readFile, readdir, rm, stat } from 'node:fs/promises' import os from 'node:os' import path from 'node:path' +import { z } from 'zod' import { afterEach, describe, expect, it, vi } from 'vitest' import { cliVersionRoute, type DeepchatRouteName } from '@shared/contracts/routes' +import { defineRouteContract } from '@shared/contracts/contract' import type { JsonValue } from '@shared/contracts/json' import { LOCAL_CONTROL_PROTOCOL_VERSION, @@ -277,6 +279,7 @@ async function createTestServer( dispatch: ReturnType dispatchUpload: ReturnType authorize: ReturnType + log: Readonly<{ warn: ReturnType; error: ReturnType }> }> { const userDataPath = await createTemporaryDirectory() let server: CliServer @@ -295,6 +298,7 @@ async function createTestServer( ) const dispatchUpload = vi.fn(options.dispatchUpload ?? (async () => ({}))) const authorize = vi.fn(options.authorize ?? (async () => ({ release: () => undefined }))) + const log = { warn: vi.fn(), error: vi.fn() } server = new CliServer({ userDataPath, appVersion: '1.2.3', @@ -326,11 +330,11 @@ async function createTestServer( dispatchUpload, ...(options.authorize ? { authorize } : {}), surface: options.surface, - log: { warn: vi.fn(), error: vi.fn() } + log }) servers.push(server) const descriptor = await server.start() - return { userDataPath, server, descriptor, dispatch, dispatchUpload, authorize } + return { userDataPath, server, descriptor, dispatch, dispatchUpload, authorize, log } } afterEach(async () => { @@ -506,6 +510,55 @@ describe('CLI local transport', () => { }) }) + it('rejects route output that passes its contract but is not a JSON value', async () => { + const sentinel = 'secret-route-key' + const contract = defineRouteContract({ + name: 'test.optionalRecord', + input: z.object({}).default({}), + output: z.object({ values: z.record(z.string(), z.string().optional()) }) + }) + const output = { values: { [sentinel]: undefined } } + const surface = new Map([ + [ + contract.name, + { + contract, + effect: 'read', + callers: ['human'], + scopes: ['system:read'], + transport: 'rpc', + approval: 'never', + limits: { maxBodyBytes: 1024, timeoutMs: 5_000 } + } + ] + ]) + expect(contract.output.safeParse(output).success).toBe(true) + const { descriptor, log } = await createTestServer({ + surface, + dispatchOutput: () => output + }) + + const response = await rpcRequest(descriptor, { + method: contract.name, + params: {} + }) + + expect(response).toMatchObject({ + status: 500, + body: { ok: false, error: { code: 'internal_error' } } + }) + expect(log.error).toHaveBeenCalledWith( + '[CLI] Route returned invalid output', + expect.objectContaining({ + method: contract.name, + stage: 'json', + issueCount: expect.any(Number), + issueCodes: expect.any(Array) + }) + ) + expect(JSON.stringify(log.error.mock.calls)).not.toContain(sentinel) + }) + it('releases policy admission after a route contract failure', async () => { const release = vi.fn() const { descriptor, authorize } = await createTestServer({ diff --git a/test/main/provider/routes.test.ts b/test/main/provider/routes.test.ts index a419256fa..39932393a 100644 --- a/test/main/provider/routes.test.ts +++ b/test/main/provider/routes.test.ts @@ -4,6 +4,7 @@ import { createProviderRoutes } from '@/provider/routes' import { modelsGetCapabilitiesRoute, modelsGetProviderCatalogRoute, + modelsListRuntimeRoute, modelsSetStatusRoute, providersImportApplyRoute, providersImportScanRoute, @@ -11,6 +12,7 @@ import { providersRemoveRoute, providersUpdateRoute } from '@shared/contracts/routes' +import { JsonValueSchema } from '@shared/contracts/json' import { ModelType } from '@shared/model' const context = createRendererRouteContext(42, 7) @@ -81,6 +83,101 @@ describe('Provider routes', () => { expect(updateModelStatus).not.toHaveBeenCalled() }) + it('returns JSON-safe runtime models across providers', async () => { + const getModelList = vi.fn(async (providerId: string) => { + if (providerId === 'empty-provider') return [] + if (providerId === 'custom-provider') { + return [ + { + id: 'custom-model', + name: 'Custom model', + group: 'custom', + providerId, + enabled: false, + isCustom: true, + vision: false, + functionCall: true, + reasoning: false, + enableSearch: true, + type: ModelType.Chat, + contextLength: 128_000, + maxTokens: 16_384, + description: 'User-defined model', + supportedEndpointTypes: ['openai-response', 'anthropic'], + selectableEndpointTypes: ['openai-response'], + endpointType: 'openai-response', + ownedBy: 'model-owner', + internalCredential: 'must-not-leak' + } + ] + } + return [ + { + id: 'builtin-model', + name: 'Built-in model', + group: 'default', + providerId, + enabled: undefined, + isCustom: undefined, + vision: undefined, + functionCall: undefined, + reasoning: undefined, + enableSearch: undefined, + contextLength: undefined, + maxTokens: undefined, + description: undefined + } + ] + }) + const routes = createRoutes({ providerSettings: {}, providerRuntime: { getModelList } }) + const invoke = async (providerId: string) => + await routes.get(modelsListRuntimeRoute.name)?.({ providerId }, context) + + const builtinResult = await invoke('builtin-provider') + const customResult = await invoke('custom-provider') + const emptyResult = await invoke('empty-provider') + + expect(builtinResult).toEqual({ + models: [ + { + id: 'builtin-model', + name: 'Built-in model', + group: 'default', + providerId: 'builtin-provider' + } + ] + }) + expect(customResult).toEqual({ + models: [ + { + id: 'custom-model', + name: 'Custom model', + group: 'custom', + providerId: 'custom-provider', + enabled: false, + isCustom: true, + vision: false, + functionCall: true, + reasoning: false, + enableSearch: true, + type: ModelType.Chat, + contextLength: 128_000, + maxTokens: 16_384, + description: 'User-defined model', + supportedEndpointTypes: ['openai-response', 'anthropic'], + selectableEndpointTypes: ['openai-response'], + endpointType: 'openai-response', + ownedBy: 'model-owner' + } + ] + }) + expect(emptyResult).toEqual({ models: [] }) + for (const result of [builtinResult, customResult, emptyResult]) { + expect(JsonValueSchema.safeParse(result).success).toBe(true) + expect(JSON.stringify(result)).not.toContain('must-not-leak') + } + }) + it('returns one authoritative capability snapshot and forwards draft route metadata', async () => { const snapshot = { identity: { diff --git a/test/main/routes/routeRegistry.test.ts b/test/main/routes/routeRegistry.test.ts index 62fb62efe..9e4f0c82b 100644 --- a/test/main/routes/routeRegistry.test.ts +++ b/test/main/routes/routeRegistry.test.ts @@ -1,7 +1,9 @@ import { describe, expect, it } from 'vitest' +import { z } from 'zod' import { createRendererRouteCaller, createRendererRouteContext, + projectJsonRouteOutput, requireRendererCaller, type RouteContext } from '@/routes/routeRegistry' @@ -30,6 +32,28 @@ describe('route caller context', () => { expect(requireRendererCaller(context)).toBe(context.caller) }) + it('projects route outputs before normalizing them for JSON', () => { + const outputSchema = z.object({ + required: z.string(), + optional: z.string().optional(), + nested: z.object({ optional: z.boolean().optional() }) + }) + const privateValue = { + toJSON: () => { + throw new Error('Private values must be removed before serialization') + } + } + + expect( + projectJsonRouteOutput(outputSchema, { + required: 'visible', + optional: undefined, + nested: { optional: undefined, privateValue }, + privateValue + }) + ).toEqual({ required: 'visible', nested: {} }) + }) + it.each([ { caller: {