mirror of
https://github.com/supabase/supabase.git
synced 2026-06-29 03:50:30 -04:00
a7d51cdf52
## 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 / type safety improvement ## What is the current behavior? The legacy log query stack (`genDefaultQuery`, `genCountQuery`, `genChartQuery`, `genWhereStatement`, `useLogsPreview`, `useSingleLog`) builds SQL from raw strings with no type-level guarantee that values are safely interpolated. Identifier helpers (`bqIdent`, `bqDottedIdent`, `clickhouseIdent`, `clickhouseDottedIdent`) are duplicated across BigQuery and ClickHouse variants, and `bqDottedIdent` wraps the entire dotted path in one backtick pair (`` `request.pathname` ``), which BigQuery treats as a literal column name rather than a UNNEST alias field — causing runtime query failures on dotted filter keys. ## What is the new behavior? - All gen functions return `SafeLogSqlFragment` and all callers route through `executeAnalyticsSql`, enforcing compile-time SQL provenance tracking across the legacy stack. - `bqIdent` / `bqDottedIdent` / `clickhouseIdent` / `clickhouseDottedIdent` are replaced by a single `quotedIdent` function that backtick-quotes each segment individually (e.g. `` `request`.`pathname` ``). ClickHouse natively accepts backticks, so one function serves both engines and the dotted-path quoting bug is fixed. - `SQL_FILTER_TEMPLATES` entries are converted to `SafeLogSqlFragment` (static via `safeSql`, dynamic via `safeSql` + `analyticsLiteral`). - `buildWhereClauses` is extracted as a private helper returning `SafeLogSqlFragment[]` so the pg_cron path can merge clauses without unsafe slice-and-cast. ## Additional context <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Logs query generation migrated to safer, engine-agnostic SQL fragments, typed filter templates, and unified identifier quoting for stronger injection protection and more consistent queries. * Logs preview and single-log retrieval now execute analytics SQL end-to-end using the unified executor. * **New Features** * Analytics SQL executor can call the backend via GET or POST and accepts method selection. * **Tests** * Updated tests to validate unified identifier quoting and safe-SQL helper behavior. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/supabase/supabase/pull/46351?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
123 lines
4.4 KiB
TypeScript
123 lines
4.4 KiB
TypeScript
import { describe, expect, it } from 'vitest'
|
|
|
|
import { executeAnalyticsSql } from './execute-analytics-sql'
|
|
import { analyticsLiteral, keyword, quotedIdent, safeSql } from './safe-analytics-sql'
|
|
|
|
describe('keyword', () => {
|
|
it('returns the matching SafeLogSqlFragment from the allow-list', () => {
|
|
expect(keyword('AND', [safeSql`AND`, safeSql`OR`])).toBe('AND')
|
|
expect(keyword('=', [safeSql`=`, safeSql`!=`, safeSql`LIKE`])).toBe('=')
|
|
})
|
|
|
|
it('throws when value is not in the allow-list', () => {
|
|
expect(() => keyword('DROP', [safeSql`AND`, safeSql`OR`])).toThrow(
|
|
'"DROP" is not in the allowed list'
|
|
)
|
|
})
|
|
|
|
it('is case-insensitive and returns the allow-listed fragment', () => {
|
|
// 'and' (lowercase) matches 'AND' in the allow-list; the returned value
|
|
// is the allow-listed fragment 'AND', not the raw input 'and'.
|
|
expect(keyword('and', [safeSql`AND`, safeSql`OR`])).toBe('AND')
|
|
expect(keyword('OR', [safeSql`and`, safeSql`or`])).toBe('or')
|
|
})
|
|
|
|
it('throws for an empty allow-list', () => {
|
|
expect(() => keyword('AND', [])).toThrow()
|
|
})
|
|
})
|
|
|
|
describe('quotedIdent', () => {
|
|
it('backtick-quotes a single-segment identifier', () => {
|
|
expect(quotedIdent('status')).toBe('`status`')
|
|
})
|
|
|
|
it('backtick-quotes each segment of a two-part dotted path individually', () => {
|
|
expect(quotedIdent('request.method')).toBe('`request`.`method`')
|
|
})
|
|
|
|
it('backtick-quotes each segment of a three-part dotted path individually', () => {
|
|
expect(quotedIdent('a.b.c')).toBe('`a`.`b`.`c`')
|
|
})
|
|
|
|
it('throws for an empty string', () => {
|
|
expect(() => quotedIdent('')).toThrow('invalid identifier')
|
|
})
|
|
|
|
it('throws when a segment contains disallowed characters', () => {
|
|
expect(() => quotedIdent('request.method; DROP TABLE')).toThrow('invalid identifier')
|
|
})
|
|
|
|
it('throws for an empty segment (double dot)', () => {
|
|
expect(() => quotedIdent('a..b')).toThrow('invalid identifier')
|
|
})
|
|
})
|
|
|
|
describe('executeAnalyticsSql — compile-time boundary', () => {
|
|
it('accepts a SafeLogSqlFragment', () => {
|
|
const sql = safeSql`SELECT 1`
|
|
// SafeLogSqlFragment is accepted — no type error expected here.
|
|
const fn = () =>
|
|
executeAnalyticsSql({
|
|
projectRef: 'test-ref',
|
|
endpoint: '/platform/projects/{ref}/analytics/endpoints/logs.all',
|
|
sql,
|
|
iso_timestamp_start: '2024-01-01T00:00:00Z',
|
|
iso_timestamp_end: '2024-01-02T00:00:00Z',
|
|
})
|
|
expect(fn).toBeDefined()
|
|
})
|
|
|
|
it('rejects a plain string at the type level', () => {
|
|
const plainSql: string = 'SELECT 1'
|
|
// Never invoked — compile-time check only. The function is defined so
|
|
// TypeScript type-checks the body, but no network request is made.
|
|
const _check = () =>
|
|
executeAnalyticsSql({
|
|
projectRef: 'test-ref',
|
|
endpoint: '/platform/projects/{ref}/analytics/endpoints/logs.all',
|
|
// @ts-expect-error — plain string must not be assignable to SafeLogSqlFragment
|
|
sql: plainSql,
|
|
iso_timestamp_start: '2024-01-01T00:00:00Z',
|
|
iso_timestamp_end: '2024-01-02T00:00:00Z',
|
|
})
|
|
expect(_check).toBeDefined()
|
|
})
|
|
|
|
it('rejects rawSql output cast back to string at the type level', () => {
|
|
const fragment = safeSql`SELECT 1`
|
|
const asString: string = fragment as string
|
|
// Never invoked — compile-time check only.
|
|
const _check = () =>
|
|
executeAnalyticsSql({
|
|
projectRef: 'test-ref',
|
|
endpoint: '/platform/projects/{ref}/analytics/endpoints/logs.all',
|
|
// @ts-expect-error — widened-to-string value must not be assignable to SafeLogSqlFragment
|
|
sql: asString,
|
|
iso_timestamp_start: '2024-01-01T00:00:00Z',
|
|
iso_timestamp_end: '2024-01-02T00:00:00Z',
|
|
})
|
|
expect(_check).toBeDefined()
|
|
})
|
|
})
|
|
|
|
describe('safeSql template tag — existing helpers still compose', () => {
|
|
it('analyticsLiteral output is composable via safeSql', () => {
|
|
const val = analyticsLiteral('hello')
|
|
const query = safeSql`SELECT ${val}`
|
|
expect(query).toBe("SELECT 'hello'")
|
|
})
|
|
|
|
it('quotedIdent output is composable via safeSql', () => {
|
|
const col = quotedIdent('request.method')
|
|
const query = safeSql`SELECT ${col} FROM logs`
|
|
expect(query).toBe('SELECT `request`.`method` FROM logs')
|
|
})
|
|
|
|
it('keyword output is composable via safeSql', () => {
|
|
const op = keyword('AND', [safeSql`AND`, safeSql`OR`])
|
|
const query = safeSql`WHERE a = 1 ${op} b = 2`
|
|
expect(query).toBe('WHERE a = 1 AND b = 2')
|
|
})
|
|
})
|