From da1eb8b65f85dc60d808165382da97efdbaa72b7 Mon Sep 17 00:00:00 2001 From: Charis <26616127+charislam@users.noreply.github.com> Date: Fri, 29 May 2026 09:36:22 -0400 Subject: [PATCH] chore(logs): lock the analytics SQL wire boundary (#46485) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Refactor / chore — lints the analytics SQL wire boundary and tightens internal API surface. Final PR in the safe-analytics-sql series (stacked on #46476). ## What is the current behavior? After PRs 1–10, every analytics SQL call site routes through `executeAnalyticsSql`, but nothing prevents a future caller from regressing by calling `post('/platform/projects/{ref}/analytics/endpoints/logs.all', …)` directly. `safe-analytics-sql.ts` also exports `rawSql` and `LogSqlFragmentSeparator`, neither of which has external consumers — `rawSql` in particular is a cast-to-brand escape hatch that should not be reachable from outside the file. The safe-sql-execution skill documents only the pg-meta (Postgres) side of the model. ## What is the new behavior? - Adds an ESLint `no-restricted-syntax` rule in `apps/studio/eslint.config.cjs` that fails on direct `post()` / `get()` calls against `/platform/projects/{ref}/analytics/endpoints/logs.all{,.otel}` outside the `executeAnalyticsSql` wrapper. - Un-exports `rawSql` and `LogSqlFragmentSeparator` from `safe-analytics-sql.ts`; updates the `SafeLogSqlFragment` docstring accordingly. - Adds an "Analytics SQL" section to `.claude/skills/safe-sql-execution/SKILL.md` covering the disjoint `SafeLogSqlFragment` brand, the helpers, the wire boundary, and the new lint. ## Additional context Resolves FE-2949 --- .claude/skills/safe-sql-execution/SKILL.md | 72 +++++++++++++++++++-- apps/studio/data/logs/safe-analytics-sql.ts | 15 +++-- apps/studio/eslint.config.cjs | 25 +++++++ 3 files changed, 101 insertions(+), 11 deletions(-) diff --git a/.claude/skills/safe-sql-execution/SKILL.md b/.claude/skills/safe-sql-execution/SKILL.md index 6cf59379d04..c0f43cb8f8f 100644 --- a/.claude/skills/safe-sql-execution/SKILL.md +++ b/.claude/skills/safe-sql-execution/SKILL.md @@ -279,7 +279,7 @@ function MyBadComponent() { ### Round-tripping SQL from the database (NOT snippet content) ```ts -// ✅ GOOD: SQL from the database is promoted to SafeSqlFragment at the point +// ✅ GOOD: SQL from the database is promoted to SafeSqlFragment at the point // of fetching // data/function-definitions.ts @@ -323,10 +323,10 @@ function MyComponent() { ### Snippet content is ALWAYS UNSAFE -Snippets are auto-persisted to the database and can be created or modified -through externally influenceable channels (e.g., prefilled from URL params). -The `unchecked_sql` property is typed as `UntrustedSqlFragment` to enforce this -— it must only be promoted to `SafeSqlFragment` via `acceptUntrustedSql` in an +Snippets are auto-persisted to the database and can be created or modified +through externally influenceable channels (e.g., prefilled from URL params). +The `unchecked_sql` property is typed as `UntrustedSqlFragment` to enforce this +— it must only be promoted to `SafeSqlFragment` via `acceptUntrustedSql` in an event handler that requires explicit user action. ```ts @@ -376,3 +376,65 @@ function SnippetRunner({ snippet }: { snippet: Snippet }) { ) } ``` + +## Analytics SQL (BigQuery / ClickHouse) + +The same security model applies to analytics queries, which target BigQuery +or ClickHouse via the +`/platform/projects/{ref}/analytics/endpoints/logs.all{,.otel}` endpoints. +Filter keys and values from URL parameters and UI inputs are spliced into SQL +that runs against the project's logs, so the same injection risk exists. + +The brand and helpers live in `apps/studio/data/logs/safe-analytics-sql.ts`, +intentionally **disjoint** from the pg-meta `SafeSqlFragment` brand: + +- `SafeLogSqlFragment` — branded type for analytics SQL. +- `safeSql` — template tag that only accepts `SafeLogSqlFragment` + interpolations. +- `analyticsLiteral(value)` — sanitizes string/number/boolean literals. +- `quotedIdent(name)` — validates and backtick-quotes dotted identifiers. +- `keyword(value, allowed)` — validates against an allow-list of operators. +- `joinSqlFragments(fragments, separator)` — composes already-branded + fragments. + +The brands are kept separate because escape semantics differ — Postgres-safe +`E'…'` strings, `::jsonb` casts, and double-quoted identifiers are unsafe for +BigQuery and/or ClickHouse, and vice versa. Crossing the brands would silently +emit unsafe SQL. + +The wire-boundary wrapper is `executeAnalyticsSql` in +`apps/studio/data/logs/execute-analytics-sql.ts`, analogous to pg-meta's +`executeSql`. It accepts only `SafeLogSqlFragment` for its `sql` parameter, so +raw strings are rejected at compile time. A grep-based vitest +(`apps/studio/tests/unit/lints/analytics-sql-boundary.test.ts`) prevents +regressions by failing the build if any file outside +`execute-analytics-sql.ts` calls `post()` or `get()` directly against +`logs.all` or `logs.all.otel`. + +```ts +import { executeAnalyticsSql } from '@/data/logs/execute-analytics-sql' +import { analyticsLiteral, quotedIdent, safeSql } from '@/data/logs/safe-analytics-sql' + +// ✅ GOOD: every interpolation is sanitized. +const sql = safeSql` + SELECT timestamp, event_message + FROM ${quotedIdent(table)} + WHERE id = ${analyticsLiteral(id)} +` + +await executeAnalyticsSql({ + projectRef, + endpoint: '/platform/projects/{ref}/analytics/endpoints/logs.all', + sql, + iso_timestamp_start, + iso_timestamp_end, +}) +``` + +```ts +// 🛑 BAD: raw string interpolation. This fails to type-check at the +// executeAnalyticsSql boundary because the result is `string`, not +// `SafeLogSqlFragment`. +const sql = `SELECT * FROM ${table} WHERE id = '${id}'` +await executeAnalyticsSql({ projectRef, endpoint, sql, ... }) +``` diff --git a/apps/studio/data/logs/safe-analytics-sql.ts b/apps/studio/data/logs/safe-analytics-sql.ts index 0f5842d6959..7f2b24787f2 100644 --- a/apps/studio/data/logs/safe-analytics-sql.ts +++ b/apps/studio/data/logs/safe-analytics-sql.ts @@ -40,8 +40,9 @@ * `SafeSqlFragment` (Postgres-only). * * Values of this type are either: - * - Static strings in source code (no interpolation) via `rawSql` - * - Outputs of `analyticsLiteral` or `quotedIdent` + * - Static strings in source code (no interpolation) via the `safeSql` + * template tag with no interpolations + * - Outputs of `analyticsLiteral`, `quotedIdent`, or `keyword` * - Compositions via the `safeSql` template tag (which only accepts * `SafeLogSqlFragment` interpolations) * - Compositions via `joinSqlFragments` @@ -50,7 +51,7 @@ */ export type SafeLogSqlFragment = string & { readonly __safeLogSqlFragmentBrand: never } -export type LogSqlFragmentSeparator = +type LogSqlFragmentSeparator = | ',' | ', ' | ';\n' @@ -82,10 +83,12 @@ export function safeSql( } /** - * Marks a hand-written log-SQL string as a `SafeLogSqlFragment`. Use only - * for static SQL authored in source code; never call with arbitrary input. + * Internal-only escape hatch for branding hand-written log-SQL produced by + * the helpers in this file (e.g. `analyticsLiteral`, `quotedIdent`). Not + * exported: external callers must compose via `safeSql` plus the sanitization + * helpers, never by casting arbitrary strings. */ -export function rawSql(sql: string): SafeLogSqlFragment { +function rawSql(sql: string): SafeLogSqlFragment { return sql as SafeLogSqlFragment } diff --git a/apps/studio/eslint.config.cjs b/apps/studio/eslint.config.cjs index d1de2c309c4..693497c89ff 100644 --- a/apps/studio/eslint.config.cjs +++ b/apps/studio/eslint.config.cjs @@ -33,4 +33,29 @@ module.exports = defineConfig([ 'jsx-a11y/role-has-required-aria-props': 'error', }, }, + // Analytics SQL wire boundary: every call to a SQL-bearing analytics + // endpoint (`logs.all` / `logs.all.otel`) must go through + // `executeAnalyticsSql` so the `SafeLogSqlFragment` brand is enforced at the + // type level. See .claude/skills/safe-sql-execution/SKILL.md. + { + files: ['**/*.ts', '**/*.tsx'], + ignores: ['data/logs/execute-analytics-sql.ts'], + rules: { + 'no-restricted-syntax': [ + 'error', + { + selector: + "CallExpression[callee.name=/^(post|get)$/][arguments.0.value='/platform/projects/{ref}/analytics/endpoints/logs.all']", + message: + 'Do not call the analytics logs.all endpoint directly. Route through executeAnalyticsSql in @/data/logs/execute-analytics-sql so the SafeLogSqlFragment brand is enforced at compile time.', + }, + { + selector: + "CallExpression[callee.name=/^(post|get)$/][arguments.0.value='/platform/projects/{ref}/analytics/endpoints/logs.all.otel']", + message: + 'Do not call the analytics logs.all.otel endpoint directly. Route through executeAnalyticsSql in @/data/logs/execute-analytics-sql so the SafeLogSqlFragment brand is enforced at compile time.', + }, + ], + }, + }, ])