mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 17:35:10 +03:00
chore(logs): lock the analytics SQL wire boundary (#46485)
## 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
This commit is contained in:
1 parent
fd1f437eca
commit
da1eb8b65f
3 files changed
+101
-11
No files matched your search
@@ -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, ... })
|
||||
```
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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.',
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
])
|
||||
Reference in new issue
Block a user