From e628ff4ad4a60c8ee5f94a8a04a98b12df367c2f Mon Sep 17 00:00:00 2001 From: lentil32 Date: Thu, 25 Jun 2026 20:43:51 +0900 Subject: [PATCH] fix(server): validate SHOW statement expressions --- apps/connector/src/jobs/validation.test.ts | 41 ++++++++++ apps/connector/src/jobs/validation.ts | 7 +- .../data-source-query/execute-query.test.ts | 2 +- .../data-source-query/validate-sql.test.ts | 44 ++++++++--- .../data-source-query/validate-sql.ts | 77 +++++++++++++++---- 5 files changed, 141 insertions(+), 30 deletions(-) create mode 100644 apps/connector/src/jobs/validation.test.ts diff --git a/apps/connector/src/jobs/validation.test.ts b/apps/connector/src/jobs/validation.test.ts new file mode 100644 index 00000000..15c2d3d8 --- /dev/null +++ b/apps/connector/src/jobs/validation.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from "vitest"; + +import { validateAthenaSql } from "./validation"; + +function expectValidSql(sql: string, expectedSql = sql) { + const result = validateAthenaSql(sql); + + expect(result.isOk()).toBe(true); + if (result.isErr()) { + throw result.error; + } + expect(result.value).toEqual({ sql: expectedSql }); +} + +function expectInvalidSql(sql: string, message: string) { + const result = validateAthenaSql(sql); + + expect(result.isErr()).toBe(true); + if (result.isOk()) { + throw new Error(`expected validation error for SQL: ${sql}`); + } + expect(result.error.message).toBe(message); +} + +describe("validateAthenaSql", () => { + it("allows read-only SHOW metadata statements", () => { + expectValidSql("SHOW TABLES"); + expectValidSql("SHOW CREATE TABLE orders"); + }); + + it("keeps rejecting mutable and multi-statement SQL", () => { + expectInvalidSql( + "CREATE TABLE copied AS SELECT 1", + "Only SELECT, WITH, or SHOW queries are allowed" + ); + expectInvalidSql( + "SHOW TABLES; DROP TABLE orders", + "Multiple statements are not allowed" + ); + }); +}); diff --git a/apps/connector/src/jobs/validation.ts b/apps/connector/src/jobs/validation.ts index 2a131188..0b1f4a2c 100644 --- a/apps/connector/src/jobs/validation.ts +++ b/apps/connector/src/jobs/validation.ts @@ -3,6 +3,7 @@ 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"]); class SqlValidationError extends TaggedError("SqlValidationError")<{ code: "INVALID_QUERY"; @@ -30,15 +31,15 @@ export function validateAthenaSql( } const firstKeyword = readFirstKeyword(normalized); - if (firstKeyword !== "SELECT" && firstKeyword !== "WITH") { - return invalid("Only SELECT queries are allowed"); + if (!READ_ONLY_FIRST_KEYWORDS.has(firstKeyword)) { + return invalid("Only SELECT, WITH, or SHOW queries are allowed"); } if (firstKeyword === "WITH" && !/\bSELECT\b/i.test(normalized)) { return invalid("WITH queries must eventually select rows"); } - if (FORBIDDEN_KEYWORDS.test(normalized)) { + if (firstKeyword !== "SHOW" && FORBIDDEN_KEYWORDS.test(normalized)) { return invalid("Query contains non-read operations"); } 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 b2cce9be..e7d9ace2 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 queries are allowed.", + "Only SELECT or SHOW 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 0f489461..6217c82d 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,6 +59,22 @@ describe("validateAndNormalizeReadOnlyQuery", () => { }); }); + it("allows read-only SHOW metadata statements", async () => { + await expectValidQuery("SHOW ALL", { sql: "SHOW ALL" }, "postgres"); + await expectValidQuery("SHOW TABLES", { sql: "SHOW TABLES" }, "mysql"); + await expectValidQuery( + "SHOW CREATE TABLE orders", + { sql: "SHOW CREATE TABLE orders" }, + "aws_athena_connector" + ); + await expectValidQuery( + "SHOW TABLES WHERE Tables_in_test LIKE 'user%'", + { sql: "SHOW TABLES WHERE Tables_in_test LIKE 'user%'" }, + "mysql" + ); + await expectValidQuery("SHOW TABLES", { sql: "SHOW TABLES" }, "laminar"); + }); + it("supports safe parenthesized selects", async () => { let sql = "(SELECT id FROM users)"; await expectValidQuery(sql, { @@ -82,7 +98,7 @@ describe("validateAndNormalizeReadOnlyQuery", () => { await expectValidationError( "DELETE FROM events", - "Only SELECT queries are allowed. Got: delete", + "Only SELECT or SHOW queries are allowed. Got: delete", "cloudflare_d1" ); }); @@ -98,7 +114,7 @@ describe("validateAndNormalizeReadOnlyQuery", () => { await expectValidationError( "CREATE TABLE copied AS SELECT 1", - "Only SELECT queries are allowed. Got: create_table", + "Only SELECT or SHOW queries are allowed. Got: create_table", "cloudflare_r2_sql" ); }); @@ -114,7 +130,7 @@ describe("validateAndNormalizeReadOnlyQuery", () => { await expectValidationError( "PUT file:///tmp/users.csv @analytics_stage", - "Only SELECT queries are allowed. Got: put", + "Only SELECT or SHOW queries are allowed. Got: put", "snowflake" ); }); @@ -130,7 +146,7 @@ describe("validateAndNormalizeReadOnlyQuery", () => { await expectValidationError( "CREATE TABLE copied AS SELECT 1", - "Only SELECT queries are allowed. Got: create_table", + "Only SELECT or SHOW queries are allowed. Got: create_table", "motherduck" ); }); @@ -157,7 +173,7 @@ describe("validateAndNormalizeReadOnlyQuery", () => { it("rejects unsafe or unsupported statements", async () => { await expectValidationError( "DELETE FROM users", - "Only SELECT queries are allowed. Got: delete" + "Only SELECT or SHOW queries are allowed. Got: delete" ); await expectValidationError( "SELECT 1; SELECT 2", @@ -176,15 +192,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 queries are allowed. Got: delete" + "Only SELECT or SHOW queries are allowed. Got: delete" ); await expectValidationError( "SELECT * FROM (INSERT INTO users(id) VALUES (1) RETURNING *) x", - "Only SELECT queries are allowed. Got: insert" + "Only SELECT or SHOW queries are allowed. Got: insert" ); await expectValidationError( "SELECT * FROM (CREATE TABLE new_users AS SELECT * FROM users) x", - "Only SELECT queries are allowed. Got: create_table" + "Only SELECT or SHOW queries are allowed. Got: create_table" ); await expectValidationError( "SELECT * FROM (SELECT * INTO new_users FROM users) x", @@ -248,9 +264,19 @@ describe("validateAndNormalizeReadOnlyQuery", () => { "Side-effecting SQL functions are not allowed: get_lock", "mysql" ); + await expectValidationError( + "SHOW TABLES WHERE GET_LOCK('x', 1)", + "Side-effecting SQL functions are not allowed: get_lock", + "mysql" + ); + await expectValidationError( + "SHOW TABLES WHERE SLEEP(10)", + "Side-effecting SQL functions are not allowed: sleep", + "mysql" + ); await expectValidationError( "SELECT @x := 1", - "Only SELECT queries are allowed. Got: property_e_q", + "Only SELECT or SHOW 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 b0e2c7bd..e38337cc 100644 --- a/packages/server/src/services/data-source-query/validate-sql.ts +++ b/packages/server/src/services/data-source-query/validate-sql.ts @@ -14,6 +14,8 @@ import type { Result as ResultType } from "better-result"; const OUTFILE_ERROR = "SELECT INTO OUTFILE/DUMPFILE is not allowed"; const ERRORS = { cteSelectOnly: "CTEs must only contain SELECT statements", + nonReadOnlyQuery: (kind: string) => + `Only SELECT or SHOW queries are allowed. Got: ${kind}`, unsafeFunction: (name: string) => `Side-effecting SQL functions are not allowed: ${name}`, locking: "SELECT locking clauses are not allowed", @@ -234,6 +236,7 @@ type ValidationContext = { type ParsedQuery = { statement: Expression; + statementKind: "query" | "show"; }; type Expression = PolyglotSdk.ast.Expression; @@ -409,7 +412,8 @@ async function parseStatements( } function extractParsedQuery( - statements: Expression[] + statements: Expression[], + context: ValidationContext ): ResultType { if (statements.length !== 1) { return invalid( @@ -424,15 +428,19 @@ function extractParsedQuery( } const query = unwrapParenthesizedQuery(statement); - if (!query || !isTopLevelQueryExpression(query)) { - const kind = getDeepestExpressionKind(statement); - return invalid( - "non_select_query", - `Only SELECT queries are allowed. Got: ${kind ?? "unknown"}` - ); + if (query && isTopLevelQueryExpression(query)) { + return Result.ok({ statement, statementKind: "query" }); + } + + if (isTopLevelShowExpression(statement, context)) { + return Result.ok({ statement, statementKind: "show" }); } - return Result.ok({ statement }); + const kind = getDeepestExpressionKind(statement); + return invalid( + "non_select_query", + ERRORS.nonReadOnlyQuery(kind ?? "unknown") + ); } function validateReadOnlyQuery( @@ -465,12 +473,9 @@ function validateReadOnlyQuery( return invalid("unsafe_function", unsafeFunctionViolation); } - const mutableKind = findSideEffectingExpressionKind(parsedQuery.statement); + const mutableKind = findSideEffectingExpressionKind(parsedQuery); if (mutableKind) { - return invalid( - "non_select_query", - `Only SELECT queries are allowed. Got: ${mutableKind}` - ); + return invalid("non_select_query", ERRORS.nonReadOnlyQuery(mutableKind)); } return Result.ok(null); @@ -546,6 +551,32 @@ function isTopLevelQueryExpression(expr: Expression): boolean { return polyglotAst.isSelect(expr) || polyglotAst.isSetOperation(expr); } +function isTopLevelShowExpression( + expr: Expression, + context: ValidationContext +): boolean { + const kind = getExpressionKind(expr); + if (kind === "show") { + return true; + } + + // Polyglot maps ClickHouse SHOW statements to generic command nodes. + return kind === "command" && firstTokenIs(context, "show"); +} + +function firstTokenIs(context: ValidationContext, keyword: string): boolean { + const tokenization = tokenize(context.trimmedSql, context.dialect); + if (!tokenization.success || !Array.isArray(tokenization.tokens)) { + return false; + } + + const firstToken = (tokenization.tokens as TokenInfo[]).find( + (token) => normalizeTokenText(token).length > 0 + ); + + return normalizeTokenText(firstToken) === keyword; +} + function isCteQueryExpression(expr: Expression): boolean { const unwrapped = unwrapParenthesizedQuery(expr); return Boolean(unwrapped && isTopLevelQueryExpression(unwrapped)); @@ -681,14 +712,26 @@ function isUnsafeFunctionName( return false; } -function findSideEffectingExpressionKind(query: Expression): string | null { - const mutableExpression = polyglotAst.findFirst(query, (expr) => - isSideEffectingExpression(expr) +function findSideEffectingExpressionKind( + parsedQuery: ParsedQuery +): string | null { + const mutableExpression = polyglotAst.findFirst( + parsedQuery.statement, + (expr) => + !isAllowedStatementRoot(expr, parsedQuery) && + isSideEffectingExpression(expr) ); return mutableExpression ? getExpressionKind(mutableExpression) : null; } +function isAllowedStatementRoot( + expr: Expression, + parsedQuery: ParsedQuery +): boolean { + return parsedQuery.statementKind === "show" && expr === parsedQuery.statement; +} + function isSideEffectingExpression(expr: Expression): boolean { return ( polyglotAst.isInsert(expr) || @@ -751,7 +794,7 @@ export async function validateAndNormalizeReadOnlyQuery( yield* Result.await(initSqlParser()); yield* validateStrictSyntax(context); const statements = yield* Result.await(parseStatements(context)); - const parsedQuery = yield* extractParsedQuery(statements); + const parsedQuery = yield* extractParsedQuery(statements, context); yield* validateReadOnlyQuery(parsedQuery, context.dbType); return finalizeSql(context); });