mirror of
https://github.com/stack-auth/stack.git
synced 2026-06-30 21:01:54 +08:00
4a2595d9f7
## Summary - Add ClickHouse error code `386` (`NO_COMMON_TYPE`) to `UNSAFE_CLICKHOUSE_ERROR_CODES` in `apps/backend/src/lib/clickhouse-errors.ts`. This stops the Sentry `StackAssertionError` (`Unknown Clickhouse error: code 386 not in safe or unsafe codes`) that was firing whenever an admin wrote a query like `SELECT [1, 'a']` or `SELECT if(1, 'a', 1)`, while keeping the raw error message out of prod responses. - Add two e2e regression tests: one against the cross-project `analytics_internal.users` table, and one against `system.query_log`, to pin that 386 is wrapped with the generic `Error during execution of this query.` message in prod (full detail only surfaces in dev/test). ## Why unsafe, not safe Both callers of `getSafeClickhouseErrorMessage` (`apps/backend/src/app/api/latest/internal/analytics/query/route.ts:59` and `apps/backend/src/lib/ai/tools/sql-query.ts:80`) execute caller-authored SQL under `readonly: "1"` with `SQL_project_id`/`SQL_branch_id` scoping. The ClickHouse client runs under a `limited_user` whose grants restrict most tables — but ClickHouse resolves types **before** enforcing ACL. That means a query like `SELECT if(1, query, 1) FROM system.query_log` surfaces code 386 with a message like `There is no supertype for types String, UInt8 ...`, leaking that `system.query_log.query` is a `String` — schema info from a table the caller can't actually read. This is the same type-before-ACL class as code 43 (`ILLEGAL_TYPE_OF_ARGUMENT`), which is already classified unsafe. Classifying 386 as unsafe keeps the defense-in-depth consistent: if per-customer tables are ever introduced and grants don't block reference-resolution in time, 386 won't leak their schema. Cost: in prod, an admin writing a malformed type-mismatch query sees only `Error during execution of this query.` instead of the supertype hint. Dev and test environments still show the full error via the existing `getNodeEnvironment()` branch, so local iteration is unaffected. ## Test plan - [x] `pnpm test run apps/e2e/tests/backend/endpoints/api/v1/analytics-query.test.ts` — all 64 tests pass, including the two 386 regression tests. - [ ] Monitor Sentry after deploy to confirm the `unknown-clickhouse-error-for-query` events for code 386 stop firing. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved handling of a ClickHouse type-mismatch error to prevent exposure of sensitive data and ensure sanitized error responses. * **Tests** * Added regression tests that verify error responses are sanitized, return consistent error codes, and include expected headers without leaking internal details. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
45 lines
2.0 KiB
TypeScript
45 lines
2.0 KiB
TypeScript
import { getNodeEnvironment } from "@stackframe/stack-shared/dist/utils/env";
|
|
import { captureError, StackAssertionError } from "@stackframe/stack-shared/dist/utils/errors";
|
|
|
|
const SAFE_CLICKHOUSE_ERROR_CODES = [
|
|
62, // SYNTAX_ERROR
|
|
159, // TIMEOUT_EXCEEDED
|
|
164, // READONLY
|
|
158, // TOO_MANY_ROWS
|
|
396, // TOO_MANY_ROWS_OR_BYTES
|
|
636, // CANNOT_EXTRACT_TABLE_STRUCTURE
|
|
];
|
|
|
|
const UNSAFE_CLICKHOUSE_ERROR_CODES = [
|
|
36, // BAD_ARGUMENTS
|
|
43, // ILLEGAL_TYPE_OF_ARGUMENT
|
|
47, // UNKNOWN_IDENTIFIER
|
|
60, // UNKNOWN_TABLE
|
|
386, // NO_COMMON_TYPE
|
|
497, // ACCESS_DENIED
|
|
];
|
|
|
|
const DEFAULT_CLICKHOUSE_ERROR_MESSAGE = "Error during execution of this query.";
|
|
|
|
export function getSafeClickhouseErrorMessage(error: unknown, query: string) {
|
|
if (typeof error !== "object" || error === null || !("code" in error) || typeof error.code !== "string" || isNaN(Number(error.code)) || !("message" in error) || typeof error.message !== "string") {
|
|
captureError("unknown-clickhouse-error-for-query-not-clickhouse-error", new StackAssertionError("Unknown error from Clickhouse is not a Clickhouse error", { cause: error, query: query }));
|
|
return DEFAULT_CLICKHOUSE_ERROR_MESSAGE;
|
|
}
|
|
|
|
const errorCode = Number(error.code);
|
|
const message = error.message;
|
|
if (SAFE_CLICKHOUSE_ERROR_CODES.includes(errorCode)) {
|
|
return message;
|
|
}
|
|
const isKnown = UNSAFE_CLICKHOUSE_ERROR_CODES.includes(errorCode);
|
|
if (!isKnown) {
|
|
captureError("unknown-clickhouse-error-for-query", new StackAssertionError(`Unknown Clickhouse error: code ${errorCode} not in safe or unsafe codes`, { cause: error, query: query }));
|
|
}
|
|
|
|
if (getNodeEnvironment() === "development" || getNodeEnvironment() === "test") {
|
|
return `${DEFAULT_CLICKHOUSE_ERROR_MESSAGE}${!isKnown ? "\n\nThis error is not known and you should probably add it to the safe or unsafe codes in clickhouse-errors.ts." : ""}\n\nAs you are in development mode, you can see the full error: ${errorCode} ${message}`;
|
|
}
|
|
return DEFAULT_CLICKHOUSE_ERROR_MESSAGE;
|
|
}
|