From b75074be590c38dbd26101c162130dceacc7a2c5 Mon Sep 17 00:00:00 2001
From: Bilal Godil <bg2002@gmail.com>
Date: Thu, 23 Apr 2026 16:42:50 -0700
Subject: [PATCH 1/2] Classify ClickHouse NO_COMMON_TYPE (386) as a safe error

Admins running SQL via /internal/analytics/query were getting a generic
"unknown Clickhouse error" with a Sentry capture when their query hit a
type mismatch (e.g. `SELECT [1, 'a']`). 386 is triggered by user-authored
SQL, so surface the real message and add a regression test confirming no
row data leaks when the same error fires against internal cross-project
tables.
---
 apps/backend/src/lib/clickhouse-errors.ts     |  1 +
 .../endpoints/api/v1/analytics-query.test.ts  | 25 +++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/apps/backend/src/lib/clickhouse-errors.ts b/apps/backend/src/lib/clickhouse-errors.ts
index 60e15398ba..d29a9838ef 100644
--- a/apps/backend/src/lib/clickhouse-errors.ts
+++ b/apps/backend/src/lib/clickhouse-errors.ts
@@ -6,6 +6,7 @@ const SAFE_CLICKHOUSE_ERROR_CODES = [
   159, // TIMEOUT_EXCEEDED
   164, // READONLY
   158, // TOO_MANY_ROWS
+  386, // NO_COMMON_TYPE
   396, // TOO_MANY_ROWS_OR_BYTES
   636, // CANNOT_EXTRACT_TABLE_STRUCTURE
 ];
diff --git a/apps/e2e/tests/backend/endpoints/api/v1/analytics-query.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/analytics-query.test.ts
index afcdc93243..f8455a8eee 100644
--- a/apps/e2e/tests/backend/endpoints/api/v1/analytics-query.test.ts
+++ b/apps/e2e/tests/backend/endpoints/api/v1/analytics-query.test.ts
@@ -232,6 +232,31 @@ 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*['"]/);
+  expect(stripQueryId(response, expect)).toMatchInlineSnapshot(`
+    NiceResponse {
+      "status": 400,
+      "body": {
+        "code": "ANALYTICS_QUERY_ERROR",
+        "details": { "error": "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": "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>,
+      },
+    }
+  `);
+});
+
 it("can execute query returning multiple rows", async ({ expect }) => {
   const response = await runQuery({ query: "SELECT arrayJoin([0, 1, 2]) AS number" });
 

From 9d99aa9cdc3df156b032b29e6dde9194bc83ccb3 Mon Sep 17 00:00:00 2001
From: Bilal Godil <bg2002@gmail.com>
Date: Thu, 23 Apr 2026 18:41:52 -0700
Subject: [PATCH 2/2] Reclassify ClickHouse NO_COMMON_TYPE (386) as unsafe

Type resolution happens before ACL checks, so a 386 error against a
restricted table (e.g. system.query_log) leaks column types. Move 386
to the unsafe list so the raw message is suppressed in prod, and add
a parallel test pinning the behavior against system.query_log.
---
 apps/backend/src/lib/clickhouse-errors.ts     |  2 +-
 .../endpoints/api/v1/analytics-query.test.ts  | 48 ++++++++++++++++++-
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/apps/backend/src/lib/clickhouse-errors.ts b/apps/backend/src/lib/clickhouse-errors.ts
index d29a9838ef..dbac2f2865 100644
--- a/apps/backend/src/lib/clickhouse-errors.ts
+++ b/apps/backend/src/lib/clickhouse-errors.ts
@@ -6,7 +6,6 @@ const SAFE_CLICKHOUSE_ERROR_CODES = [
   159, // TIMEOUT_EXCEEDED
   164, // READONLY
   158, // TOO_MANY_ROWS
-  386, // NO_COMMON_TYPE
   396, // TOO_MANY_ROWS_OR_BYTES
   636, // CANNOT_EXTRACT_TABLE_STRUCTURE
 ];
@@ -16,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
 ];
 
diff --git a/apps/e2e/tests/backend/endpoints/api/v1/analytics-query.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/analytics-query.test.ts
index f8455a8eee..7776140d4b 100644
--- a/apps/e2e/tests/backend/endpoints/api/v1/analytics-query.test.ts
+++ b/apps/e2e/tests/backend/endpoints/api/v1/analytics-query.test.ts
@@ -246,8 +246,18 @@ it("does not leak data from the internal cross-project users table via type-mism
       "status": 400,
       "body": {
         "code": "ANALYTICS_QUERY_ERROR",
-        "details": { "error": "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": "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. ",
+        "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",
@@ -1683,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