From e6a7f3beef14e8333c4ce1d35da908ef2340ac22 Mon Sep 17 00:00:00 2001 From: siisee11 Date: Fri, 26 Jun 2026 09:52:01 +0900 Subject: [PATCH] fix(server): allow R2 SQL metadata queries --- .../data-source-query/execute-query.test.ts | 43 +++++++++++++++ .../data-source-query/validate-sql.test.ts | 33 ++++++++++- .../data-source-query/validate-sql.ts | 55 +++++++++++++++---- 3 files changed, 120 insertions(+), 11 deletions(-) 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 e7d9ace2..11331c86 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 @@ -298,6 +298,49 @@ describe("data source query execution", () => { }); }); + it("executes Cloudflare R2 SQL metadata queries after validation", async () => { + const fetchSpy = vi.fn(async (_url: string | URL, _init?: RequestInit) => ({ + json: async () => ({ + errors: [], + messages: [], + result: { + metrics: { + bytes_scanned: 0, + files_scanned: 0, + r2_requests_count: 1, + }, + rows: [{ name: "transactions" }], + schema: [{ name: "name", type: "Utf8" }], + }, + success: true, + }), + ok: true, + status: 200, + text: async () => "", + })); + globalThis.fetch = fetchSpy as unknown as typeof fetch; + + const rows = unwrapQueryResult( + await executeDatabaseQuery({ + credentials: { + accountId: "acct_123", + apiToken: "cf-r2-sql-token", + bucketName: "analytics-events", + type: "cloudflare_r2_sql", + }, + sql: "SHOW TABLES", + }) + ); + + expect(rows).toEqual([{ name: "transactions" }]); + expect(fetchSpy).toHaveBeenCalledTimes(1); + const [, init] = fetchSpy.mock.calls[0] ?? []; + expect(JSON.parse(init?.body as string)).toEqual({ + query: "SHOW TABLES", + warehouse: "acct_123_analytics-events", + }); + }); + it("tests Cloudflare R2 SQL connections with a catalog metadata command", async () => { const fetchSpy = vi.fn(async (_url: string | URL, _init?: RequestInit) => ({ json: async () => ({ 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 6217c82d..5c22a9b9 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 @@ -114,7 +114,38 @@ describe("validateAndNormalizeReadOnlyQuery", () => { await expectValidationError( "CREATE TABLE copied AS SELECT 1", - "Only SELECT or SHOW queries are allowed. Got: create_table", + "Only SELECT, SHOW, or DESCRIBE queries are allowed. Got: create_table", + "cloudflare_r2_sql" + ); + }); + + it("allows Cloudflare R2 SQL metadata commands", async () => { + const metadataQueries = [ + "SHOW DATABASES", + "SHOW SCHEMAS", + "SHOW TABLES", + "SHOW TABLES IN default", + "SHOW COLUMNS FROM default.transactions", + "DESCRIBE default.transactions", + "DESCRIBE TABLE default.transactions", + "DESC default.transactions", + ]; + + for (const sql of metadataQueries) { + await expectValidQuery( + sql, + { + sql, + }, + "cloudflare_r2_sql" + ); + } + }); + + 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" ); }); 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 e38337cc..ffe1104c 100644 --- a/packages/server/src/services/data-source-query/validate-sql.ts +++ b/packages/server/src/services/data-source-query/validate-sql.ts @@ -14,8 +14,6 @@ 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", @@ -236,7 +234,7 @@ type ValidationContext = { type ParsedQuery = { statement: Expression; - statementKind: "query" | "show"; + statementKind: "describe" | "query" | "show"; }; type Expression = PolyglotSdk.ast.Expression; @@ -436,16 +434,20 @@ function extractParsedQuery( return Result.ok({ statement, statementKind: "show" }); } + if (isTopLevelDescribeExpression(statement, context)) { + return Result.ok({ statement, statementKind: "describe" }); + } + const kind = getDeepestExpressionKind(statement); return invalid( "non_select_query", - ERRORS.nonReadOnlyQuery(kind ?? "unknown") + buildNonReadOnlyQueryError(context, kind ?? "unknown") ); } function validateReadOnlyQuery( parsedQuery: ParsedQuery, - dbType: DatabaseCredentialProviderType + context: ValidationContext ): ResultType { const cteViolation = findCteViolation(parsedQuery.statement); if (cteViolation) { @@ -454,7 +456,7 @@ function validateReadOnlyQuery( const selectIntoViolation = findSelectIntoViolationInQuery( parsedQuery.statement, - dbType + context.dbType ); if (selectIntoViolation) { return invalid("select_into_not_allowed", selectIntoViolation); @@ -467,7 +469,7 @@ function validateReadOnlyQuery( const unsafeFunctionViolation = findUnsafeFunctionViolation( parsedQuery.statement, - dbType + context.dbType ); if (unsafeFunctionViolation) { return invalid("unsafe_function", unsafeFunctionViolation); @@ -475,12 +477,26 @@ function validateReadOnlyQuery( const mutableKind = findSideEffectingExpressionKind(parsedQuery); if (mutableKind) { - return invalid("non_select_query", ERRORS.nonReadOnlyQuery(mutableKind)); + return invalid( + "non_select_query", + buildNonReadOnlyQueryError(context, mutableKind) + ); } return Result.ok(null); } +function buildNonReadOnlyQueryError( + context: ValidationContext, + kind: string +): string { + const allowed = + context.dbType === "cloudflare_r2_sql" + ? "SELECT, SHOW, or DESCRIBE" + : "SELECT or SHOW"; + return `Only ${allowed} queries are allowed. Got: ${kind}`; +} + function finalizeSql(context: ValidationContext): ValidationResult { return Result.ok({ sql: context.trimmedSql }); } @@ -564,6 +580,23 @@ function isTopLevelShowExpression( return kind === "command" && firstTokenIs(context, "show"); } +function isTopLevelDescribeExpression( + expr: Expression, + context: ValidationContext +): boolean { + if (context.dbType !== "cloudflare_r2_sql") { + return false; + } + + if (getExpressionKind(expr) !== "describe") { + return false; + } + + // 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 firstTokenIs(context: ValidationContext, keyword: string): boolean { const tokenization = tokenize(context.trimmedSql, context.dialect); if (!tokenization.success || !Array.isArray(tokenization.tokens)) { @@ -729,7 +762,9 @@ function isAllowedStatementRoot( expr: Expression, parsedQuery: ParsedQuery ): boolean { - return parsedQuery.statementKind === "show" && expr === parsedQuery.statement; + return ( + parsedQuery.statementKind !== "query" && expr === parsedQuery.statement + ); } function isSideEffectingExpression(expr: Expression): boolean { @@ -795,7 +830,7 @@ export async function validateAndNormalizeReadOnlyQuery( yield* validateStrictSyntax(context); const statements = yield* Result.await(parseStatements(context)); const parsedQuery = yield* extractParsedQuery(statements, context); - yield* validateReadOnlyQuery(parsedQuery, context.dbType); + yield* validateReadOnlyQuery(parsedQuery, context); return finalizeSql(context); }); }