fix: read semantic sources safely

This commit is contained in:
Andrey Avtomonov 2026-06-10 01:06:52 +02:00
parent 65de75ebd7
commit de1f1a8d5e
8 changed files with 292 additions and 66 deletions

View file

@ -15,6 +15,19 @@ describe('assertReadOnlySql', () => {
'Only read-only SELECT/WITH queries can be executed locally',
);
});
it('accepts read-only queries that begin with leading comments', () => {
expect(assertReadOnlySql('-- signups per day\nselect count(*) from public.signed_up')).toBe(
'select count(*) from public.signed_up',
);
expect(assertReadOnlySql('/* block */\n with paid as (select 1) select * from paid')).toContain('with paid');
});
it('still rejects mutating statements hidden behind leading comments', () => {
expect(() => assertReadOnlySql('-- harmless\n delete from orders')).toThrow(
'Only read-only SELECT/WITH queries can be executed locally',
);
});
});
describe('limitSqlForExecution', () => {
@ -27,4 +40,10 @@ describe('limitSqlForExecution', () => {
it('returns the trimmed SQL when no maxRows value is provided', () => {
expect(limitSqlForExecution('select * from orders; ', undefined)).toBe('select * from orders');
});
it('strips leading comments before wrapping with a row limit', () => {
expect(limitSqlForExecution('-- top customers\nselect * from public.orders', 25)).toBe(
'select * from (select * from public.orders) as ktx_query_result limit 25',
);
});
});

View file

@ -690,7 +690,54 @@ describe('createLocalProjectMcpContextPorts', () => {
});
});
it('rejects path traversal keys before touching the project directory', async () => {
it('reads manifest-backed sources with uppercase warehouse identifiers', async () => {
const project = await initKtxProject({ projectDir: tempDir });
await project.fileStore.writeFile(
'semantic-layer/warehouse/_schema/PUBLIC.yaml',
[
'tables:',
' SIGNED_UP:',
' table: PUBLIC.SIGNED_UP',
' columns:',
' - name: ID',
' type: number',
' pk: true',
'',
].join('\n'),
'ktx',
'ktx@example.com',
'seed uppercase manifest shard',
);
const ports = createLocalProjectMcpContextPorts(project, { embeddingService: null });
await expect(
ports.semanticLayer?.readSource({ connectionId: 'warehouse', sourceName: 'SIGNED_UP' }),
).resolves.toMatchObject({
sourceName: 'SIGNED_UP',
yaml: expect.stringContaining('table: PUBLIC.SIGNED_UP'),
});
});
it('returns a standalone source verbatim even when its YAML is currently broken', async () => {
const project = await initKtxProject({ projectDir: tempDir });
await project.fileStore.writeFile(
'semantic-layer/warehouse/orders.yaml',
'name: orders\nmeasures:\n - name: revenue\n expr: [unterminated\n',
'ktx',
'ktx@example.com',
'seed broken source mid-edit',
);
const ports = createLocalProjectMcpContextPorts(project, { embeddingService: null });
await expect(
ports.semanticLayer?.readSource({ connectionId: 'warehouse', sourceName: 'orders' }),
).resolves.toMatchObject({
sourceName: 'orders',
yaml: expect.stringContaining('[unterminated'),
});
});
it('keeps path-traversal keys away from the project directory', async () => {
const project = await initKtxProject({ projectDir: tempDir });
const ports = createLocalProjectMcpContextPorts(project, { embeddingService: null });
@ -701,12 +748,14 @@ describe('createLocalProjectMcpContextPorts', () => {
}),
).rejects.toThrow('Invalid wiki key "../outside". Wiki keys must be flat; use "outside".');
// Source reads never derive a file path from the name; a traversal-style
// name simply matches no record.
await expect(
ports.semanticLayer?.readSource({
connectionId: 'warehouse',
sourceName: '../orders',
}),
).rejects.toThrow('Unsafe semantic-layer source name');
).resolves.toBeNull();
});
it('uses semantic compute for compile-only sl_query when supplied', async () => {

View file

@ -261,6 +261,119 @@ describe('local semantic-layer helpers', () => {
);
});
it('reads manifest-backed scan sources whose warehouse identifiers are uppercase', async () => {
await project.fileStore.writeFile(
'semantic-layer/warehouse/_schema/PUBLIC.yaml',
`tables:
SIGNED_UP:
table: PUBLIC.SIGNED_UP
columns:
- name: ID
type: number
pk: true
- name: EMAIL
type: string
`,
'ktx',
'ktx@example.com',
'Add uppercase manifest shard',
);
await expect(readLocalSlSource(project, { connectionId: 'warehouse', sourceName: 'SIGNED_UP' })).resolves.toEqual(
expect.objectContaining({
connectionId: 'warehouse',
name: 'SIGNED_UP',
path: 'semantic-layer/warehouse/_schema/PUBLIC.yaml#SIGNED_UP',
yaml: expect.stringContaining('table: PUBLIC.SIGNED_UP'),
}),
);
});
it('reads manifest-backed sources whose names are not filename-safe', async () => {
// Snowflake and Postgres unquoted identifiers allow `$`; manifest keys
// carry the warehouse name verbatim, so the lookup must accept it.
await project.fileStore.writeFile(
'semantic-layer/warehouse/_schema/PUBLIC.yaml',
`tables:
EVENT$LOG:
table: PUBLIC.EVENT$LOG
columns:
- name: ID
type: number
pk: true
`,
'ktx',
'ktx@example.com',
'Add manifest shard with dollar-sign table name',
);
await expect(readLocalSlSource(project, { connectionId: 'warehouse', sourceName: 'EVENT$LOG' })).resolves.toEqual(
expect.objectContaining({
connectionId: 'warehouse',
name: 'EVENT$LOG',
path: 'semantic-layer/warehouse/_schema/PUBLIC.yaml#EVENT$LOG',
yaml: expect.stringContaining('table: PUBLIC.EVENT$LOG'),
}),
);
});
it('reads a manifest-backed source while a sibling standalone file has broken YAML', async () => {
await project.fileStore.writeFile(
'semantic-layer/warehouse/_schema/PUBLIC.yaml',
`tables:
SIGNED_UP:
table: PUBLIC.SIGNED_UP
columns:
- name: ID
type: number
pk: true
`,
'ktx',
'ktx@example.com',
'Add manifest shard',
);
await project.fileStore.writeFile(
'semantic-layer/warehouse/orders.yaml',
'name: orders\nmeasures:\n - name: revenue\n expr: [unterminated\n',
'ktx',
'ktx@example.com',
'seed a sibling source mid-edit with broken YAML',
);
await expect(readLocalSlSource(project, { connectionId: 'warehouse', sourceName: 'SIGNED_UP' })).resolves.toEqual(
expect.objectContaining({
name: 'SIGNED_UP',
yaml: expect.stringContaining('table: PUBLIC.SIGNED_UP'),
}),
);
// The broken sibling stays visible in listings instead of hiding or
// failing the whole connection.
await expect(listLocalSlSources(project, { connectionId: 'warehouse' })).resolves.toEqual([
expect.objectContaining({ name: 'orders', columnCount: 0 }),
expect.objectContaining({ name: 'SIGNED_UP', columnCount: 1 }),
]);
});
it('returns the raw YAML of a standalone source whose content no longer parses', async () => {
await project.fileStore.writeFile(
'semantic-layer/warehouse/orders.yaml',
'name: orders\nmeasures:\n - name: revenue\n expr: [unterminated\n',
'ktx',
'ktx@example.com',
'seed a source mid-edit with broken YAML',
);
await expect(readLocalSlSource(project, { connectionId: 'warehouse', sourceName: 'orders' })).resolves.toEqual(
expect.objectContaining({
connectionId: 'warehouse',
name: 'orders',
path: 'semantic-layer/warehouse/orders.yaml',
yaml: expect.stringContaining('[unterminated'),
}),
);
});
it('expands manifest-backed scan sources when listing all connections', async () => {
await project.fileStore.writeFile(
'semantic-layer/warehouse/_schema/public.yaml',
@ -506,12 +619,22 @@ describe('local semantic-layer helpers', () => {
});
});
it('rejects unsafe source paths', async () => {
it('never derives a file path from a traversal-style source name', async () => {
// Reads match names against loaded records, so a traversal-style name is
// simply not found; writes build a file path from the name and must throw.
await expect(
readLocalSlSource(project, {
connectionId: 'warehouse',
sourceName: '../orders',
}),
).resolves.toBeNull();
await expect(
writeLocalSlSource(project, {
connectionId: 'warehouse',
sourceName: '../orders',
yaml: ORDERS_YAML.replace('name: orders', 'name: ../orders'),
}),
).rejects.toThrow('Unsafe semantic-layer source name');
});
});