test(ai-chat): cover safety-critical code paths (review follow-ups P1) #3
@@ -1,4 +1,4 @@
|
||||
import { compactToolOutput } from './ai-chat.service';
|
||||
import { compactToolOutput, assistantParts, type StepLike } from './ai-chat.service';
|
||||
|
||||
/**
|
||||
* Unit tests for compactToolOutput: the pure helper that shrinks LARGE tool
|
||||
@@ -66,3 +66,78 @@ describe('compactToolOutput', () => {
|
||||
expect(compactedBytes).toBeLessThan(originalBytes / 10);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Unit tests for assistantParts: the pure helper that rebuilds UIMessage parts
|
||||
* from SDK step objects so multi-turn history replays prior tool-calls and
|
||||
* results to the model. The key invariant is that an UNPAIRED tool call (no
|
||||
* matching result) is emitted as a synthetic `output-error` — without this,
|
||||
* convertToModelMessages would throw MissingToolResultsError on the next turn.
|
||||
*/
|
||||
describe('assistantParts', () => {
|
||||
it('emits output-available for a paired tool call with its (compacted) output', () => {
|
||||
const steps: StepLike[] = [
|
||||
{
|
||||
toolCalls: [{ toolCallId: 'c1', toolName: 'getPage', input: { pageId: 'p1' } }],
|
||||
toolResults: [{ toolCallId: 'c1', output: { title: 'T' } }],
|
||||
},
|
||||
];
|
||||
const parts = assistantParts(steps, 'irrelevant') as Array<
|
||||
Record<string, unknown>
|
||||
>;
|
||||
const toolPart = parts.find((p) => p.type === 'tool-getPage');
|
||||
expect(toolPart).toBeDefined();
|
||||
expect(toolPart!.state).toBe('output-available');
|
||||
expect(toolPart!.output).toEqual({ title: 'T' });
|
||||
expect(toolPart!.input).toEqual({ pageId: 'p1' });
|
||||
});
|
||||
|
||||
it('emits output-error for an unpaired tool call (regression guard)', () => {
|
||||
const steps: StepLike[] = [
|
||||
{
|
||||
toolCalls: [{ toolCallId: 'c2', toolName: 'getPage', input: {} }],
|
||||
toolResults: [],
|
||||
},
|
||||
];
|
||||
const parts = assistantParts(steps, '') as Array<Record<string, unknown>>;
|
||||
const toolPart = parts.find((p) => p.type === 'tool-getPage');
|
||||
expect(toolPart).toBeDefined();
|
||||
expect(toolPart!.state).toBe('output-error');
|
||||
expect(toolPart!.errorText).toBe('Tool call did not complete.');
|
||||
});
|
||||
|
||||
it('skips tool calls missing toolName or toolCallId', () => {
|
||||
const steps: StepLike[] = [
|
||||
{
|
||||
toolCalls: [
|
||||
{ input: { x: 1 } },
|
||||
{ toolName: 'getPage' },
|
||||
{ toolCallId: 'c4' },
|
||||
],
|
||||
toolResults: [],
|
||||
},
|
||||
];
|
||||
const parts = assistantParts(steps, '') as Array<Record<string, unknown>>;
|
||||
expect(parts.filter((p) => String(p.type).startsWith('tool-'))).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('includes a text part for a step that carries text', () => {
|
||||
const steps: StepLike[] = [{ text: 'hello' }];
|
||||
const parts = assistantParts(steps, '') as Array<Record<string, unknown>>;
|
||||
expect(parts).toContainEqual({ type: 'text', text: 'hello' });
|
||||
});
|
||||
|
||||
it('appends the fallback text when no step carries text', () => {
|
||||
const steps: StepLike[] = [
|
||||
{
|
||||
toolCalls: [{ toolCallId: 'c5', toolName: 'getPage', input: {} }],
|
||||
toolResults: [],
|
||||
},
|
||||
];
|
||||
const parts = assistantParts(steps, 'final answer') as Array<
|
||||
Record<string, unknown>
|
||||
>;
|
||||
const last = parts[parts.length - 1];
|
||||
expect(last).toEqual({ type: 'text', text: 'final answer' });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -419,7 +419,8 @@ function textPart(text: string): Array<{ type: 'text'; text: string }> {
|
||||
* `toolResults` -> TypedToolResult). Typed loosely so this survives provider
|
||||
* variation; only the fields we persist are referenced.
|
||||
*/
|
||||
type StepLike = {
|
||||
// Exported for unit testing (ai-chat.service.spec.ts).
|
||||
export type StepLike = {
|
||||
text?: string;
|
||||
toolCalls?: ReadonlyArray<{
|
||||
toolCallId?: string;
|
||||
@@ -538,7 +539,8 @@ function compactValue(value: unknown, depth: number): unknown {
|
||||
* recovers the name. Falls back to a single `text` part built from
|
||||
* `fallbackText` when the steps carry no text.
|
||||
*/
|
||||
function assistantParts(
|
||||
// Exported for unit testing (ai-chat.service.spec.ts).
|
||||
export function assistantParts(
|
||||
steps: ReadonlyArray<StepLike> | undefined,
|
||||
fallbackText: string,
|
||||
): UIMessage['parts'] {
|
||||
|
||||
116
apps/server/src/core/ai-chat/external-mcp/ssrf-guard.spec.ts
Normal file
116
apps/server/src/core/ai-chat/external-mcp/ssrf-guard.spec.ts
Normal file
@@ -0,0 +1,116 @@
|
||||
// Hoisted before imports: ssrf-guard.ts calls `promisify(dns.lookup)` at module
|
||||
// load time, so the mock must be in place before the first import of the guard.
|
||||
jest.mock('node:dns', () => ({ lookup: jest.fn() }));
|
||||
|
||||
import { lookup as dnsLookup } from 'node:dns';
|
||||
import { isIpAllowed, isUrlAllowed } from './ssrf-guard';
|
||||
|
||||
const lookupMock = dnsLookup as unknown as jest.Mock;
|
||||
|
||||
/**
|
||||
* Unit tests for the SSRF guard that protects the admin-configured external MCP
|
||||
* server URLs. `isIpAllowed` is pure and synchronous; `isUrlAllowed` resolves
|
||||
* the hostname via dns.lookup (mocked here) and blocks if ANY resolved address
|
||||
* is non-public. Every blocked range corresponds to a real exploit vector.
|
||||
*/
|
||||
describe('isIpAllowed', () => {
|
||||
it.each([
|
||||
['127.0.0.1', 'loopback'],
|
||||
['::1', 'loopback'],
|
||||
])('blocks IPv4/IPv6 loopback %s', (ip) => {
|
||||
expect(isIpAllowed(ip)).toEqual({ ok: false, reason: expect.any(String) });
|
||||
expect(isIpAllowed(ip).ok).toBe(false);
|
||||
});
|
||||
|
||||
it.each(['169.254.169.254', 'fe80::1'])(
|
||||
'blocks link-local / metadata endpoint %s',
|
||||
(ip) => {
|
||||
expect(isIpAllowed(ip).ok).toBe(false);
|
||||
},
|
||||
);
|
||||
|
||||
it.each(['10.0.0.1', '172.16.0.1', '192.168.1.1'])(
|
||||
'blocks private range %s',
|
||||
(ip) => {
|
||||
expect(isIpAllowed(ip).ok).toBe(false);
|
||||
},
|
||||
);
|
||||
|
||||
it('blocks CGNAT 100.64.0.1', () => {
|
||||
expect(isIpAllowed('100.64.0.1').ok).toBe(false);
|
||||
});
|
||||
|
||||
it('blocks ULA fd00::1', () => {
|
||||
expect(isIpAllowed('fd00::1').ok).toBe(false);
|
||||
});
|
||||
|
||||
it.each(['0.0.0.0', '::'])('blocks unspecified %s', (ip) => {
|
||||
expect(isIpAllowed(ip).ok).toBe(false);
|
||||
});
|
||||
|
||||
it('blocks IPv4-mapped IPv6 loopback ::ffff:127.0.0.1', () => {
|
||||
expect(isIpAllowed('::ffff:127.0.0.1').ok).toBe(false);
|
||||
});
|
||||
|
||||
it('returns an unparseable reason for garbage', () => {
|
||||
expect(isIpAllowed('not-an-ip')).toEqual({
|
||||
ok: false,
|
||||
reason: 'unparseable IP address',
|
||||
});
|
||||
});
|
||||
|
||||
it.each(['8.8.8.8', '1.1.1.1'])('allows public IP %s', (ip) => {
|
||||
expect(isIpAllowed(ip)).toEqual({ ok: true });
|
||||
});
|
||||
});
|
||||
|
||||
describe('isUrlAllowed', () => {
|
||||
afterEach(() => {
|
||||
lookupMock.mockReset();
|
||||
});
|
||||
|
||||
it('rejects a non-http scheme', async () => {
|
||||
const res = await isUrlAllowed('ftp://example.com');
|
||||
expect(res).toEqual({ ok: false, reason: 'only http/https URLs are allowed' });
|
||||
});
|
||||
|
||||
it('rejects an invalid URL', async () => {
|
||||
const res = await isUrlAllowed('not a url');
|
||||
expect(res).toEqual({ ok: false, reason: 'invalid URL' });
|
||||
});
|
||||
|
||||
it('blocks a private IP-literal host (no DNS needed)', async () => {
|
||||
const res = await isUrlAllowed('http://127.0.0.1');
|
||||
expect(res.ok).toBe(false);
|
||||
});
|
||||
|
||||
it('allows a public IP-literal host', async () => {
|
||||
const res = await isUrlAllowed('http://8.8.8.8');
|
||||
expect(res.ok).toBe(true);
|
||||
});
|
||||
|
||||
it('blocks DNS-rebinding: hostname resolves to a private address', async () => {
|
||||
lookupMock.mockImplementation(
|
||||
(host, opts, cb) => cb(null, [{ address: '10.0.0.5' }]),
|
||||
);
|
||||
const res = await isUrlAllowed('http://rebind.test');
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.reason).toContain('blocked address range');
|
||||
});
|
||||
|
||||
it('allows a hostname that resolves to a public address', async () => {
|
||||
lookupMock.mockImplementation(
|
||||
(host, opts, cb) => cb(null, [{ address: '93.184.216.34' }]),
|
||||
);
|
||||
const res = await isUrlAllowed('http://example.com');
|
||||
expect(res.ok).toBe(true);
|
||||
});
|
||||
|
||||
it('blocks an unresolvable hostname', async () => {
|
||||
lookupMock.mockImplementation(
|
||||
(host, opts, cb) => cb(new Error('ENOTFOUND')),
|
||||
);
|
||||
const res = await isUrlAllowed('http://nope.test');
|
||||
expect(res).toEqual({ ok: false, reason: 'host could not be resolved' });
|
||||
});
|
||||
});
|
||||
@@ -211,3 +211,171 @@ describe('AiChatToolsService expanded toolset guardrails', () => {
|
||||
expect(parsed).not.toHaveProperty('deleteComments');
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Tests for the JSON-string node/content coercion in patchNode, insertNode, and
|
||||
* updatePageJson. OpenAI tool-calls sometimes serialize object arguments as
|
||||
* JSON strings; the execute body parses them before forwarding to the client.
|
||||
* A regression that removes the parse would silently break insert/patch under
|
||||
* OpenAI. The recording fake client captures forwarded args so we can assert
|
||||
* the PARSED object (not the string) reaches the client.
|
||||
*/
|
||||
describe('AiChatToolsService JSON-string node/content coercion', () => {
|
||||
const forwarded = {
|
||||
patchNode: [] as unknown[][],
|
||||
insertNode: [] as unknown[][],
|
||||
updatePageJson: [] as unknown[][],
|
||||
};
|
||||
|
||||
const fakeClient: Partial<DocmostClientLike> = {
|
||||
patchNode: (...args: unknown[]) => {
|
||||
forwarded.patchNode.push(args);
|
||||
return Promise.resolve({});
|
||||
},
|
||||
insertNode: (...args: unknown[]) => {
|
||||
forwarded.insertNode.push(args);
|
||||
return Promise.resolve({});
|
||||
},
|
||||
updatePageJson: (...args: unknown[]) => {
|
||||
forwarded.updatePageJson.push(args);
|
||||
return Promise.resolve({});
|
||||
},
|
||||
};
|
||||
|
||||
const tokenServiceStub = {
|
||||
generateAccessToken: jest.fn().mockResolvedValue('access-token'),
|
||||
generateCollabToken: jest.fn().mockResolvedValue('collab-token'),
|
||||
};
|
||||
|
||||
let service: AiChatToolsService;
|
||||
|
||||
beforeEach(() => {
|
||||
forwarded.patchNode.length = 0;
|
||||
forwarded.insertNode.length = 0;
|
||||
forwarded.updatePageJson.length = 0;
|
||||
jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue({
|
||||
DocmostClient: function () {
|
||||
return fakeClient as DocmostClientLike;
|
||||
} as unknown as loader.DocmostClientCtor,
|
||||
});
|
||||
service = new AiChatToolsService(
|
||||
tokenServiceStub as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.restoreAllMocks();
|
||||
});
|
||||
|
||||
async function buildTools() {
|
||||
return service.forUser(
|
||||
{ id: 'user-1', email: 'u@example.com', workspaceId: 'ws-1' } as never,
|
||||
'session-1',
|
||||
'ws-1',
|
||||
'chat-1',
|
||||
);
|
||||
}
|
||||
|
||||
it('patchNode: parses a JSON-string node and forwards the object', async () => {
|
||||
const tools = await buildTools();
|
||||
await tools.patchNode.execute(
|
||||
{ pageId: 'p1', nodeId: 'n1', node: '{"type":"paragraph"}' } as never,
|
||||
{} as never,
|
||||
);
|
||||
expect(forwarded.patchNode[0]).toEqual([
|
||||
'p1',
|
||||
'n1',
|
||||
{ type: 'paragraph' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('patchNode: throws on an invalid JSON string', async () => {
|
||||
const tools = await buildTools();
|
||||
await expect(
|
||||
tools.patchNode.execute(
|
||||
{ pageId: 'p1', nodeId: 'n1', node: '{not json' } as never,
|
||||
{} as never,
|
||||
),
|
||||
).rejects.toThrow('node was a string but not valid JSON');
|
||||
});
|
||||
|
||||
it('insertNode: parses a JSON-string node and forwards the object', async () => {
|
||||
const tools = await buildTools();
|
||||
await tools.insertNode.execute(
|
||||
{
|
||||
pageId: 'p1',
|
||||
node: '{"type":"paragraph"}',
|
||||
position: 'append',
|
||||
} as never,
|
||||
{} as never,
|
||||
);
|
||||
expect(forwarded.insertNode[0]).toEqual([
|
||||
'p1',
|
||||
{ type: 'paragraph' },
|
||||
{ position: 'append', anchorNodeId: undefined, anchorText: undefined },
|
||||
]);
|
||||
});
|
||||
|
||||
it('insertNode: throws on an invalid JSON string', async () => {
|
||||
const tools = await buildTools();
|
||||
await expect(
|
||||
tools.insertNode.execute(
|
||||
{
|
||||
pageId: 'p1',
|
||||
node: 'not json',
|
||||
position: 'append',
|
||||
} as never,
|
||||
{} as never,
|
||||
),
|
||||
).rejects.toThrow('node was a string but not valid JSON');
|
||||
});
|
||||
|
||||
it('updatePageJson: content undefined forwards doc undefined (title-only)', async () => {
|
||||
const tools = await buildTools();
|
||||
await tools.updatePageJson.execute(
|
||||
{ pageId: 'p1', title: 'T' } as never,
|
||||
{} as never,
|
||||
);
|
||||
expect(forwarded.updatePageJson[0]).toEqual(['p1', undefined, 'T']);
|
||||
});
|
||||
|
||||
it('updatePageJson: content as object is forwarded as-is', async () => {
|
||||
const tools = await buildTools();
|
||||
const doc = { type: 'doc', content: [] };
|
||||
await tools.updatePageJson.execute(
|
||||
{ pageId: 'p1', content: doc } as never,
|
||||
{} as never,
|
||||
);
|
||||
expect(forwarded.updatePageJson[0]).toEqual(['p1', doc, undefined]);
|
||||
});
|
||||
|
||||
it('updatePageJson: content as JSON string is parsed and forwarded as object', async () => {
|
||||
const tools = await buildTools();
|
||||
await tools.updatePageJson.execute(
|
||||
{
|
||||
pageId: 'p1',
|
||||
content: '{"type":"doc","content":[]}',
|
||||
} as never,
|
||||
{} as never,
|
||||
);
|
||||
expect(forwarded.updatePageJson[0]).toEqual([
|
||||
'p1',
|
||||
{ type: 'doc', content: [] },
|
||||
undefined,
|
||||
]);
|
||||
});
|
||||
|
||||
it('updatePageJson: invalid JSON string throws', async () => {
|
||||
const tools = await buildTools();
|
||||
await expect(
|
||||
tools.updatePageJson.execute(
|
||||
{ pageId: 'p1', content: '{bad' } as never,
|
||||
{} as never,
|
||||
),
|
||||
).rejects.toThrow('content was a string but not valid JSON');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
import { PageEmbeddingRepo } from './page-embedding.repo';
|
||||
import type { KyselyDB } from '../../types/kysely.types';
|
||||
|
||||
/**
|
||||
* Unit test for the access-scoping early-return in searchByEmbedding: when
|
||||
* spaceIds is empty the repo MUST return [] without touching the DB. This is a
|
||||
* pure access-scoping guard — an empty space list means the caller has no
|
||||
* readable spaces, so the result set is always empty.
|
||||
*
|
||||
* The dimension-mismatch case (filtering rows by model_dimensions) requires a
|
||||
* real pgvector test database and is out of scope here.
|
||||
*/
|
||||
describe('PageEmbeddingRepo.searchByEmbedding early-return', () => {
|
||||
it('returns [] without accessing the DB when spaceIds is empty', async () => {
|
||||
const explosiveDb = new Proxy(
|
||||
{},
|
||||
{
|
||||
get() {
|
||||
throw new Error('DB must not be accessed when spaceIds is empty');
|
||||
},
|
||||
},
|
||||
) as unknown as KyselyDB;
|
||||
|
||||
const repo = new PageEmbeddingRepo(explosiveDb);
|
||||
|
||||
await expect(
|
||||
repo.searchByEmbedding('ws-1', [1, 2, 3], [], 10),
|
||||
).resolves.toEqual([]);
|
||||
});
|
||||
});
|
||||
66
apps/server/src/integrations/crypto/secret-box.spec.ts
Normal file
66
apps/server/src/integrations/crypto/secret-box.spec.ts
Normal file
@@ -0,0 +1,66 @@
|
||||
import { SecretBoxService } from './secret-box';
|
||||
|
||||
/**
|
||||
* Unit tests for SecretBoxService: the AES-256-GCM symmetric encryption that
|
||||
* protects provider API keys at rest. The output layout is
|
||||
* base64(salt[16] | iv[12] | authTag[16] | ciphertext). These tests guard the
|
||||
* round-trip correctness, the per-record randomness (salt + iv), and the tamper
|
||||
* / wrong-key detection (the error message the UI relies on).
|
||||
*/
|
||||
describe('SecretBoxService', () => {
|
||||
function makeService(secret = 'test-secret'): SecretBoxService {
|
||||
return new SecretBoxService({
|
||||
getAppSecret: () => secret,
|
||||
} as never);
|
||||
}
|
||||
|
||||
const PLAIN = 'sk-test-1234567890';
|
||||
|
||||
it('round-trips: decryptSecret(encryptSecret(plain)) === plain', () => {
|
||||
const svc = makeService();
|
||||
const blob = svc.encryptSecret(PLAIN);
|
||||
expect(svc.decryptSecret(blob)).toBe(PLAIN);
|
||||
});
|
||||
|
||||
it('produces different blobs for the same plaintext (random salt+iv), both decrypt back', () => {
|
||||
const svc = makeService();
|
||||
const blob1 = svc.encryptSecret(PLAIN);
|
||||
const blob2 = svc.encryptSecret(PLAIN);
|
||||
expect(blob1).not.toBe(blob2);
|
||||
expect(svc.decryptSecret(blob1)).toBe(PLAIN);
|
||||
expect(svc.decryptSecret(blob2)).toBe(PLAIN);
|
||||
});
|
||||
|
||||
it('detects a flipped authTag byte and throws the APP_SECRET message', () => {
|
||||
const svc = makeService();
|
||||
const blob = svc.encryptSecret(PLAIN);
|
||||
const data = Buffer.from(blob, 'base64');
|
||||
// authTag region: SALT[16] + IV[12] = offset 28, AUTH_TAG[16] = 28..43
|
||||
data[28] ^= 0xff;
|
||||
const tampered = data.toString('base64');
|
||||
expect(() => svc.decryptSecret(tampered)).toThrow(
|
||||
'APP_SECRET may have changed',
|
||||
);
|
||||
});
|
||||
|
||||
it('detects a flipped ciphertext byte and throws the APP_SECRET message', () => {
|
||||
const svc = makeService();
|
||||
const blob = svc.encryptSecret(PLAIN);
|
||||
const data = Buffer.from(blob, 'base64');
|
||||
// ciphertext region: SALT[16] + IV[12] + AUTH_TAG[16] = offset 44
|
||||
data[44] ^= 0xff;
|
||||
const tampered = data.toString('base64');
|
||||
expect(() => svc.decryptSecret(tampered)).toThrow(
|
||||
'APP_SECRET may have changed',
|
||||
);
|
||||
});
|
||||
|
||||
it('throws the APP_SECRET message when decrypted with a different APP_SECRET', () => {
|
||||
const encrypter = makeService('secret-a');
|
||||
const blob = encrypter.encryptSecret(PLAIN);
|
||||
const decrypter = makeService('secret-b');
|
||||
expect(() => decrypter.decryptSecret(blob)).toThrow(
|
||||
'APP_SECRET may have changed',
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user