mirror of
https://github.com/thedotmack/claude-mem
synced 2026-04-25 17:15:04 +02:00
fix: coerce corpus route filters (#1776)
* fix: coerce corpus route filters * test: cover unsupported corpus type filters
This commit is contained in:
@@ -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
|
||||
|
||||
174
tests/worker/http/routes/corpus-routes-coercion.test.ts
Normal file
174
tests/worker/http/routes/corpus-routes-coercion.test.ts
Normal file
@@ -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<Request>;
|
||||
res: Partial<Response>;
|
||||
jsonSpy: ReturnType<typeof mock>;
|
||||
statusSpy: ReturnType<typeof mock>;
|
||||
} {
|
||||
const jsonSpy = mock(() => {});
|
||||
const statusSpy = mock(() => ({ json: jsonSpy }));
|
||||
return {
|
||||
req: { body, path: '/api/corpus', params: {}, query: {} } as Partial<Request>,
|
||||
res: { json: jsonSpy, status: statusSpy, headersSent: false } as unknown as Partial<Response>,
|
||||
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<void> {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
}
|
||||
|
||||
describe('CorpusRoutes Type Coercion', () => {
|
||||
let handler: (req: Request, res: Response) => void;
|
||||
let mockBuild: ReturnType<typeof mock>;
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user