feat: eager vLLM warmup and smart page titles in paginate stage
- Add warmup() to LlmProvider interface for eager subprocess startup - ManagedVllmProvider.warmup() starts vLLM in background on project load - ProviderRegistry.warmupAll() triggers all managed providers - NamedProvider proxies warmup() to inner provider - paginate stage generates LLM-powered descriptive page titles when available, cached by content hash, falls back to generic "Page N" - project-mcp-endpoint calls warmupAll() on router creation so vLLM is loading while the session initializes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Michal
178
src/mcpd/tests/audit-event-routes.test.ts
Normal file
178
src/mcpd/tests/audit-event-routes.test.ts
Normal file
@@ -0,0 +1,178 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import Fastify from 'fastify';
|
||||
import type { FastifyInstance } from 'fastify';
|
||||
import { registerAuditEventRoutes } from '../src/routes/audit-events.js';
|
||||
import { AuditEventService } from '../src/services/audit-event.service.js';
|
||||
import { errorHandler } from '../src/middleware/error-handler.js';
|
||||
import type { IAuditEventRepository, AuditEventFilter } from '../src/repositories/interfaces.js';
|
||||
|
||||
function mockRepo(): IAuditEventRepository {
|
||||
return {
|
||||
findAll: vi.fn(async () => []),
|
||||
findById: vi.fn(async () => null),
|
||||
createMany: vi.fn(async (events: unknown[]) => events.length),
|
||||
count: vi.fn(async () => 0),
|
||||
};
|
||||
}
|
||||
|
||||
function makeEvent(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
id: 'evt-1',
|
||||
timestamp: new Date('2026-03-01T12:00:00Z'),
|
||||
sessionId: 'sess-1',
|
||||
projectName: 'ha-project',
|
||||
eventKind: 'gate_decision',
|
||||
source: 'mcplocal',
|
||||
verified: false,
|
||||
serverName: null,
|
||||
correlationId: null,
|
||||
parentEventId: null,
|
||||
payload: { trigger: 'begin_session' },
|
||||
createdAt: new Date(),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('audit event routes', () => {
|
||||
let app: FastifyInstance;
|
||||
let repo: ReturnType<typeof mockRepo>;
|
||||
let service: AuditEventService;
|
||||
|
||||
beforeEach(async () => {
|
||||
app = Fastify();
|
||||
app.setErrorHandler(errorHandler);
|
||||
repo = mockRepo();
|
||||
service = new AuditEventService(repo);
|
||||
registerAuditEventRoutes(app, service);
|
||||
await app.ready();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await app.close();
|
||||
});
|
||||
|
||||
describe('POST /api/v1/audit/events', () => {
|
||||
it('inserts batch of events', async () => {
|
||||
const events = [
|
||||
{ timestamp: '2026-03-01T12:00:00Z', sessionId: 's1', projectName: 'p1', eventKind: 'gate_decision', source: 'mcplocal', verified: false, payload: {} },
|
||||
{ timestamp: '2026-03-01T12:00:01Z', sessionId: 's1', projectName: 'p1', eventKind: 'stage_execution', source: 'mcplocal', verified: true, payload: {} },
|
||||
{ timestamp: '2026-03-01T12:00:02Z', sessionId: 's1', projectName: 'p1', eventKind: 'pipeline_execution', source: 'mcplocal', verified: true, payload: {} },
|
||||
];
|
||||
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/audit/events',
|
||||
payload: events,
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(201);
|
||||
expect(JSON.parse(res.payload)).toEqual({ inserted: 3 });
|
||||
expect(repo.createMany).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('rejects invalid event (missing eventKind)', async () => {
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/audit/events',
|
||||
payload: [{ sessionId: 'x', projectName: 'p', source: 'mcplocal', timestamp: '2026-03-01T00:00:00Z' }],
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(400);
|
||||
});
|
||||
|
||||
it('rejects empty array', async () => {
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/audit/events',
|
||||
payload: [],
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(400);
|
||||
});
|
||||
});
|
||||
|
||||
describe('GET /api/v1/audit/events', () => {
|
||||
it('returns events filtered by sessionId', async () => {
|
||||
vi.mocked(repo.findAll).mockResolvedValue([makeEvent()]);
|
||||
vi.mocked(repo.count).mockResolvedValue(1);
|
||||
|
||||
const res = await app.inject({
|
||||
method: 'GET',
|
||||
url: '/api/v1/audit/events?sessionId=s1',
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
const body = JSON.parse(res.payload);
|
||||
expect(body.events).toHaveLength(1);
|
||||
expect(body.total).toBe(1);
|
||||
});
|
||||
|
||||
it('returns events filtered by projectName and eventKind', async () => {
|
||||
vi.mocked(repo.findAll).mockResolvedValue([]);
|
||||
vi.mocked(repo.count).mockResolvedValue(0);
|
||||
|
||||
await app.inject({
|
||||
method: 'GET',
|
||||
url: '/api/v1/audit/events?projectName=ha&eventKind=gate_decision',
|
||||
});
|
||||
|
||||
const call = vi.mocked(repo.findAll).mock.calls[0]![0] as AuditEventFilter;
|
||||
expect(call.projectName).toBe('ha');
|
||||
expect(call.eventKind).toBe('gate_decision');
|
||||
});
|
||||
|
||||
it('supports time range filtering', async () => {
|
||||
vi.mocked(repo.findAll).mockResolvedValue([]);
|
||||
vi.mocked(repo.count).mockResolvedValue(0);
|
||||
|
||||
await app.inject({
|
||||
method: 'GET',
|
||||
url: '/api/v1/audit/events?from=2026-03-01&to=2026-03-02',
|
||||
});
|
||||
|
||||
const call = vi.mocked(repo.findAll).mock.calls[0]![0] as AuditEventFilter;
|
||||
expect(call.from).toEqual(new Date('2026-03-01'));
|
||||
expect(call.to).toEqual(new Date('2026-03-02'));
|
||||
});
|
||||
|
||||
it('paginates with limit and offset', async () => {
|
||||
vi.mocked(repo.findAll).mockResolvedValue([]);
|
||||
vi.mocked(repo.count).mockResolvedValue(100);
|
||||
|
||||
await app.inject({
|
||||
method: 'GET',
|
||||
url: '/api/v1/audit/events?limit=10&offset=20',
|
||||
});
|
||||
|
||||
const call = vi.mocked(repo.findAll).mock.calls[0]![0] as AuditEventFilter;
|
||||
expect(call.limit).toBe(10);
|
||||
expect(call.offset).toBe(20);
|
||||
});
|
||||
});
|
||||
|
||||
describe('GET /api/v1/audit/events/:id', () => {
|
||||
it('returns single event by id', async () => {
|
||||
vi.mocked(repo.findById).mockResolvedValue(makeEvent({ id: 'evt-42' }));
|
||||
|
||||
const res = await app.inject({
|
||||
method: 'GET',
|
||||
url: '/api/v1/audit/events/evt-42',
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
const body = JSON.parse(res.payload);
|
||||
expect(body.id).toBe('evt-42');
|
||||
});
|
||||
|
||||
it('returns 404 for missing event', async () => {
|
||||
vi.mocked(repo.findById).mockResolvedValue(null);
|
||||
|
||||
const res = await app.inject({
|
||||
method: 'GET',
|
||||
url: '/api/v1/audit/events/nonexistent',
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(404);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -34,7 +34,7 @@ const mockSecrets = [
|
||||
|
||||
const mockProjects = [
|
||||
{
|
||||
id: 'proj1', name: 'my-project', description: 'Test project', proxyMode: 'direct', llmProvider: null, llmModel: null,
|
||||
id: 'proj1', name: 'my-project', description: 'Test project', proxyMode: 'direct', proxyModel: '', llmProvider: null, llmModel: null,
|
||||
ownerId: 'user1', version: 1, createdAt: new Date(), updatedAt: new Date(),
|
||||
servers: [{ id: 'ps1', server: { id: 's1', name: 'github' } }],
|
||||
},
|
||||
|
||||
@@ -16,9 +16,12 @@ function makeProject(overrides: Partial<ProjectWithRelations> = {}): ProjectWith
|
||||
description: '',
|
||||
ownerId: 'user-1',
|
||||
proxyMode: 'direct',
|
||||
prompt: '',
|
||||
proxyModel: '',
|
||||
gated: true,
|
||||
llmProvider: null,
|
||||
llmModel: null,
|
||||
serverOverrides: null,
|
||||
version: 1,
|
||||
createdAt: new Date(),
|
||||
updatedAt: new Date(),
|
||||
@@ -149,6 +152,21 @@ describe('Project Routes', () => {
|
||||
expect(res.statusCode).toBe(201);
|
||||
});
|
||||
|
||||
it('creates a project with proxyModel', async () => {
|
||||
const repo = mockProjectRepo();
|
||||
vi.mocked(repo.findById).mockResolvedValue(makeProject({ name: 'pm-proj', proxyModel: 'subindex' }));
|
||||
await createApp(repo);
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/projects',
|
||||
payload: { name: 'pm-proj', proxyModel: 'subindex' },
|
||||
});
|
||||
expect(res.statusCode).toBe(201);
|
||||
expect(repo.create).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ proxyModel: 'subindex' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('returns 400 for invalid input', async () => {
|
||||
const repo = mockProjectRepo();
|
||||
await createApp(repo);
|
||||
@@ -186,6 +204,19 @@ describe('Project Routes', () => {
|
||||
expect(res.statusCode).toBe(200);
|
||||
});
|
||||
|
||||
it('updates proxyModel on a project', async () => {
|
||||
const repo = mockProjectRepo();
|
||||
vi.mocked(repo.findById).mockResolvedValue(makeProject({ id: 'p1' }));
|
||||
await createApp(repo);
|
||||
const res = await app.inject({
|
||||
method: 'PUT',
|
||||
url: '/api/v1/projects/p1',
|
||||
payload: { proxyModel: 'subindex' },
|
||||
});
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(repo.update).toHaveBeenCalledWith('p1', expect.objectContaining({ proxyModel: 'subindex' }));
|
||||
});
|
||||
|
||||
it('returns 404 when not found', async () => {
|
||||
const repo = mockProjectRepo();
|
||||
await createApp(repo);
|
||||
@@ -281,4 +312,50 @@ describe('Project Routes', () => {
|
||||
expect(res.statusCode).toBe(404);
|
||||
});
|
||||
});
|
||||
|
||||
describe('serverOverrides', () => {
|
||||
it('accepts serverOverrides in project create', async () => {
|
||||
const repo = mockProjectRepo();
|
||||
vi.mocked(repo.findById).mockResolvedValue(
|
||||
makeProject({ name: 'override-proj', serverOverrides: { ha: { proxyModel: 'ha-special' } } }),
|
||||
);
|
||||
await createApp(repo);
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/projects',
|
||||
payload: { name: 'override-proj', serverOverrides: { ha: { proxyModel: 'ha-special' } } },
|
||||
});
|
||||
expect(res.statusCode).toBe(201);
|
||||
expect(repo.create).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ serverOverrides: { ha: { proxyModel: 'ha-special' } } }),
|
||||
);
|
||||
});
|
||||
|
||||
it('accepts serverOverrides in project update', async () => {
|
||||
const repo = mockProjectRepo();
|
||||
vi.mocked(repo.findById).mockResolvedValue(makeProject({ id: 'p1' }));
|
||||
await createApp(repo);
|
||||
const res = await app.inject({
|
||||
method: 'PUT',
|
||||
url: '/api/v1/projects/p1',
|
||||
payload: { serverOverrides: { ha: { proxyModel: 'ha-special' } } },
|
||||
});
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(repo.update).toHaveBeenCalledWith('p1', expect.objectContaining({
|
||||
serverOverrides: { ha: { proxyModel: 'ha-special' } },
|
||||
}));
|
||||
});
|
||||
|
||||
it('returns serverOverrides in project GET', async () => {
|
||||
const repo = mockProjectRepo();
|
||||
vi.mocked(repo.findById).mockResolvedValue(
|
||||
makeProject({ id: 'p1', name: 'ha-proj', serverOverrides: { ha: { proxyModel: 'ha-special' } } }),
|
||||
);
|
||||
await createApp(repo);
|
||||
const res = await app.inject({ method: 'GET', url: '/api/v1/projects/p1' });
|
||||
expect(res.statusCode).toBe(200);
|
||||
const body = res.json<{ serverOverrides: unknown }>();
|
||||
expect(body.serverOverrides).toEqual({ ha: { proxyModel: 'ha-special' } });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -12,6 +12,7 @@ function makeProject(overrides: Partial<ProjectWithRelations> = {}): ProjectWith
|
||||
description: '',
|
||||
ownerId: 'user-1',
|
||||
proxyMode: 'direct',
|
||||
proxyModel: '',
|
||||
gated: true,
|
||||
llmProvider: null,
|
||||
llmModel: null,
|
||||
|
||||
@@ -49,6 +49,7 @@ function makeProject(overrides: Partial<Project> = {}): Project {
|
||||
description: '',
|
||||
prompt: '',
|
||||
proxyMode: 'direct',
|
||||
proxyModel: '',
|
||||
gated: true,
|
||||
llmProvider: null,
|
||||
llmModel: null,
|
||||
|
||||
476
src/mcpd/tests/security.test.ts
Normal file
476
src/mcpd/tests/security.test.ts
Normal file
@@ -0,0 +1,476 @@
|
||||
/**
|
||||
* Security tests for mcpd.
|
||||
*
|
||||
* Tests for identified security issues:
|
||||
* 1. audit-events endpoint bypasses RBAC (mapUrlToPermission returns 'skip')
|
||||
* 2. x-service-account header impersonation (any authenticated user can set it)
|
||||
* 3. MCP proxy maps to wrong RBAC action (POST → 'create' instead of 'run')
|
||||
* 4. externalUrl has no scheme/destination restriction (SSRF)
|
||||
* 5. MCP proxy has no input validation on method/serverId
|
||||
* 6. RBAC list filtering only checks 'name' field
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import Fastify from 'fastify';
|
||||
import type { FastifyInstance, FastifyRequest, FastifyReply } from 'fastify';
|
||||
import { registerMcpProxyRoutes } from '../src/routes/mcp-proxy.js';
|
||||
import type { McpProxyRouteDeps } from '../src/routes/mcp-proxy.js';
|
||||
import { registerAuditEventRoutes } from '../src/routes/audit-events.js';
|
||||
import { AuditEventService } from '../src/services/audit-event.service.js';
|
||||
import type { IAuditEventRepository } from '../src/repositories/interfaces.js';
|
||||
import { errorHandler } from '../src/middleware/error-handler.js';
|
||||
import { CreateMcpServerSchema } from '../src/validation/mcp-server.schema.js';
|
||||
|
||||
// ─────────────────────────────────────────────────────────
|
||||
// § 1 audit-events endpoint bypasses RBAC
|
||||
// ─────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Reproduce mapUrlToPermission from main.ts to test which URLs
|
||||
* get RBAC checks and which are skipped.
|
||||
*/
|
||||
type PermissionCheck =
|
||||
| { kind: 'resource'; resource: string; action: string; resourceName?: string }
|
||||
| { kind: 'operation'; operation: string }
|
||||
| { kind: 'skip' };
|
||||
|
||||
function mapUrlToPermission(method: string, url: string): PermissionCheck {
|
||||
const match = url.match(/^\/api\/v1\/([a-z-]+)/);
|
||||
if (!match) return { kind: 'skip' };
|
||||
|
||||
const segment = match[1] as string;
|
||||
|
||||
if (segment === 'backup') return { kind: 'operation', operation: 'backup' };
|
||||
if (segment === 'restore') return { kind: 'operation', operation: 'restore' };
|
||||
if (segment === 'audit-logs' && method === 'DELETE') return { kind: 'operation', operation: 'audit-purge' };
|
||||
|
||||
const resourceMap: Record<string, string | undefined> = {
|
||||
'servers': 'servers',
|
||||
'instances': 'instances',
|
||||
'secrets': 'secrets',
|
||||
'projects': 'projects',
|
||||
'templates': 'templates',
|
||||
'users': 'users',
|
||||
'groups': 'groups',
|
||||
'rbac': 'rbac',
|
||||
'audit-logs': 'rbac',
|
||||
'mcp': 'servers',
|
||||
'prompts': 'prompts',
|
||||
'promptrequests': 'promptrequests',
|
||||
};
|
||||
|
||||
const resource = resourceMap[segment];
|
||||
if (resource === undefined) return { kind: 'skip' };
|
||||
|
||||
let action: string;
|
||||
switch (method) {
|
||||
case 'GET':
|
||||
case 'HEAD':
|
||||
action = 'view';
|
||||
break;
|
||||
case 'POST':
|
||||
action = 'create';
|
||||
break;
|
||||
case 'DELETE':
|
||||
action = 'delete';
|
||||
break;
|
||||
default:
|
||||
action = 'edit';
|
||||
break;
|
||||
}
|
||||
|
||||
const nameMatch = url.match(/^\/api\/v1\/[a-z-]+\/([^/?]+)/);
|
||||
const resourceName = nameMatch?.[1];
|
||||
const check: PermissionCheck = { kind: 'resource', resource, action };
|
||||
if (resourceName !== undefined) (check as { resourceName: string }).resourceName = resourceName;
|
||||
return check;
|
||||
}
|
||||
|
||||
describe('Security: RBAC coverage gaps in mapUrlToPermission', () => {
|
||||
it('audit-events endpoint is NOT in resourceMap — bypasses RBAC', () => {
|
||||
// This documents a known security issue: any authenticated user can query
|
||||
// all audit events regardless of their RBAC permissions
|
||||
const check = mapUrlToPermission('GET', '/api/v1/audit-events');
|
||||
// Currently returns 'skip' — this is the bug
|
||||
expect(check.kind).toBe('skip');
|
||||
});
|
||||
|
||||
it('audit-events POST (batch insert) also bypasses RBAC', () => {
|
||||
const check = mapUrlToPermission('POST', '/api/v1/audit-events');
|
||||
expect(check.kind).toBe('skip');
|
||||
});
|
||||
|
||||
it('audit-events by ID also bypasses RBAC', () => {
|
||||
const check = mapUrlToPermission('GET', '/api/v1/audit-events/some-cuid');
|
||||
expect(check.kind).toBe('skip');
|
||||
});
|
||||
|
||||
it('all known resource endpoints DO have RBAC coverage', () => {
|
||||
const coveredEndpoints = [
|
||||
'servers', 'instances', 'secrets', 'projects', 'templates',
|
||||
'users', 'groups', 'rbac', 'audit-logs', 'prompts', 'promptrequests',
|
||||
];
|
||||
|
||||
for (const endpoint of coveredEndpoints) {
|
||||
const check = mapUrlToPermission('GET', `/api/v1/${endpoint}`);
|
||||
expect(check.kind, `${endpoint} should have RBAC check`).not.toBe('skip');
|
||||
}
|
||||
});
|
||||
|
||||
it('MCP proxy maps POST to servers:create instead of servers:run', () => {
|
||||
// /api/v1/mcp/proxy is a POST that executes tools — semantically this is
|
||||
// a 'run' action, but mapUrlToPermission maps POST → 'create'
|
||||
const check = mapUrlToPermission('POST', '/api/v1/mcp/proxy');
|
||||
expect(check.kind).toBe('resource');
|
||||
if (check.kind === 'resource') {
|
||||
expect(check.resource).toBe('servers');
|
||||
// BUG: should be 'run' for executing tools, not 'create'
|
||||
expect(check.action).toBe('create');
|
||||
}
|
||||
});
|
||||
|
||||
it('non-api URLs correctly return skip', () => {
|
||||
expect(mapUrlToPermission('GET', '/healthz').kind).toBe('skip');
|
||||
expect(mapUrlToPermission('GET', '/health').kind).toBe('skip');
|
||||
expect(mapUrlToPermission('GET', '/').kind).toBe('skip');
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────
|
||||
// § 2 x-service-account header impersonation
|
||||
// ─────────────────────────────────────────────────────────
|
||||
|
||||
describe('Security: x-service-account header impersonation', () => {
|
||||
// This test documents that any authenticated user can impersonate service accounts
|
||||
// by setting the x-service-account header. The RBAC service trusts this header
|
||||
// and adds the service account's permissions to the user's permissions.
|
||||
|
||||
it('x-service-account header is passed to RBAC without verification', () => {
|
||||
// The RBAC service's getPermissions() accepts serviceAccountName directly.
|
||||
// In main.ts, the value comes from: request.headers['x-service-account']
|
||||
// There is no validation that the authenticated user IS the service account,
|
||||
// or that the user is authorized to act as that service account.
|
||||
//
|
||||
// Attack scenario:
|
||||
// 1. Attacker authenticates as regular user (low-privilege)
|
||||
// 2. Sends request with header: x-service-account: project:admin
|
||||
// 3. RBAC service treats them as having the service account's bindings
|
||||
// 4. Attacker gets elevated permissions
|
||||
|
||||
// We verify this by examining the RBAC service code path:
|
||||
// In rbac.service.ts line 144:
|
||||
// if (s.kind === 'ServiceAccount') return serviceAccountName !== undefined && s.name === serviceAccountName;
|
||||
// This matches ANY request with the right header value — no ownership check.
|
||||
expect(true).toBe(true); // Structural documentation test
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────
|
||||
// § 3 MCP proxy input validation
|
||||
// ─────────────────────────────────────────────────────────
|
||||
|
||||
describe('Security: MCP proxy input validation', () => {
|
||||
let app: FastifyInstance;
|
||||
|
||||
afterEach(async () => {
|
||||
if (app) await app.close();
|
||||
});
|
||||
|
||||
function buildApp() {
|
||||
const mcpProxyService = {
|
||||
execute: vi.fn(async () => ({
|
||||
jsonrpc: '2.0' as const,
|
||||
id: 1,
|
||||
result: { tools: [] },
|
||||
})),
|
||||
};
|
||||
const auditLogService = {
|
||||
create: vi.fn(async () => ({ id: 'log-1' })),
|
||||
};
|
||||
const authDeps = {
|
||||
findSession: vi.fn(async () => ({
|
||||
userId: 'user-1',
|
||||
expiresAt: new Date(Date.now() + 3600_000),
|
||||
})),
|
||||
};
|
||||
|
||||
app = Fastify({ logger: false });
|
||||
app.setErrorHandler(errorHandler);
|
||||
registerMcpProxyRoutes(app, {
|
||||
mcpProxyService,
|
||||
auditLogService,
|
||||
authDeps,
|
||||
} as unknown as McpProxyRouteDeps);
|
||||
return { mcpProxyService, auditLogService };
|
||||
}
|
||||
|
||||
it('accepts arbitrary method strings (no allowlist)', async () => {
|
||||
// Any JSON-RPC method is forwarded to upstream servers without validation.
|
||||
// An attacker could send methods like 'shutdown', 'admin/reset', etc.
|
||||
const { mcpProxyService } = buildApp();
|
||||
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/mcp/proxy',
|
||||
payload: {
|
||||
serverId: 'srv-1',
|
||||
method: 'dangerous/admin_shutdown',
|
||||
params: {},
|
||||
},
|
||||
headers: { authorization: 'Bearer valid-token' },
|
||||
});
|
||||
|
||||
// Request succeeds — method is forwarded without validation
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(mcpProxyService.execute).toHaveBeenCalledWith({
|
||||
serverId: 'srv-1',
|
||||
method: 'dangerous/admin_shutdown',
|
||||
params: {},
|
||||
});
|
||||
});
|
||||
|
||||
it('accepts empty method string', async () => {
|
||||
const { mcpProxyService } = buildApp();
|
||||
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/mcp/proxy',
|
||||
payload: {
|
||||
serverId: 'srv-1',
|
||||
method: '',
|
||||
params: {},
|
||||
},
|
||||
headers: { authorization: 'Bearer valid-token' },
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(mcpProxyService.execute).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ method: '' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('no Zod schema validation on request body', async () => {
|
||||
// The route destructures body without schema validation.
|
||||
// Extra fields are silently accepted.
|
||||
const { mcpProxyService } = buildApp();
|
||||
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/mcp/proxy',
|
||||
payload: {
|
||||
serverId: 'srv-1',
|
||||
method: 'tools/list',
|
||||
params: {},
|
||||
__proto__: { isAdmin: true },
|
||||
extraField: 'injected',
|
||||
},
|
||||
headers: { authorization: 'Bearer valid-token' },
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────
|
||||
// § 4 externalUrl SSRF validation
|
||||
// ─────────────────────────────────────────────────────────
|
||||
|
||||
describe('Security: externalUrl SSRF via CreateMcpServerSchema', () => {
|
||||
it('accepts internal IP addresses (SSRF risk)', () => {
|
||||
// externalUrl uses z.string().url() which validates format but not destination
|
||||
const internalUrls = [
|
||||
'http://169.254.169.254/latest/meta-data/', // AWS metadata
|
||||
'http://metadata.google.internal/', // GCP metadata
|
||||
'http://100.100.100.200/latest/meta-data/', // Alibaba Cloud metadata
|
||||
'http://10.0.0.1/', // Private network
|
||||
'http://192.168.1.1/', // Private network
|
||||
'http://172.16.0.1/', // Private network
|
||||
'http://127.0.0.1:3100/', // Localhost (mcpd itself!)
|
||||
'http://[::1]:3100/', // IPv6 localhost
|
||||
'http://0.0.0.0/', // All interfaces
|
||||
];
|
||||
|
||||
for (const url of internalUrls) {
|
||||
const result = CreateMcpServerSchema.safeParse({
|
||||
name: 'test-server',
|
||||
externalUrl: url,
|
||||
});
|
||||
// All currently pass validation — this is the SSRF vulnerability
|
||||
expect(result.success, `${url} should be flagged but currently passes`).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('accepts file:// URLs', () => {
|
||||
const result = CreateMcpServerSchema.safeParse({
|
||||
name: 'test-server',
|
||||
externalUrl: 'file:///etc/passwd',
|
||||
});
|
||||
// z.string().url() validates format, and file:// is a valid URL scheme
|
||||
// Whether this passes or fails depends on the Zod version's url() validator
|
||||
// This test documents the current behavior
|
||||
if (result.success) {
|
||||
// If this passes, it's an additional SSRF vector
|
||||
expect(result.data.externalUrl).toBe('file:///etc/passwd');
|
||||
}
|
||||
});
|
||||
|
||||
it('correctly validates URL format', () => {
|
||||
const invalid = CreateMcpServerSchema.safeParse({
|
||||
name: 'test-server',
|
||||
externalUrl: 'not-a-url',
|
||||
});
|
||||
expect(invalid.success).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────
|
||||
// § 5 Audit events route — unauthenticated batch insert
|
||||
// ─────────────────────────────────────────────────────────
|
||||
|
||||
describe('Security: audit-events batch insert has no auth in route definition', () => {
|
||||
let app: FastifyInstance;
|
||||
let repo: IAuditEventRepository;
|
||||
|
||||
beforeEach(async () => {
|
||||
app = Fastify({ logger: false });
|
||||
app.setErrorHandler(errorHandler);
|
||||
repo = {
|
||||
findAll: vi.fn(async () => []),
|
||||
findById: vi.fn(async () => null),
|
||||
createMany: vi.fn(async (events: unknown[]) => events.length),
|
||||
count: vi.fn(async () => 0),
|
||||
};
|
||||
const service = new AuditEventService(repo);
|
||||
registerAuditEventRoutes(app, service);
|
||||
await app.ready();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
if (app) await app.close();
|
||||
});
|
||||
|
||||
it('batch insert accepts events without authentication at route level', async () => {
|
||||
// The route itself has no preHandler auth middleware (unlike mcp-proxy).
|
||||
// Auth is only applied via the global hook in main.ts.
|
||||
// If registerAuditEventRoutes is used outside of main.ts's global hook setup,
|
||||
// audit events can be inserted without auth.
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/audit/events',
|
||||
payload: [
|
||||
{
|
||||
timestamp: new Date().toISOString(),
|
||||
sessionId: 'fake-session',
|
||||
projectName: 'injected-project',
|
||||
eventKind: 'gate_decision',
|
||||
source: 'attacker',
|
||||
verified: true, // Attacker can claim verified=true
|
||||
payload: { trigger: 'fake', intent: 'malicious' },
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
// Without global auth hook, this succeeds
|
||||
expect(res.statusCode).toBe(201);
|
||||
expect(repo.createMany).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('attacker can inject events with verified=true (no server-side enforcement)', async () => {
|
||||
// The verified flag is accepted from the client without validation.
|
||||
// mcplocal (which runs on untrusted user devices) sends verified=true for its events.
|
||||
// An attacker could inject fake "verified" events to pollute the audit trail.
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/audit/events',
|
||||
payload: [
|
||||
{
|
||||
timestamp: new Date().toISOString(),
|
||||
sessionId: 'attacker-session',
|
||||
projectName: 'target-project',
|
||||
eventKind: 'gate_decision',
|
||||
source: 'mcpd', // Impersonate mcpd as source
|
||||
verified: true, // Claim it's verified
|
||||
payload: { trigger: 'begin_session', intent: 'legitimate looking' },
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(201);
|
||||
|
||||
// Verify the event was stored with attacker-controlled values
|
||||
const storedEvents = (repo.createMany as ReturnType<typeof vi.fn>).mock.calls[0]![0] as Array<Record<string, unknown>>;
|
||||
expect(storedEvents[0]).toMatchObject({
|
||||
source: 'mcpd',
|
||||
verified: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('attacker can inject events for any project', async () => {
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/audit/events',
|
||||
payload: [
|
||||
{
|
||||
timestamp: new Date().toISOString(),
|
||||
sessionId: 'attacker-session',
|
||||
projectName: 'production-sensitive-project',
|
||||
eventKind: 'tool_call_trace',
|
||||
source: 'mcplocal',
|
||||
verified: true,
|
||||
payload: { toolName: 'legitimate_tool' },
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(201);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────
|
||||
// § 6 RBAC list filtering only checks 'name' field
|
||||
// ─────────────────────────────────────────────────────────
|
||||
|
||||
describe('Security: RBAC list filtering gaps', () => {
|
||||
it('preSerialization hook only filters by name field', () => {
|
||||
// From main.ts lines 390-397:
|
||||
// The hook filters array responses by checking item['name'].
|
||||
// Resources without a 'name' field pass through unfiltered.
|
||||
//
|
||||
// Affected resources:
|
||||
// - AuditEvent (has no 'name' field → never filtered)
|
||||
// - AuditLog (has no 'name' field → never filtered)
|
||||
// - Any future resource without a 'name' field
|
||||
|
||||
// Simulate the filtering logic
|
||||
const payload = [
|
||||
{ id: '1', name: 'allowed-server', description: 'visible' },
|
||||
{ id: '2', name: 'forbidden-server', description: 'should be hidden' },
|
||||
{ id: '3', description: 'no name field — passes through' },
|
||||
];
|
||||
|
||||
const rbacScope = { wildcard: false, names: new Set(['allowed-server']) };
|
||||
|
||||
// Apply the filtering logic from main.ts
|
||||
const filtered = payload.filter((item) => {
|
||||
const name = item['name' as keyof typeof item];
|
||||
return typeof name === 'string' && rbacScope.names.has(name);
|
||||
});
|
||||
|
||||
// Items with matching name are included
|
||||
expect(filtered).toHaveLength(1);
|
||||
expect(filtered[0]!.name).toBe('allowed-server');
|
||||
|
||||
// BUG: Items without a name field are EXCLUDED, not leaked through.
|
||||
// Actually re-reading: typeof undefined === 'undefined', so the filter
|
||||
// returns false for items without name. This means nameless items are
|
||||
// EXCLUDED when rbacScope is active — which may cause audit events to
|
||||
// disappear from filtered responses. Not a leak, but a usability issue.
|
||||
});
|
||||
|
||||
it('wildcard scope bypasses all filtering', () => {
|
||||
const rbacScope = { wildcard: true, names: new Set<string>() };
|
||||
|
||||
// When wildcard is true, the hook returns payload as-is
|
||||
// This is correct behavior — wildcard means "see everything"
|
||||
expect(rbacScope.wildcard).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -43,6 +43,7 @@ function makeProject(overrides: Partial<Project> = {}): Project {
|
||||
description: '',
|
||||
prompt: '',
|
||||
proxyMode: 'direct',
|
||||
proxyModel: '',
|
||||
gated: true,
|
||||
llmProvider: null,
|
||||
llmModel: null,
|
||||
@@ -400,8 +401,8 @@ describe('PromptService', () => {
|
||||
const result = await service.getVisiblePrompts('proj-1', 'sess-1');
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result[0]).toEqual({ name: 'approved-1', content: 'A', type: 'prompt' });
|
||||
expect(result[1]).toEqual({ name: 'pending-1', content: 'B', type: 'promptrequest' });
|
||||
expect(result[0]).toMatchObject({ name: 'approved-1', content: 'A', type: 'prompt' });
|
||||
expect(result[1]).toMatchObject({ name: 'pending-1', content: 'B', type: 'promptrequest' });
|
||||
});
|
||||
|
||||
it('should not include pending requests without sessionId', async () => {
|
||||
|
||||
Reference in New Issue
Block a user