Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions apps/connector/src/jobs/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
26 changes: 23 additions & 3 deletions apps/connector/src/jobs/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
56 changes: 38 additions & 18 deletions packages/server/src/services/data-source-query/validate-sql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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 () => {
Expand All @@ -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"
);
});
Expand All @@ -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"
);
});
Expand All @@ -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) {
Expand All @@ -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",
Expand All @@ -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"
);
});
Expand All @@ -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"
);
});
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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(
Expand Down
Loading
Loading