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
1 change: 1 addition & 0 deletions apps/backend/src/lib/clickhouse-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const UNSAFE_CLICKHOUSE_ERROR_CODES = [
43, // ILLEGAL_TYPE_OF_ARGUMENT
47, // UNKNOWN_IDENTIFIER
60, // UNKNOWN_TABLE
386, // NO_COMMON_TYPE
497, // ACCESS_DENIED
];

Expand Down
69 changes: 69 additions & 0 deletions apps/e2e/tests/backend/endpoints/api/v1/analytics-query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,41 @@ it("handles invalid SQL query", async ({ expect }) => {
`);
});

it("does not leak data from the internal cross-project users table via type-mismatch errors", async ({ expect }) => {
const response = await runQuery({
query: "SELECT if(1, primary_email, 1) AS leaked FROM analytics_internal.users LIMIT 1",
});

expect(response.status).toBe(400);
const errorText = JSON.stringify(response.body);
expect(errorText).not.toContain("@");
expect(errorText).not.toMatch(/primary_email\s*[:=]\s*['"]/);
Comment thread
BilalG1 marked this conversation as resolved.
expect(stripQueryId(response, expect)).toMatchInlineSnapshot(`
NiceResponse {
"status": 400,
"body": {
"code": "ANALYTICS_QUERY_ERROR",
"details": {
"error": deindent\`
Error during execution of this query.

As you are in development mode, you can see the full error: 386 There is no supertype for types String, UInt8 because some of them are String\\\\/FixedString\\\\/Enum and some of them are not: In scope SELECT if(1, primary_email, 1) AS leaked FROM analytics_internal.users LIMIT 1.
\`,
},
"error": deindent\`
Error during execution of this query.

As you are in development mode, you can see the full error: 386 There is no supertype for types String, UInt8 because some of them are String\\\\/FixedString\\\\/Enum and some of them are not: In scope SELECT if(1, primary_email, 1) AS leaked FROM analytics_internal.users LIMIT 1.
\`,
},
"headers": Headers {
"x-stack-known-error": "ANALYTICS_QUERY_ERROR",
<some fields may have been hidden>,
},
}
`);
});
Comment thread
BilalG1 marked this conversation as resolved.
Comment thread
BilalG1 marked this conversation as resolved.

it("can execute query returning multiple rows", async ({ expect }) => {
const response = await runQuery({ query: "SELECT arrayJoin([0, 1, 2]) AS number" });

Expand Down Expand Up @@ -1658,6 +1693,40 @@ it("does not leak column names from restricted tables via illegal type of argume
`);
});

it("does not leak data from restricted tables via type-mismatch errors (code 386)", async ({ expect }) => {
// ClickHouse resolves types before checking permissions, so a code 386
// referencing a restricted table column would otherwise leak its type.
// 386 is classified unsafe so the raw message must not reach prod.
const response = await runQuery({
query: "SELECT if(1, query, 1) FROM system.query_log",
});

expect(stripQueryId(response, expect)).toMatchInlineSnapshot(`
NiceResponse {
"status": 400,
"body": {
"code": "ANALYTICS_QUERY_ERROR",
"details": {
"error": deindent\`
Error during execution of this query.

As you are in development mode, you can see the full error: 386 There is no supertype for types String, UInt8 because some of them are String\\\\/FixedString\\\\/Enum and some of them are not: In scope SELECT if(1, query, 1) FROM system.query_log.
\`,
},
"error": deindent\`
Error during execution of this query.

As you are in development mode, you can see the full error: 386 There is no supertype for types String, UInt8 because some of them are String\\\\/FixedString\\\\/Enum and some of them are not: In scope SELECT if(1, query, 1) FROM system.query_log.
\`,
},
"headers": Headers {
"x-stack-known-error": "ANALYTICS_QUERY_ERROR",
<some fields may have been hidden>,
},
}
`);
});

it("does not leak column names from restricted tables via unknown identifier (code 47)", async ({ expect }) => {
// ClickHouse resolves identifiers before checking permissions, and suggests
// real column names ("Maybe you meant: ..."), so code 47 must be unsafe
Expand Down
Loading