From eeb68410337d955a20780201b2f7b6d2f1407c00 Mon Sep 17 00:00:00 2001 From: suyua9 <1521777066@qq.com> Date: Wed, 15 Apr 2026 15:58:01 +0800 Subject: [PATCH] fix: coerce corpus route filters (#1776) * fix: coerce corpus route filters * test: cover unsupported corpus type filters --- .../worker/http/routes/CorpusRoutes.ts | 62 ++++++- .../routes/corpus-routes-coercion.test.ts | 174 ++++++++++++++++++ 2 files changed, 232 insertions(+), 4 deletions(-) create mode 100644 tests/worker/http/routes/corpus-routes-coercion.test.ts diff --git a/src/services/worker/http/routes/CorpusRoutes.ts b/src/services/worker/http/routes/CorpusRoutes.ts index 928a06ea..5ddfa188 100644 --- a/src/services/worker/http/routes/CorpusRoutes.ts +++ b/src/services/worker/http/routes/CorpusRoutes.ts @@ -12,6 +12,8 @@ import { CorpusBuilder } from '../../knowledge/CorpusBuilder.js'; import { KnowledgeAgent } from '../../knowledge/KnowledgeAgent.js'; import type { CorpusFilter } from '../../knowledge/types.js'; +const ALLOWED_CORPUS_TYPES = new Set(['decision', 'bugfix', 'feature', 'refactor', 'discovery', 'change']); + export class CorpusRoutes extends BaseRouteHandler { constructor( private corpusStore: CorpusStore, @@ -49,15 +51,31 @@ export class CorpusRoutes extends BaseRouteHandler { const { name, description, project, types, concepts, files, query, date_start, date_end, limit } = req.body; + const coercedTypes = this.coerceStringArray(types, 'types', res); + if (coercedTypes === null) return; + if (coercedTypes && !coercedTypes.every(type => ALLOWED_CORPUS_TYPES.has(type))) { + this.badRequest(res, 'types must contain valid observation types'); + return; + } + + const coercedConcepts = this.coerceStringArray(concepts, 'concepts', res); + if (coercedConcepts === null) return; + + const coercedFiles = this.coerceStringArray(files, 'files', res); + if (coercedFiles === null) return; + + const coercedLimit = this.coercePositiveInteger(limit, 'limit', res); + if (coercedLimit === null) return; + const filter: CorpusFilter = {}; if (project) filter.project = project; - if (types) filter.types = types; - if (concepts) filter.concepts = concepts; - if (files) filter.files = files; + if (coercedTypes && coercedTypes.length > 0) filter.types = coercedTypes as CorpusFilter['types']; + if (coercedConcepts && coercedConcepts.length > 0) filter.concepts = coercedConcepts; + if (coercedFiles && coercedFiles.length > 0) filter.files = coercedFiles; if (query) filter.query = query; if (date_start) filter.date_start = date_start; if (date_end) filter.date_end = date_end; - if (limit) filter.limit = limit; + if (coercedLimit !== undefined) filter.limit = coercedLimit; const corpus = await this.corpusBuilder.build(name, description || '', filter); @@ -66,6 +84,42 @@ export class CorpusRoutes extends BaseRouteHandler { res.json(metadata); }); + private coerceStringArray(value: unknown, fieldName: string, res: Response): string[] | null | undefined { + if (value === undefined || value === null || value === '') { + return undefined; + } + + let parsed = value; + if (typeof value === 'string') { + try { + parsed = JSON.parse(value); + } catch { + parsed = value.split(',').map(part => part.trim()).filter(Boolean); + } + } + + if (!Array.isArray(parsed) || !parsed.every(item => typeof item === 'string')) { + this.badRequest(res, `${fieldName} must be an array of strings`); + return null; + } + + return parsed.map(item => item.trim()).filter(Boolean); + } + + private coercePositiveInteger(value: unknown, fieldName: string, res: Response): number | null | undefined { + if (value === undefined || value === null || value === '') { + return undefined; + } + + const parsed = typeof value === 'string' ? Number(value) : value; + if (typeof parsed !== 'number' || !Number.isInteger(parsed) || parsed <= 0) { + this.badRequest(res, `${fieldName} must be a positive integer`); + return null; + } + + return parsed; + } + /** * List all corpora with stats * GET /api/corpus diff --git a/tests/worker/http/routes/corpus-routes-coercion.test.ts b/tests/worker/http/routes/corpus-routes-coercion.test.ts new file mode 100644 index 00000000..e6315617 --- /dev/null +++ b/tests/worker/http/routes/corpus-routes-coercion.test.ts @@ -0,0 +1,174 @@ +/** + * CorpusRoutes Type Coercion Tests + * + * Tests that MCP/HTTP clients sending string-encoded corpus filters are coerced + * before CorpusBuilder assumes array and number fields. + */ + +import { describe, it, expect, mock, beforeEach } from 'bun:test'; +import type { Request, Response } from 'express'; +import { CorpusRoutes } from '../../../../src/services/worker/http/routes/CorpusRoutes.js'; + +function createMockReqRes(body: any): { + req: Partial; + res: Partial; + jsonSpy: ReturnType; + statusSpy: ReturnType; +} { + const jsonSpy = mock(() => {}); + const statusSpy = mock(() => ({ json: jsonSpy })); + return { + req: { body, path: '/api/corpus', params: {}, query: {} } as Partial, + res: { json: jsonSpy, status: statusSpy, headersSent: false } as unknown as Partial, + jsonSpy, + statusSpy, + }; +} + +function createCorpus(name: string, filter: any) { + return { + version: 1 as const, + name, + description: '', + created_at: '2026-04-14T00:00:00.000Z', + updated_at: '2026-04-14T00:00:00.000Z', + filter, + stats: { + observation_count: 0, + token_estimate: 0, + date_range: { earliest: '', latest: '' }, + type_breakdown: {}, + }, + system_prompt: '', + session_id: null, + observations: [], + }; +} + +async function flushPromises(): Promise { + await Promise.resolve(); + await Promise.resolve(); +} + +describe('CorpusRoutes Type Coercion', () => { + let handler: (req: Request, res: Response) => void; + let mockBuild: ReturnType; + + beforeEach(() => { + mockBuild = mock((name: string, description: string, filter: any) => Promise.resolve(createCorpus(name, filter))); + + const routes = new CorpusRoutes( + { list: mock(() => []), read: mock(() => null), delete: mock(() => false) } as any, + { build: mockBuild } as any, + {} as any + ); + + const mockApp = { + post: mock((path: string, fn: any) => { + if (path === '/api/corpus') handler = fn; + }), + get: mock(() => {}), + delete: mock(() => {}), + }; + + routes.setupRoutes(mockApp as any); + }); + + it('accepts native array filters and numeric limit', async () => { + const { req, res, jsonSpy } = createMockReqRes({ + name: 'native', + types: ['decision', 'bugfix'], + concepts: ['hooks'], + files: ['src/a.ts'], + limit: 10, + }); + + handler(req as Request, res as Response); + await flushPromises(); + + expect(mockBuild).toHaveBeenCalledWith('native', '', { + types: ['decision', 'bugfix'], + concepts: ['hooks'], + files: ['src/a.ts'], + limit: 10, + }); + expect(jsonSpy).toHaveBeenCalled(); + }); + + it('coerces JSON-encoded string filters and string limit', async () => { + const { req, res } = createMockReqRes({ + name: 'json-strings', + types: '["decision","bugfix"]', + concepts: '["hooks","agent"]', + files: '["src/a.ts","src/b.ts"]', + limit: '25', + }); + + handler(req as Request, res as Response); + await flushPromises(); + + expect(mockBuild).toHaveBeenCalledWith('json-strings', '', { + types: ['decision', 'bugfix'], + concepts: ['hooks', 'agent'], + files: ['src/a.ts', 'src/b.ts'], + limit: 25, + }); + }); + + it('coerces comma-separated filters and trims whitespace', async () => { + const { req, res } = createMockReqRes({ + name: 'comma-strings', + types: 'decision, bugfix', + concepts: 'hooks, agent', + files: 'src/a.ts, src/b.ts', + }); + + handler(req as Request, res as Response); + await flushPromises(); + + expect(mockBuild).toHaveBeenCalledWith('comma-strings', '', { + types: ['decision', 'bugfix'], + concepts: ['hooks', 'agent'], + files: ['src/a.ts', 'src/b.ts'], + }); + }); + + it('rejects invalid array items before calling CorpusBuilder', async () => { + const { req, res, statusSpy } = createMockReqRes({ + name: 'bad-array', + concepts: ['hooks', 42], + }); + + handler(req as Request, res as Response); + await flushPromises(); + + expect(statusSpy).toHaveBeenCalledWith(400); + expect(mockBuild).not.toHaveBeenCalled(); + }); + + it('rejects unsupported corpus types before calling CorpusBuilder', async () => { + const { req, res, statusSpy } = createMockReqRes({ + name: 'bad-type', + types: ['typo'], + }); + + handler(req as Request, res as Response); + await flushPromises(); + + expect(statusSpy).toHaveBeenCalledWith(400); + expect(mockBuild).not.toHaveBeenCalled(); + }); + + it('rejects invalid limit before calling CorpusBuilder', async () => { + const { req, res, statusSpy } = createMockReqRes({ + name: 'bad-limit', + limit: 'many', + }); + + handler(req as Request, res as Response); + await flushPromises(); + + expect(statusSpy).toHaveBeenCalledWith(400); + expect(mockBuild).not.toHaveBeenCalled(); + }); +});