diff --git a/apps/connector/src/jobs/validation.test.ts b/apps/connector/src/jobs/validation.test.ts index 15c2d3d8..41602d8e 100644 --- a/apps/connector/src/jobs/validation.test.ts +++ b/apps/connector/src/jobs/validation.test.ts @@ -23,15 +23,23 @@ function expectInvalidSql(sql: string, message: string) { } describe("validateAthenaSql", () => { - it("allows read-only SHOW metadata statements", () => { + it("allows read-only metadata statements", () => { expectValidSql("SHOW TABLES"); expectValidSql("SHOW CREATE TABLE orders"); + expectValidSql("DESCRIBE orders"); + expectValidSql("DESC orders"); + expectValidSql("EXPLAIN SELECT * FROM orders"); + expectValidSql("EXPLAIN ANALYZE SELECT * FROM orders"); }); it("keeps rejecting mutable and multi-statement SQL", () => { expectInvalidSql( "CREATE TABLE copied AS SELECT 1", - "Only SELECT, WITH, or SHOW queries are allowed" + "Only SELECT, WITH, SHOW, DESCRIBE, or EXPLAIN queries are allowed" + ); + expectInvalidSql( + "EXPLAIN INSERT INTO orders VALUES (1)", + "Query contains non-read operations" ); expectInvalidSql( "SHOW TABLES; DROP TABLE orders", diff --git a/apps/connector/src/jobs/validation.ts b/apps/connector/src/jobs/validation.ts index 0b1f4a2c..b86dd4e7 100644 --- a/apps/connector/src/jobs/validation.ts +++ b/apps/connector/src/jobs/validation.ts @@ -3,7 +3,18 @@ import type { Result as ResultType } from "better-result"; const FORBIDDEN_KEYWORDS = /\b(INSERT|UPDATE|DELETE|MERGE|CREATE|DROP|ALTER|TRUNCATE|GRANT|REVOKE|CALL|UNLOAD|COPY|MSCK|VACUUM|ANALYZE|OPTIMIZE)\b/i; -const READ_ONLY_FIRST_KEYWORDS = new Set(["SELECT", "WITH", "SHOW"]); +const FORBIDDEN_EXPLAIN_KEYWORDS = + /\b(INSERT|UPDATE|DELETE|MERGE|CREATE|DROP|ALTER|TRUNCATE|GRANT|REVOKE|CALL|UNLOAD|COPY|MSCK|VACUUM|OPTIMIZE)\b/i; +const READ_ONLY_KEYWORD_ERROR = + "Only SELECT, WITH, SHOW, DESCRIBE, or EXPLAIN queries are allowed"; +const READ_ONLY_FIRST_KEYWORDS = new Set([ + "SELECT", + "WITH", + "SHOW", + "DESCRIBE", + "DESC", + "EXPLAIN", +]); class SqlValidationError extends TaggedError("SqlValidationError")<{ code: "INVALID_QUERY"; @@ -32,17 +43,26 @@ export function validateAthenaSql( const firstKeyword = readFirstKeyword(normalized); if (!READ_ONLY_FIRST_KEYWORDS.has(firstKeyword)) { - return invalid("Only SELECT, WITH, or SHOW queries are allowed"); + return invalid(READ_ONLY_KEYWORD_ERROR); } if (firstKeyword === "WITH" && !/\bSELECT\b/i.test(normalized)) { return invalid("WITH queries must eventually select rows"); } - if (firstKeyword !== "SHOW" && FORBIDDEN_KEYWORDS.test(normalized)) { + const forbiddenKeywords = + firstKeyword === "EXPLAIN" + ? FORBIDDEN_EXPLAIN_KEYWORDS + : FORBIDDEN_KEYWORDS; + + if (firstKeyword !== "SHOW" && forbiddenKeywords.test(normalized)) { return invalid("Query contains non-read operations"); } + if (firstKeyword === "EXPLAIN" && !/\bSELECT\b/i.test(normalized)) { + return invalid("EXPLAIN queries must explain SELECT statements"); + } + return Result.ok({ sql: trimmed, }); diff --git a/packages/server/src/services/data-source-query/execute-query.test.ts b/packages/server/src/services/data-source-query/execute-query.test.ts index 11331c86..3fe22569 100644 --- a/packages/server/src/services/data-source-query/execute-query.test.ts +++ b/packages/server/src/services/data-source-query/execute-query.test.ts @@ -107,7 +107,7 @@ describe("data source query execution", () => { }, sql: "DELETE FROM users", }), - "Only SELECT or SHOW queries are allowed.", + "Only SELECT, SHOW, DESCRIBE, or EXPLAIN queries are allowed.", ], [ "invalid BigQuery locations", diff --git a/packages/server/src/services/data-source-query/validate-sql.test.ts b/packages/server/src/services/data-source-query/validate-sql.test.ts index 02b84965..48e7e14d 100644 --- a/packages/server/src/services/data-source-query/validate-sql.test.ts +++ b/packages/server/src/services/data-source-query/validate-sql.test.ts @@ -59,7 +59,7 @@ describe("validateAndNormalizeReadOnlyQuery", () => { }); }); - it("allows read-only SHOW metadata statements", async () => { + it("allows read-only metadata statements", async () => { await expectValidQuery("SHOW ALL", { sql: "SHOW ALL" }, "postgres"); await expectValidQuery("SHOW TABLES", { sql: "SHOW TABLES" }, "mysql"); await expectValidQuery( @@ -73,6 +73,22 @@ describe("validateAndNormalizeReadOnlyQuery", () => { "mysql" ); await expectValidQuery("SHOW TABLES", { sql: "SHOW TABLES" }, "laminar"); + await expectValidQuery( + "DESCRIBE users", + { sql: "DESCRIBE users" }, + "mysql" + ); + await expectValidQuery("DESC users", { sql: "DESC users" }, "mysql"); + await expectValidQuery( + "EXPLAIN SELECT * FROM users", + { sql: "EXPLAIN SELECT * FROM users" }, + "postgres" + ); + await expectValidQuery( + "EXPLAIN ANALYZE SELECT * FROM users", + { sql: "EXPLAIN ANALYZE SELECT * FROM users" }, + "postgres" + ); }); it("supports safe parenthesized selects", async () => { @@ -98,7 +114,7 @@ describe("validateAndNormalizeReadOnlyQuery", () => { await expectValidationError( "DELETE FROM events", - "Only SELECT or SHOW queries are allowed. Got: delete", + "Only SELECT, SHOW, DESCRIBE, or EXPLAIN queries are allowed. Got: delete", "cloudflare_d1" ); }); @@ -114,7 +130,7 @@ describe("validateAndNormalizeReadOnlyQuery", () => { await expectValidationError( "CREATE TABLE copied AS SELECT 1", - "Only SELECT, SHOW, or DESCRIBE queries are allowed. Got: create_table", + "Only SELECT, SHOW, DESCRIBE, or EXPLAIN queries are allowed. Got: create_table", "cloudflare_r2_sql" ); }); @@ -129,6 +145,8 @@ describe("validateAndNormalizeReadOnlyQuery", () => { "DESCRIBE default.transactions", "DESCRIBE TABLE default.transactions", "DESC default.transactions", + "EXPLAIN SELECT * FROM default.transactions", + "EXPLAIN ANALYZE SELECT * FROM default.transactions", ]; for (const sql of metadataQueries) { @@ -142,14 +160,6 @@ describe("validateAndNormalizeReadOnlyQuery", () => { } }); - it("does not allow non-DESCRIBE statements that parse as R2 SQL describe expressions", async () => { - await expectValidationError( - "EXPLAIN SELECT 1", - "Only SELECT, SHOW, or DESCRIBE queries are allowed. Got: describe", - "cloudflare_r2_sql" - ); - }); - it("validates Snowflake queries with the Snowflake dialect", async () => { await expectValidQuery( "SELECT id FROM events QUALIFY ROW_NUMBER() OVER (ORDER BY created_at DESC) <= 10", @@ -161,7 +171,7 @@ describe("validateAndNormalizeReadOnlyQuery", () => { await expectValidationError( "PUT file:///tmp/users.csv @analytics_stage", - "Only SELECT or SHOW queries are allowed. Got: put", + "Only SELECT, SHOW, DESCRIBE, or EXPLAIN queries are allowed. Got: put", "snowflake" ); }); @@ -177,7 +187,7 @@ describe("validateAndNormalizeReadOnlyQuery", () => { await expectValidationError( "CREATE TABLE copied AS SELECT 1", - "Only SELECT or SHOW queries are allowed. Got: create_table", + "Only SELECT, SHOW, DESCRIBE, or EXPLAIN queries are allowed. Got: create_table", "motherduck" ); }); @@ -204,7 +214,7 @@ describe("validateAndNormalizeReadOnlyQuery", () => { it("rejects unsafe or unsupported statements", async () => { await expectValidationError( "DELETE FROM users", - "Only SELECT or SHOW queries are allowed. Got: delete" + "Only SELECT, SHOW, DESCRIBE, or EXPLAIN queries are allowed. Got: delete" ); await expectValidationError( "SELECT 1; SELECT 2", @@ -223,15 +233,15 @@ describe("validateAndNormalizeReadOnlyQuery", () => { it("rejects mutable or side-effecting constructs anywhere in the tree", async () => { await expectValidationError( "SELECT * FROM (DELETE FROM users RETURNING *) x", - "Only SELECT or SHOW queries are allowed. Got: delete" + "Only SELECT, SHOW, DESCRIBE, or EXPLAIN queries are allowed. Got: delete" ); await expectValidationError( "SELECT * FROM (INSERT INTO users(id) VALUES (1) RETURNING *) x", - "Only SELECT or SHOW queries are allowed. Got: insert" + "Only SELECT, SHOW, DESCRIBE, or EXPLAIN queries are allowed. Got: insert" ); await expectValidationError( "SELECT * FROM (CREATE TABLE new_users AS SELECT * FROM users) x", - "Only SELECT or SHOW queries are allowed. Got: create_table" + "Only SELECT, SHOW, DESCRIBE, or EXPLAIN queries are allowed. Got: create_table" ); await expectValidationError( "SELECT * FROM (SELECT * INTO new_users FROM users) x", @@ -310,9 +320,19 @@ describe("validateAndNormalizeReadOnlyQuery", () => { "Side-effecting SQL functions are not allowed: sleep", "mysql" ); + await expectValidationError( + "DESCRIBE SELECT SLEEP(10)", + "Side-effecting SQL functions are not allowed: sleep", + "mysql" + ); + await expectValidationError( + "EXPLAIN SELECT GET_LOCK('x', 1)", + "Side-effecting SQL functions are not allowed: get_lock", + "mysql" + ); await expectValidationError( "SELECT @x := 1", - "Only SELECT or SHOW queries are allowed. Got: property_e_q", + "Only SELECT, SHOW, DESCRIBE, or EXPLAIN queries are allowed. Got: property_e_q", "mysql" ); await expectValidationError( diff --git a/packages/server/src/services/data-source-query/validate-sql.ts b/packages/server/src/services/data-source-query/validate-sql.ts index 8aab3c95..d8499333 100644 --- a/packages/server/src/services/data-source-query/validate-sql.ts +++ b/packages/server/src/services/data-source-query/validate-sql.ts @@ -232,9 +232,19 @@ type ValidationContext = { dialect: (typeof DIALECT_MAP)[DatabaseCredentialProviderType]; }; +type MetadataStatementKind = "describe" | "explain" | "show"; +type ReadOnlyStatementKind = "query" | MetadataStatementKind; type ParsedQuery = { statement: Expression; - statementKind: "describe" | "query" | "show"; + statementKind: ReadOnlyStatementKind; +}; + +type MetadataStatementDefinition = { + allowOpaqueCommand: boolean; + displayName: string; + expressionKinds: readonly string[]; + firstKeywords: readonly string[]; + statementKind: MetadataStatementKind; }; type Expression = PolyglotSdk.ast.Expression; @@ -277,6 +287,33 @@ type CteData = ExpressionRecord & { this?: Expression; }; +const DEFAULT_READ_ONLY_METADATA_STATEMENTS = [ + { + allowOpaqueCommand: true, + displayName: "SHOW", + expressionKinds: ["show"], + firstKeywords: ["show"], + statementKind: "show", + }, + { + allowOpaqueCommand: true, + displayName: "DESCRIBE", + expressionKinds: ["describe"], + firstKeywords: ["describe", "desc"], + statementKind: "describe", + }, + { + allowOpaqueCommand: false, + displayName: "EXPLAIN", + expressionKinds: ["describe"], + firstKeywords: ["explain"], + statementKind: "explain", + }, +] as const satisfies readonly MetadataStatementDefinition[]; +const READ_ONLY_METADATA_STATEMENT_OVERRIDES = {} as Partial< + Record +>; + function getSelectIntoError(dbType: DatabaseCredentialProviderType): Violation { return dbType === "mysql" ? OUTFILE_ERROR : ERRORS.selectInto; } @@ -430,12 +467,12 @@ function extractParsedQuery( return Result.ok({ statement, statementKind: "query" }); } - if (isTopLevelShowExpression(statement, context)) { - return Result.ok({ statement, statementKind: "show" }); - } - - if (isTopLevelDescribeExpression(statement, context)) { - return Result.ok({ statement, statementKind: "describe" }); + const metadataStatement = findReadOnlyMetadataStatement(statement, context); + if (metadataStatement) { + return Result.ok({ + statement, + statementKind: metadataStatement.statementKind, + }); } const kind = getDeepestExpressionKind(statement); @@ -490,13 +527,31 @@ function buildNonReadOnlyQueryError( context: ValidationContext, kind: string ): string { - const allowed = - context.dbType === "cloudflare_r2_sql" - ? "SELECT, SHOW, or DESCRIBE" - : "SELECT or SHOW"; + const allowed = formatAllowedStatementList( + getAllowedStatementDisplayNames(context) + ); return `Only ${allowed} queries are allowed. Got: ${kind}`; } +function getAllowedStatementDisplayNames( + context: ValidationContext +): readonly string[] { + return [ + "SELECT", + ...getAllowedMetadataStatementDefinitions(context).map( + (definition) => definition.displayName + ), + ]; +} + +function formatAllowedStatementList(names: readonly string[]): string { + if (names.length <= 2) { + return names.join(" or "); + } + + return `${names.slice(0, -1).join(", ")}, or ${names.at(-1)}`; +} + function finalizeSql(context: ValidationContext): ValidationResult { return Result.ok({ sql: context.trimmedSql }); } @@ -567,47 +622,55 @@ function isTopLevelQueryExpression(expr: Expression): boolean { return polyglotAst.isSelect(expr) || polyglotAst.isSetOperation(expr); } -function isTopLevelShowExpression( +function findReadOnlyMetadataStatement( expr: Expression, context: ValidationContext -): boolean { - const kind = getExpressionKind(expr); - if (kind === "show") { - return true; +): MetadataStatementDefinition | null { + const expressionKind = getExpressionKind(expr); + const firstKeyword = readFirstKeyword(context); + + if (!firstKeyword) { + return null; } - // Polyglot maps ClickHouse SHOW statements to generic command nodes. - return kind === "command" && firstTokenIs(context, "show"); -} + return ( + getAllowedMetadataStatementDefinitions(context).find((definition) => { + if (!definition.firstKeywords.includes(firstKeyword)) { + return false; + } -function isTopLevelDescribeExpression( - expr: Expression, - context: ValidationContext -): boolean { - if (context.dbType !== "cloudflare_r2_sql") { - return false; - } + if (definition.expressionKinds.includes(expressionKind)) { + return true; + } - if (getExpressionKind(expr) !== "describe") { - return false; - } + // Polyglot maps some dialect-specific metadata statements, such as + // ClickHouse SHOW/DESCRIBE, to opaque command nodes. + return definition.allowOpaqueCommand && expressionKind === "command"; + }) ?? null + ); +} - // Polyglot Athena currently parses EXPLAIN SELECT as a describe expression. - // Require a source DESCRIBE/DESC keyword before allowing it. - return firstTokenIs(context, "describe") || firstTokenIs(context, "desc"); +function getAllowedMetadataStatementDefinitions( + context: ValidationContext +): readonly MetadataStatementDefinition[] { + return ( + READ_ONLY_METADATA_STATEMENT_OVERRIDES[context.dbType] ?? + DEFAULT_READ_ONLY_METADATA_STATEMENTS + ); } -function firstTokenIs(context: ValidationContext, keyword: string): boolean { +function readFirstKeyword(context: ValidationContext): string | null { const tokenization = tokenize(context.trimmedSql, context.dialect); if (!tokenization.success || !Array.isArray(tokenization.tokens)) { - return false; + return null; } const firstToken = (tokenization.tokens as TokenInfo[]).find( (token) => normalizeTokenText(token).length > 0 ); - return normalizeTokenText(firstToken) === keyword; + const keyword = normalizeTokenText(firstToken); + return keyword.length > 0 ? keyword : null; } function isCteQueryExpression(expr: Expression): boolean {