ktx/packages/cli/test/context/connections/read-only-sql.test.ts

133 lines
6 KiB
TypeScript
Raw Permalink Normal View History

2026-05-10 23:12:26 +02:00
import { describe, expect, it } from 'vitest';
import { KtxExpectedError, KtxQueryError } from '../../../src/errors.js';
fix: read semantic sources safely (#284) * fix: read semantic sources safely * test: retarget reindex per-scope error case to a broken manifest Reading a broken standalone source was made non-fatal in de1f1a8d (it is surfaced for repair instead of throwing), so the reindex per-scope error test no longer captured an error. Point it at a corrupt manifest shard, which is the remaining fatal read failure the per-scope catch must isolate, and assert the captured error names the offending file. * fix(sl): decouple semantic-layer file names from warehouse naming rules The in-file `name:` field is now the sole source identity; the filename is a derived label that never participates in identity. This removes the "Unsafe semantic-layer source name" failure class entirely: any warehouse identifier (Snowflake's uppercase SIGNED_UP, EVENT$LOG, dotted names) can be read, overlaid, edited, and deleted. - New `source-files.ts`: one total filename derivation (safe lowercase names verbatim; otherwise slug + sha256-hash suffix, immune to case-insensitive-filesystem collisions) and one by-name file resolver. - Reads resolve by name everywhere; the path-from-name fast path and `assertSafeSourceName` are gone. - Writes resolve-then-write: rewrites land on the file that declares the name (human renames survive); new sources get a derived filename; a derived path occupied by a different source fails instead of clobbering. - `readSourceFile` returns null for missing files instead of forcing every caller to launder IO errors; `deleteSource` distinguishes manifest-backed sources from not-found instead of silently succeeding. - `sl_write_source` accepts verbatim warehouse identifiers (snake_case is now a recommendation for new sources) and rejects sourceName/source.name mismatches; `sl_edit_source` rejects name-changing edits. - Ingest projection commits, gate-repair allowlists, and touched-source derivation use resolved paths / in-file names instead of interpolating `<connId>/<name>.yaml`. - Collapsed the five parallel path derivations and duplicated path-token helpers onto the shared module; dropped dead service methods. * fix(sl): resolve sources by declared name end-to-end and gate warehouse SQL with the parser-backed validator - Key broken/renamed semantic-layer files by their recoverable in-file name (slSourceNameForFile) so mid-edit sources stay reachable under their real identity in reads, listings, and search - Derive finalization touched sources from composed-source diffs and recover deleted files' declared names from the pre-change commit instead of parsing hash-derived filenames - Resolve revert/rollback paths against history (listFilesAtCommit) so human-renamed files are restored where they lived at preHead - Validate ingest sql_execution through the daemon's sqlglot validateReadOnly in the connection's dialect, sharing one driver-to-dialect map (sql-analysis/dialect.ts) across MCP and ingest - Harden the local read-only SQL backstop: accept leading comments, reject smuggled second statements, and strip trailing semicolons/comments before row-limit wrapping
2026-06-10 14:06:13 +02:00
import {
assertReadOnlySql,
limitSqlForExecution,
stripTrailingSqlNoise,
} from '../../../src/context/connections/read-only-sql.js';
2026-05-10 23:12:26 +02:00
describe('assertReadOnlySql', () => {
it('allows select and with queries', () => {
expect(assertReadOnlySql('select * from orders')).toBe('select * from orders');
expect(assertReadOnlySql('with paid as (select * from orders) select * from paid')).toContain('with paid');
});
it('rejects mutating statements before opening a database connection', () => {
expect(() => assertReadOnlySql('delete from orders')).toThrow(
'Only read-only SELECT/WITH queries can be executed locally',
);
expect(() => assertReadOnlySql('create table x(id int)')).toThrow(
'Only read-only SELECT/WITH queries can be executed locally',
);
});
fix: read semantic sources safely (#284) * fix: read semantic sources safely * test: retarget reindex per-scope error case to a broken manifest Reading a broken standalone source was made non-fatal in de1f1a8d (it is surfaced for repair instead of throwing), so the reindex per-scope error test no longer captured an error. Point it at a corrupt manifest shard, which is the remaining fatal read failure the per-scope catch must isolate, and assert the captured error names the offending file. * fix(sl): decouple semantic-layer file names from warehouse naming rules The in-file `name:` field is now the sole source identity; the filename is a derived label that never participates in identity. This removes the "Unsafe semantic-layer source name" failure class entirely: any warehouse identifier (Snowflake's uppercase SIGNED_UP, EVENT$LOG, dotted names) can be read, overlaid, edited, and deleted. - New `source-files.ts`: one total filename derivation (safe lowercase names verbatim; otherwise slug + sha256-hash suffix, immune to case-insensitive-filesystem collisions) and one by-name file resolver. - Reads resolve by name everywhere; the path-from-name fast path and `assertSafeSourceName` are gone. - Writes resolve-then-write: rewrites land on the file that declares the name (human renames survive); new sources get a derived filename; a derived path occupied by a different source fails instead of clobbering. - `readSourceFile` returns null for missing files instead of forcing every caller to launder IO errors; `deleteSource` distinguishes manifest-backed sources from not-found instead of silently succeeding. - `sl_write_source` accepts verbatim warehouse identifiers (snake_case is now a recommendation for new sources) and rejects sourceName/source.name mismatches; `sl_edit_source` rejects name-changing edits. - Ingest projection commits, gate-repair allowlists, and touched-source derivation use resolved paths / in-file names instead of interpolating `<connId>/<name>.yaml`. - Collapsed the five parallel path derivations and duplicated path-token helpers onto the shared module; dropped dead service methods. * fix(sl): resolve sources by declared name end-to-end and gate warehouse SQL with the parser-backed validator - Key broken/renamed semantic-layer files by their recoverable in-file name (slSourceNameForFile) so mid-edit sources stay reachable under their real identity in reads, listings, and search - Derive finalization touched sources from composed-source diffs and recover deleted files' declared names from the pre-change commit instead of parsing hash-derived filenames - Resolve revert/rollback paths against history (listFilesAtCommit) so human-renamed files are restored where they lived at preHead - Validate ingest sql_execution through the daemon's sqlglot validateReadOnly in the connection's dialect, sharing one driver-to-dialect map (sql-analysis/dialect.ts) across MCP and ingest - Harden the local read-only SQL backstop: accept leading comments, reject smuggled second statements, and strip trailing semicolons/comments before row-limit wrapping
2026-06-10 14:06:13 +02:00
// A guard refusing the agent's SQL is an expected outcome; classifying it as
// KtxQueryError keeps reportException from filing it as a ktx fault.
it('rejects with an expected KtxQueryError, not a bare Error', () => {
expect(() => assertReadOnlySql('delete from orders')).toThrow(KtxQueryError);
expect(() => assertReadOnlySql('describe orders')).toThrow(KtxQueryError);
expect(() => assertReadOnlySql('select 1; drop table orders')).toThrow(KtxQueryError);
try {
assertReadOnlySql('describe orders');
expect.unreachable('expected a throw');
} catch (error) {
expect(error).toBeInstanceOf(KtxExpectedError);
}
});
fix: read semantic sources safely (#284) * fix: read semantic sources safely * test: retarget reindex per-scope error case to a broken manifest Reading a broken standalone source was made non-fatal in de1f1a8d (it is surfaced for repair instead of throwing), so the reindex per-scope error test no longer captured an error. Point it at a corrupt manifest shard, which is the remaining fatal read failure the per-scope catch must isolate, and assert the captured error names the offending file. * fix(sl): decouple semantic-layer file names from warehouse naming rules The in-file `name:` field is now the sole source identity; the filename is a derived label that never participates in identity. This removes the "Unsafe semantic-layer source name" failure class entirely: any warehouse identifier (Snowflake's uppercase SIGNED_UP, EVENT$LOG, dotted names) can be read, overlaid, edited, and deleted. - New `source-files.ts`: one total filename derivation (safe lowercase names verbatim; otherwise slug + sha256-hash suffix, immune to case-insensitive-filesystem collisions) and one by-name file resolver. - Reads resolve by name everywhere; the path-from-name fast path and `assertSafeSourceName` are gone. - Writes resolve-then-write: rewrites land on the file that declares the name (human renames survive); new sources get a derived filename; a derived path occupied by a different source fails instead of clobbering. - `readSourceFile` returns null for missing files instead of forcing every caller to launder IO errors; `deleteSource` distinguishes manifest-backed sources from not-found instead of silently succeeding. - `sl_write_source` accepts verbatim warehouse identifiers (snake_case is now a recommendation for new sources) and rejects sourceName/source.name mismatches; `sl_edit_source` rejects name-changing edits. - Ingest projection commits, gate-repair allowlists, and touched-source derivation use resolved paths / in-file names instead of interpolating `<connId>/<name>.yaml`. - Collapsed the five parallel path derivations and duplicated path-token helpers onto the shared module; dropped dead service methods. * fix(sl): resolve sources by declared name end-to-end and gate warehouse SQL with the parser-backed validator - Key broken/renamed semantic-layer files by their recoverable in-file name (slSourceNameForFile) so mid-edit sources stay reachable under their real identity in reads, listings, and search - Derive finalization touched sources from composed-source diffs and recover deleted files' declared names from the pre-change commit instead of parsing hash-derived filenames - Resolve revert/rollback paths against history (listFilesAtCommit) so human-renamed files are restored where they lived at preHead - Validate ingest sql_execution through the daemon's sqlglot validateReadOnly in the connection's dialect, sharing one driver-to-dialect map (sql-analysis/dialect.ts) across MCP and ingest - Harden the local read-only SQL backstop: accept leading comments, reject smuggled second statements, and strip trailing semicolons/comments before row-limit wrapping
2026-06-10 14:06:13 +02:00
it('accepts read-only queries that begin with leading comments', () => {
expect(assertReadOnlySql('-- daily widget sales\nselect count(*) from public.widget_sales')).toBe(
'select count(*) from public.widget_sales',
);
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',
);
});
it('rejects a second statement smuggled after a semicolon', () => {
expect(() => assertReadOnlySql('select 1; drop table orders')).toThrow(
'Only one SQL statement can be executed.',
);
expect(() => assertReadOnlySql('select 1;\n-- pad\ndelete from orders')).toThrow(
'Only one SQL statement can be executed.',
);
expect(() => assertReadOnlySql('select 1; /* pad */ truncate orders;')).toThrow(
'Only one SQL statement can be executed.',
);
});
it('accepts trailing semicolons, including repeated ones followed by comments', () => {
expect(assertReadOnlySql('select 1;')).toBe('select 1;');
expect(assertReadOnlySql('select 1 ;; \n')).toBe('select 1 ;;');
expect(assertReadOnlySql('select 1; -- done')).toBe('select 1; -- done');
});
it('ignores semicolons inside string literals, quoted identifiers, and comments', () => {
expect(assertReadOnlySql("select string_agg(name, '; ') from t")).toBe("select string_agg(name, '; ') from t");
expect(assertReadOnlySql("select 'it''s; quoted' from t")).toBe("select 'it''s; quoted' from t");
expect(assertReadOnlySql('select ";" from "t;u"')).toBe('select ";" from "t;u"');
expect(assertReadOnlySql('select 1 -- tail; comment')).toBe('select 1 -- tail; comment');
expect(assertReadOnlySql('select 1 /* a;b */ + 2')).toBe('select 1 /* a;b */ + 2');
});
it('rejects statements smuggled after a string literal that closes a semicolon early', () => {
expect(() => assertReadOnlySql("select 'a'; delete from orders")).toThrow(
'Only one SQL statement can be executed.',
);
});
2026-05-10 23:12:26 +02:00
});
describe('limitSqlForExecution', () => {
it('wraps compiled SQL and strips trailing semicolons', () => {
expect(limitSqlForExecution('select * from public.orders; ', 25)).toBe(
2026-05-10 23:51:24 +02:00
'select * from (select * from public.orders) as ktx_query_result limit 25',
2026-05-10 23:12:26 +02:00
);
});
it('returns the trimmed SQL when no maxRows value is provided', () => {
expect(limitSqlForExecution('select * from orders; ', undefined)).toBe('select * from orders');
});
fix: read semantic sources safely (#284) * fix: read semantic sources safely * test: retarget reindex per-scope error case to a broken manifest Reading a broken standalone source was made non-fatal in de1f1a8d (it is surfaced for repair instead of throwing), so the reindex per-scope error test no longer captured an error. Point it at a corrupt manifest shard, which is the remaining fatal read failure the per-scope catch must isolate, and assert the captured error names the offending file. * fix(sl): decouple semantic-layer file names from warehouse naming rules The in-file `name:` field is now the sole source identity; the filename is a derived label that never participates in identity. This removes the "Unsafe semantic-layer source name" failure class entirely: any warehouse identifier (Snowflake's uppercase SIGNED_UP, EVENT$LOG, dotted names) can be read, overlaid, edited, and deleted. - New `source-files.ts`: one total filename derivation (safe lowercase names verbatim; otherwise slug + sha256-hash suffix, immune to case-insensitive-filesystem collisions) and one by-name file resolver. - Reads resolve by name everywhere; the path-from-name fast path and `assertSafeSourceName` are gone. - Writes resolve-then-write: rewrites land on the file that declares the name (human renames survive); new sources get a derived filename; a derived path occupied by a different source fails instead of clobbering. - `readSourceFile` returns null for missing files instead of forcing every caller to launder IO errors; `deleteSource` distinguishes manifest-backed sources from not-found instead of silently succeeding. - `sl_write_source` accepts verbatim warehouse identifiers (snake_case is now a recommendation for new sources) and rejects sourceName/source.name mismatches; `sl_edit_source` rejects name-changing edits. - Ingest projection commits, gate-repair allowlists, and touched-source derivation use resolved paths / in-file names instead of interpolating `<connId>/<name>.yaml`. - Collapsed the five parallel path derivations and duplicated path-token helpers onto the shared module; dropped dead service methods. * fix(sl): resolve sources by declared name end-to-end and gate warehouse SQL with the parser-backed validator - Key broken/renamed semantic-layer files by their recoverable in-file name (slSourceNameForFile) so mid-edit sources stay reachable under their real identity in reads, listings, and search - Derive finalization touched sources from composed-source diffs and recover deleted files' declared names from the pre-change commit instead of parsing hash-derived filenames - Resolve revert/rollback paths against history (listFilesAtCommit) so human-renamed files are restored where they lived at preHead - Validate ingest sql_execution through the daemon's sqlglot validateReadOnly in the connection's dialect, sharing one driver-to-dialect map (sql-analysis/dialect.ts) across MCP and ingest - Harden the local read-only SQL backstop: accept leading comments, reject smuggled second statements, and strip trailing semicolons/comments before row-limit wrapping
2026-06-10 14:06:13 +02:00
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',
);
});
it('drops a trailing semicolon followed by a comment so the subquery stays valid', () => {
// The single-statement gate accepts `select 1; -- done`; without stripping
// the terminator the wrapper would embed `select 1; -- done` and comment out
// the closing paren and limit clause.
expect(limitSqlForExecution('select 1; -- done', 5)).toBe(
'select * from (select 1) as ktx_query_result limit 5',
);
expect(limitSqlForExecution('select 1; /* note */', 5)).toBe(
'select * from (select 1) as ktx_query_result limit 5',
);
});
it('drops a trailing line comment with no semicolon before wrapping', () => {
expect(limitSqlForExecution('select 1 -- done', 5)).toBe('select * from (select 1) as ktx_query_result limit 5');
});
});
describe('stripTrailingSqlNoise', () => {
it('removes trailing semicolons, comments, and whitespace', () => {
expect(stripTrailingSqlNoise('select 1;')).toBe('select 1');
expect(stripTrailingSqlNoise('select 1 ;; ')).toBe('select 1');
expect(stripTrailingSqlNoise('select 1; -- done')).toBe('select 1');
expect(stripTrailingSqlNoise('select 1 -- done')).toBe('select 1');
expect(stripTrailingSqlNoise('select 1; /* trailing */')).toBe('select 1');
});
it('preserves semicolons and comment markers inside literals and mid-statement', () => {
expect(stripTrailingSqlNoise("select 'a; -- b'")).toBe("select 'a; -- b'");
expect(stripTrailingSqlNoise('select 1 /* a;b */ + 2')).toBe('select 1 /* a;b */ + 2');
expect(stripTrailingSqlNoise('select ";" from "t;u"')).toBe('select ";" from "t;u"');
});
2026-05-10 23:12:26 +02:00
});