From 2087d6ca70a1878183df34ebb8c4ae2bb8ace4b2 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Sun, 18 Oct 2026 11:55:28 +0000 Subject: [PATCH 01/12] fix: a revoke on an object the same apply drops is done, not left out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A view the target has and the source lacks is dropped by the schema check's fix and its grants revoked by the roles check's. When the drop ran first, each revoke failed with "does not exist", was reported as left out, and the run exited 1 — though the view, and its grants with it, were gone as intended. A fix that only revokes privileges on one object, still failing that way once retrying it after the others no longer helps, is now counted as applied, with a note that there was nothing left to revoke. A revoke whose object a later fix creates is still retried after it. Fixes #147 Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/commands/diff.ts | 2 +- packages/cli/src/promote.ts | 31 ++++++- packages/cli/test/promote-savepoints.test.ts | 92 +++++++++++++++++++- 3 files changed, 121 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/commands/diff.ts b/packages/cli/src/commands/diff.ts index 06e9410..bf1d2aa 100644 --- a/packages/cli/src/commands/diff.ts +++ b/packages/cli/src/commands/diff.ts @@ -246,7 +246,7 @@ export default class Diff extends BaseCommand { this.log(dryRun ? bold(heading) : ok(heading)) result.applied.forEach((stmt, i) => { const glyph = dryRun ? dim(`${i + 1}.`) : ok('✓') - this.log(` ${glyph} ${dim(`[${stmt.check}]`)} ${stmt.issueId}`) + this.log(` ${glyph} ${dim(`[${stmt.check}]`)} ${stmt.issueId}${stmt.note ? dim(` — ${stmt.note}`) : ''}`) // The SQL itself only in a dry run: it is the thing being reviewed, // and printing it after the fact would just repeat --detail. if (dryRun && stmt.sql) this.log(` ${dim(stmt.sql.replace(/\s*\n\s*/g, ' '))}`) diff --git a/packages/cli/src/promote.ts b/packages/cli/src/promote.ts index 8200327..a3922cd 100644 --- a/packages/cli/src/promote.ts +++ b/packages/cli/src/promote.ts @@ -75,7 +75,8 @@ type PlannedAction = FixOrigin & { action: SyncAction } type PlannedSkip = FixOrigin & { reason: string } export interface PromoteResult { - applied: (FixOrigin & { sql?: string; action?: string })[] + /** `note`: why a fix that did nothing still counts — see revokesFromAbsent. */ + applied: (FixOrigin & { sql?: string; action?: string; note?: string })[] skipped: PlannedSkip[] errors: (FixOrigin & { error: string })[] /** @@ -579,6 +580,26 @@ export function unmetDependency(sql: string, err: unknown): string | undefined { return undefined } +/** + * Whether `sql` only revokes privileges on one object, so that object being + * absent leaves nothing to do. Asked only of a fix that failed because + * something does not exist, once retrying it after the others no longer + * helps — a view dropped and created again by the same apply takes its + * revoke after the creation. + */ +export function revokesFromAbsent(sql: string): boolean { + const statements = parseStatements(sql) + if (!statements?.length) return false + const targets = new Set() + for (const { kind, node } of statements) { + if (kind !== 'GrantStmt' || node.is_grant || node.targtype !== 'ACL_TARGET_OBJECT') return false + if ((node.objects ?? []).length !== 1) return false + // Where in the text each was written is not which object it is. + targets.add(JSON.stringify([node.objtype, node.objects[0]], (key, value) => key === 'location' ? undefined : value)) + } + return targets.size === 1 +} + /** One transaction; true when it committed. */ async function runBatch( client: pg.Client, @@ -618,6 +639,14 @@ async function runBatch( pending = unmet.map(u => u.stmt) } + // A revoke on an object the target no longer has — another fix in this + // apply dropped it — has nothing left to take away: done, not left out. + const gone = unmet.filter(u => revokesFromAbsent(u.stmt.sql)) + unmet = unmet.filter(u => !gone.includes(u)) + for (const { stmt } of gone) { + done.push({ check: stmt.check, issueId: stmt.issueId, sql: stmt.sql, note: 'nothing to revoke: the object is not on the target' }) + } + await client.query('COMMIT') result.applied.push(...done) for (const { stmt, err, why } of unmet) { diff --git a/packages/cli/test/promote-savepoints.test.ts b/packages/cli/test/promote-savepoints.test.ts index c1f3460..26840e9 100644 --- a/packages/cli/test/promote-savepoints.test.ts +++ b/packages/cli/test/promote-savepoints.test.ts @@ -27,7 +27,14 @@ vi.mock('pg', () => ({ else if (sql === 'ROLLBACK') pending = new Set(committed) else if (sql.startsWith('SAVEPOINT')) atSavepoint = new Set(pending) else if (sql.startsWith('ROLLBACK TO SAVEPOINT')) pending = new Set(atSavepoint) - else if (sql.startsWith('RELEASE SAVEPOINT')) { /* kept */ } else { + else if (sql.startsWith('RELEASE SAVEPOINT')) { /* kept */ } + // A revoke on `public.` needs to exist. + else if (/^REVOKE\b/i.test(sql)) { + const on = /\bON\s+(?:TABLE\s+)?public\.(\w+)/i.exec(sql)?.[1] + if (on && !pending.has(on)) { + throw Object.assign(new Error(`relation "public.${on}" does not exist`), { code: '42P01' }) + } + } else { const code = /code:(\w+)/.exec(sql)?.[1] if (code) throw Object.assign(new Error(`failed with ${code}`), { code }) for (const need of /needs:(\w+)/.exec(sql)?.[1].split(',') ?? []) { @@ -45,7 +52,7 @@ vi.mock('pg', () => ({ }, })) -const { promote, unmetDependency } = await import('../src/promote.js') +const { promote, unmetDependency, revokesFromAbsent } = await import('../src/promote.js') function scan(fixes: Array<[id: string, spec: string]>): ScanResult { const issues = fixes.map(([id, spec]) => ({ @@ -145,3 +152,84 @@ describe('unmetDependency', () => { expect(unmetDependency('CREATE TABLE t (id int)', new Error('connection terminated'))).toBeUndefined() }) }) + +// A view the target has and the source lacks is dropped by one fix, and its +// grants revoked by others. Whichever ran first, the revokes are done: the +// object they would take privileges from is gone (issue #147). +describe('promote: revoking on an object the same apply drops', () => { + beforeEach(() => { + log.length = 0 + committed = new Set(['old_v']) + }) + + const issue = (id: string, check: 'schema' | 'roles', up: string) => + ({ id, check, severity: 'warning' as const, title: id, description: 'test', sql: { up, down: '' } }) + const scanOf = (issues: ReturnType[]): ScanResult => ({ + timestamp: '', source: 's', target: 't', score: 0, postureScore: null, + checks: [{ check: 'schema', status: 'drifted', issues, durationMs: 1 }], + summary: { total: issues.length, critical: 0, warning: issues.length, info: 0 }, + }) + + it('counts the revokes as done when the drop ran first', async () => { + // The stand-in "drops" old_v by forgetting it; the revokes then fail. + committed = new Set() + const result = await promote({ + dbUrl: 'postgres://t', + scanResult: scanOf([ + issue('drop-old_v', 'schema', "SELECT 'drops old_v'"), + issue('revoke-anon', 'roles', 'REVOKE SELECT ON TABLE public.old_v FROM anon'), + issue('revoke-authenticated', 'roles', 'REVOKE SELECT ON TABLE public.old_v FROM authenticated'), + ]), + }) + + expect(result.errors).toEqual([]) + expect(result.applied.map(f => f.issueId)).toEqual(['drop-old_v', 'revoke-anon', 'revoke-authenticated']) + expect(result.applied.filter(f => f.note).map(f => f.note)).toEqual([ + 'nothing to revoke: the object is not on the target', + 'nothing to revoke: the object is not on the target', + ]) + expect(log).toContain('COMMIT') + }) + + it('still retries a revoke whose object a later fix creates', async () => { + committed = new Set() + const result = await promote({ + dbUrl: 'postgres://t', + scanResult: scanOf([ + issue('revoke-anon', 'roles', 'REVOKE SELECT ON TABLE public.made FROM anon'), + issue('create-made', 'schema', "SELECT 'provides:made'"), + ]), + }) + + expect(result.errors).toEqual([]) + expect(result.applied.map(f => [f.issueId, f.note])).toEqual([['create-made', undefined], ['revoke-anon', undefined]]) + }) + + it('still leaves out a grant on an object the target lacks', async () => { + committed = new Set() + const result = await promote({ + dbUrl: 'postgres://t', + scanResult: scanOf([issue('grant-anon', 'roles', "SELECT 'needs:absent' /* GRANT */")]), + }) + + expect(result.applied).toEqual([]) + expect(result.errors.map(e => e.issueId)).toEqual(['grant-anon']) + }) +}) + +describe('revokesFromAbsent', () => { + it('is a revoke, or revokes, on one object', () => { + expect(revokesFromAbsent('REVOKE SELECT ON TABLE "public"."v" FROM "anon";')).toBe(true) + expect(revokesFromAbsent('REVOKE GRANT OPTION FOR SELECT ON public.v FROM anon; REVOKE SELECT ON public.v FROM anon;')).toBe(true) + expect(revokesFromAbsent('REVOKE EXECUTE ON FUNCTION public.f(int) FROM anon;')).toBe(true) + }) + + it('is not a grant, a drop, several objects or a whole schema', () => { + expect(revokesFromAbsent('GRANT SELECT ON public.v TO anon;')).toBe(false) + expect(revokesFromAbsent('DROP VIEW public.v;')).toBe(false) + expect(revokesFromAbsent('REVOKE SELECT ON public.a, public.b FROM anon;')).toBe(false) + expect(revokesFromAbsent('REVOKE SELECT ON public.a FROM anon; REVOKE SELECT ON public.b FROM anon;')).toBe(false) + expect(revokesFromAbsent('REVOKE SELECT ON ALL TABLES IN SCHEMA public FROM anon;')).toBe(false) + expect(revokesFromAbsent('not sql')).toBe(false) + }) +}) From 7afda35f0e0e209eda0160c9aaf4d397db69b24b Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Sun, 18 Oct 2026 12:06:10 +0000 Subject: [PATCH 02/12] feat: warn before a restore or apply needs more locks than the target has MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A restore or apply runs in one transaction, which holds a lock on every relation it creates, alters or drops until it commits, in a table the whole server shares. A large one ran out part way, after minutes of work, and rolled everything back with "out of shared memory". The statements are now counted before the transaction starts — each relation once, and what a new table brings with it: its TOAST table and index, an index per key, an identity sequence — against the target's lock table, sized from max_locks_per_transaction and its backend slots, less what other sessions hold. When they look likely not to fit, the run says so first, with both numbers, raising max_locks_per_transaction and --no-transaction as the ways round it. It warns rather than refuses: the table can grow into spare shared memory, so a count over its size may still fit. Co-Authored-By: Claude Opus 5.5 --- packages/cli/README.md | 4 +- packages/cli/src/commands/diff.ts | 1 + packages/cli/src/commands/restore.ts | 2 + packages/cli/src/lock-budget.ts | 98 +++++++++++++++++++++++++++ packages/cli/src/promote.ts | 6 ++ packages/cli/src/restore.ts | 28 +++++++- packages/cli/test/lock-budget.test.ts | 63 +++++++++++++++++ 7 files changed, 200 insertions(+), 2 deletions(-) create mode 100644 packages/cli/src/lock-budget.ts create mode 100644 packages/cli/test/lock-budget.test.ts diff --git a/packages/cli/README.md b/packages/cli/README.md index 9f5934e..9e3c593 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -197,7 +197,9 @@ the output. - **Targets.** Restore runs as one transaction and expects an empty database. `--force` clears the snapshot's own schemas first (never Supabase's) and puts back the triggers and policies outside them that depended on what it - cleared. + cleared. A restore or apply that looks likely to need more locks than the + target allows in one transaction says so before it starts; + `--no-transaction` commits as it goes instead. - **Plain PostgreSQL.** Restoring into plain PostgreSQL works. What can't exist there is skipped and named: a foreign key to `auth.users`, a webhook, an extension the server doesn't ship, and everything that needs it. diff --git a/packages/cli/src/commands/diff.ts b/packages/cli/src/commands/diff.ts index bf1d2aa..35b1379 100644 --- a/packages/cli/src/commands/diff.ts +++ b/packages/cli/src/commands/diff.ts @@ -241,6 +241,7 @@ export default class Diff extends BaseCommand { */ private renderApplyResult(result: PromoteResult, dryRun: boolean): void { const verb = dryRun ? 'Would apply' : 'Applied' + for (const warning of result.warnings ?? []) this.log(`${warn(warning)}\n`) if (result.applied.length > 0) { const heading = `${verb} ${result.applied.length} fix(es)${dryRun ? ', in this order' : ''}:` this.log(dryRun ? bold(heading) : ok(heading)) diff --git a/packages/cli/src/commands/restore.ts b/packages/cli/src/commands/restore.ts index 1b93638..4082118 100644 --- a/packages/cli/src/commands/restore.ts +++ b/packages/cli/src/commands/restore.ts @@ -252,6 +252,8 @@ export default class Restore extends BaseCommand { return } + for (const warning of result.warnings ?? []) this.log(`⚠ ${warning}\n`) + if (result.applied.length > 0) { this.log(`✅ Applied ${result.applied.length} operation(s):`) for (const op of result.applied.slice(0, 20)) { diff --git a/packages/cli/src/lock-budget.ts b/packages/cli/src/lock-budget.ts new file mode 100644 index 0000000..9b9e696 --- /dev/null +++ b/packages/cli/src/lock-budget.ts @@ -0,0 +1,98 @@ +/** + * Whether one transaction can hold the locks a restore or an apply takes. + * + * Every relation a transaction creates, alters, drops or reads stays locked + * until it commits, and PostgreSQL keeps those locks in one table shared by + * the whole server: `max_locks_per_transaction` for each of its backend + * slots — connections, autovacuum and background workers, WAL senders — and + * prepared transactions, less what other sessions hold. A large + * restore in one transaction can run out part way, after minutes of work, + * and roll everything back with "out of shared memory". + * + * So the statements are counted first, and the run warns before it starts + * when they look likely to need more than the server has. It only warns: the + * table can grow into spare shared memory, so a count over the nominal size + * may still fit. + */ +import type pg from 'pg' +import { parseStatements } from './sql-ast.js' + +/** Constraints that build an index of their own. */ +const INDEXED = new Set(['CONSTR_PRIMARY', 'CONSTR_UNIQUE', 'CONSTR_EXCLUSION']) + +/** + * About how many lock-table slots running `statements` in one transaction + * holds: one per distinct relation named, one per index or identity + * sequence made along the way, and two per new table for its TOAST table + * and index. Generous on purpose; a statement that does not parse counts one. + */ +export function estimateLocks(statements: string[]): number { + const relations = new Set() + let made = 0 + for (const sql of statements) { + const parsed = parseStatements(sql) + if (!parsed) { + made += 1 + continue + } + for (const { kind, node } of parsed) { + if (kind === 'CreateStmt') made += 2 + if (kind === 'IndexStmt') made += 1 + walk(node, (key, value) => { + // A relation: wrapped as `RangeVar` in a list, bare in a field of its own. + if (typeof value.relname === 'string') relations.add(`${value.schemaname ?? ''}.${value.relname}`.toLowerCase()) + if (key === 'Constraint' && (INDEXED.has(value.contype) || value.contype === 'CONSTR_IDENTITY')) made += 1 + }) + } + } + return relations.size + made +} + +function walk(node: unknown, visit: (key: string, value: any) => void): void { + if (Array.isArray(node)) { + for (const item of node) walk(item, visit) + } else if (node && typeof node === 'object') { + for (const [key, value] of Object.entries(node)) { + if (value && typeof value === 'object' && !Array.isArray(value)) visit(key, value) + walk(value, visit) + } + } +} + +export interface LockBudget { + /** The server's lock table, in slots. */ + capacity: number + /** Slots other sessions hold now. */ + inUse: number +} + +/** The target's lock table, and how much of it is taken. Undefined when it cannot be read. */ +export async function readLockBudget(client: Pick): Promise { + try { + const { rows: [row] } = await client.query<{ capacity: string; in_use: string }>(` + SELECT current_setting('max_locks_per_transaction')::bigint + * (current_setting('max_connections')::bigint + + coalesce(current_setting('autovacuum_worker_slots', true), current_setting('autovacuum_max_workers'))::bigint + 1 + + current_setting('max_worker_processes')::bigint + + current_setting('max_wal_senders')::bigint + + current_setting('max_prepared_transactions')::bigint) AS capacity, + (SELECT count(*) FROM pg_locks WHERE NOT fastpath) AS in_use`) + return { capacity: Number(row.capacity), inUse: Number(row.in_use) } + } catch { + return undefined + } +} + +/** + * A warning when `needed` slots look like more than the target has free, + * keeping a tenth of the table for everyone else; undefined when they fit. + */ +export function lockBudgetWarning(needed: number, budget: LockBudget | undefined, what: string): string | undefined { + if (!budget) return undefined + const free = budget.capacity - budget.inUse - Math.ceil(budget.capacity / 10) + if (needed <= free) return undefined + return `${what} takes about ${needed} locks in one transaction, and the target has room for about ${Math.max(free, 0)} ` + + `(max_locks_per_transaction for each connection slot, less what other sessions hold). It may fail with ` + + `"out of shared memory" and roll back. Raising max_locks_per_transaction on the target (a restart) makes room; ` + + `--no-transaction commits as it goes instead.` +} diff --git a/packages/cli/src/promote.ts b/packages/cli/src/promote.ts index a3922cd..93f5bef 100644 --- a/packages/cli/src/promote.ts +++ b/packages/cli/src/promote.ts @@ -5,6 +5,7 @@ import { errMsg } from './utils/error' import { ABSENT_ON_TARGET, EXTENSION_UNAVAILABLE, UNDEFINED_COLUMN, sqlState } from './pg-errors.js' import { destructiveReason } from './dbdiff' import { parseStatements } from './sql-ast' +import { estimateLocks, lockBudgetWarning, readLockBudget } from './lock-budget.js' import { orderStatements, referencedTables, createdPolicies, createsOnlyPolicies, @@ -85,6 +86,8 @@ export interface PromoteResult { * lists what is actually in the target now. */ rolledBack?: (FixOrigin & { sql?: string })[] + /** Said before the fixes ran: they may need more locks than the target has — see lock-budget. */ + warnings?: string[] } interface PlannedWork { @@ -557,6 +560,9 @@ async function runInTransaction( const additions = statements.filter(s => isEnumValueAddition(s.sql)) const rest = statements.filter(s => !isEnumValueAddition(s.sql)) + const warning = lockBudgetWarning(estimateLocks(rest.map(s => s.sql)), await readLockBudget(client), 'This apply') + if (warning) result.warnings = [...(result.warnings ?? []), warning] + if (additions.length > 0 && !(await runBatch(client, additions, result))) return await runBatch(client, rest, result) } diff --git a/packages/cli/src/restore.ts b/packages/cli/src/restore.ts index cbb48eb..8a85e9c 100644 --- a/packages/cli/src/restore.ts +++ b/packages/cli/src/restore.ts @@ -3,6 +3,7 @@ import { constants } from 'node:fs' import { join } from 'node:path' import pg from 'pg' import { newPgClient } from './db.js' +import { estimateLocks, lockBudgetWarning, readLockBudget } from './lock-budget.js' import type { MigrationFile, SnapshotManifest } from './types/config' import { loadMigrations } from './migration' import { MIGRATIONS_TABLE } from './constants.js' @@ -83,6 +84,23 @@ export interface RestoreResult { * most misleading thing this command could tell anyone (issue #95). */ rolledBack?: { type: 'sql' | 'api'; label: string }[] + /** Said before the restore started: it may need more locks than the target has — see lock-budget. */ + warnings?: string[] +} + +/** About how many locks replaying the snapshot's SQL layers in one transaction holds. */ +async function locksForSnapshot( + snapshotDir: string, + layers: readonly string[], + manifest: Awaited>, +): Promise { + const statements: string[] = [] + for (const layer of layers) { + if (!manifest.layers[layer === 'storage-policies' ? 'storage' : layer]?.captured) continue + const raw = await readFile(join(snapshotDir, layerRestoreFile(layer)), 'utf-8').catch(() => '') + statements.push(...extractExecutableStatements(raw)) + } + return estimateLocks(statements) } // ─── Safety Check ──────────────────────────────────────────────────────────── @@ -132,7 +150,15 @@ export async function restoreFromSnapshot(options: RestoreOptions): Promise() diff --git a/packages/cli/test/lock-budget.test.ts b/packages/cli/test/lock-budget.test.ts new file mode 100644 index 0000000..c0418a3 --- /dev/null +++ b/packages/cli/test/lock-budget.test.ts @@ -0,0 +1,63 @@ +import { describe, it, expect } from 'vitest' +import { estimateLocks, lockBudgetWarning, readLockBudget } from '../src/lock-budget.js' + +describe('estimateLocks', () => { + it('counts each relation once, however often it is named', () => { + expect(estimateLocks([ + 'ALTER TABLE public.a ADD COLUMN x int;', + 'ALTER TABLE public.a ADD COLUMN y int;', + 'COMMENT ON TABLE public.a IS \'one\';', + ])).toBe(1) + }) + + it('adds what a new table brings: its TOAST table and index, and an index per key', () => { + // a, its TOAST table and index, its primary key's index, its identity sequence + expect(estimateLocks(['CREATE TABLE public.a (id int GENERATED ALWAYS AS IDENTITY PRIMARY KEY, body text);'])).toBe(5) + // b and c, b's TOAST pair, its unique key's index; c is only referenced + expect(estimateLocks(['CREATE TABLE public.b (id int UNIQUE REFERENCES public.c (id));'])).toBe(5) + }) + + it('counts a key added later, an index, and the tables they are on', () => { + expect(estimateLocks([ + 'ALTER TABLE ONLY public.a ADD CONSTRAINT a_pkey PRIMARY KEY (id);', + 'CREATE INDEX a_body ON public.a (body);', + ])).toBe(3) + }) + + it('grows with the schema', () => { + const tables = Array.from({ length: 200 }, (_, i) => `CREATE TABLE public.t${i} (id int PRIMARY KEY);`) + expect(estimateLocks(tables)).toBe(800) + }) + + it('counts a statement it cannot parse as one', () => { + expect(estimateLocks(['\\connect other', 'not sql at all'])).toBe(2) + }) +}) + +describe('lockBudgetWarning', () => { + const budget = { capacity: 1000, inUse: 100 } + + it('says nothing when the locks fit, keeping a tenth for everyone else', () => { + expect(lockBudgetWarning(800, budget, 'This restore')).toBeUndefined() + expect(lockBudgetWarning(800, undefined, 'This restore')).toBeUndefined() + }) + + it('warns, with both numbers and the ways round it, when they do not', () => { + const warning = lockBudgetWarning(801, budget, 'This restore') + expect(warning).toMatch(/^This restore takes about 801 locks in one transaction, and the target has room for about 800/) + expect(warning).toContain('max_locks_per_transaction') + expect(warning).toContain('--no-transaction') + }) +}) + +describe('readLockBudget', () => { + it('reads the lock table and what is held', async () => { + const client = { query: async () => ({ rows: [{ capacity: '6400', in_use: '12' }] }) } + expect(await readLockBudget(client as never)).toEqual({ capacity: 6400, inUse: 12 }) + }) + + it('is undefined when the server will not say', async () => { + const client = { query: async () => { throw new Error('permission denied') } } + expect(await readLockBudget(client as never)).toBeUndefined() + }) +}) From 2b3bc252f151c804d56a02a2c3dd3090c5630804 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Sun, 18 Oct 2026 12:20:55 +0000 Subject: [PATCH 03/12] fix: compare grants on schemas USAGE on a schema of the project's own is what lets a role reach the tables in it at all, and it was not compared: a target without it read as matching, and a sync left the schema out of the Data API's reach. The roles check now reads schema grants as it reads table and routine grants, offers the GRANT or REVOKE, and leaves the grants on a schema only the target has to the schema check, as it does for routines. PostgreSQL 15 stopped granting CREATE on public to everyone, so between servers either side of that change the difference is each version's default; copying it into a newer server would undo its hardening, and it is not compared there. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/checks/roles.ts | 56 ++++++++++++++++++++---- packages/cli/test/checks/roles.test.ts | 59 ++++++++++++++++++++++++-- 2 files changed, 104 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/checks/roles.ts b/packages/cli/src/checks/roles.ts index b6de8a1..36148a4 100644 --- a/packages/cli/src/checks/roles.ts +++ b/packages/cli/src/checks/roles.ts @@ -23,12 +23,12 @@ interface PgRole { * syntax; a sequence and a routine each need their own keyword, and a * routine its argument types, since overloads share a name. */ -type ObjectKind = 'table' | 'sequence' | 'function' | 'procedure' +type ObjectKind = 'table' | 'sequence' | 'function' | 'procedure' | 'schema' interface RoleGrant { grantee: string table_schema: string - /** The object's name; for a routine, without its arguments. */ + /** The object's name; for a routine, without its arguments; empty for a schema. */ table_name: string privilege_type: string is_grantable: boolean @@ -135,6 +135,34 @@ const grantsSql = (ignoreSchemas: string[]) => ` ORDER BY 1, 2, 3, 4 ` +/** + * Grants on schemas, read from `nspacl`: `USAGE` on a schema of its own is + * what lets `authenticated` reach the tables in it through the Data API at + * all. A schema an extension made is the extension's. + */ +const schemaGrantsSql = (ignoreSchemas: string[]) => ` + SELECT CASE WHEN a.grantee = 0 THEN 'PUBLIC' ELSE pg_get_userbyid(a.grantee) END AS grantee, + n.nspname AS table_schema, '' AS table_name, + a.privilege_type, a.is_grantable, 'schema' AS object_kind + FROM pg_namespace n + CROSS JOIN LATERAL aclexplode(n.nspacl) a + WHERE n.nspacl IS NOT NULL + AND a.grantee <> n.nspowner + AND n.nspname ${grantSchemaFilter(ignoreSchemas)} + AND n.nspname NOT LIKE 'pg\\_%' AND n.nspname <> 'information_schema' + AND NOT EXISTS (SELECT 1 FROM pg_depend d WHERE d.classid = 'pg_namespace'::regclass AND d.objid = n.oid AND d.deptype = 'e') + AND (a.grantee = 0 OR pg_get_userbyid(a.grantee) NOT IN (${quoted(PLATFORM_ROLES)})) + AND (a.grantee = 0 OR pg_get_userbyid(a.grantee) NOT LIKE 'pg_%') + ORDER BY 1, 2, 4 +` + +/** Schemas, as objects whose grants go with them. */ +const schemasSql = (ignoreSchemas: string[]) => ` + SELECT nspname AS table_schema, '' AS table_name, 'schema' AS object_kind + FROM pg_namespace + WHERE nspname ${grantSchemaFilter(ignoreSchemas)} +` + /** * Column-level grants, read from `attacl`. * @@ -241,6 +269,7 @@ type DefaultGrant = { /** The object a grant or relation names, with its kind and a routine's arguments. */ function objectKey(o: { table_schema: string; table_name: string; object_kind?: ObjectKind; args?: string | null }): string { + if (o.object_kind === 'schema') return `schema:${o.table_schema}` const routine = o.object_kind === 'function' || o.object_kind === 'procedure' return `${routine ? 'routine' : o.object_kind ?? 'table'}:${o.table_schema}.${o.table_name}${routine ? `(${o.args ?? ''})` : ''}` } @@ -351,6 +380,7 @@ export class RolesCheck extends Check { sourceRoles, targetRoles, sourceGrants, targetGrants, sourceColumns, targetColumns, sourceVersion, targetVersion, sourceRelations, targetRelations, targetDefaults, sourceRoutineGrants, targetRoutineGrants, sourceRoutines, targetRoutines, + sourceSchemaGrants, targetSchemaGrants, sourceSchemas, ] = await Promise.all([ this.queryFn(ctx.source.dbUrl, ROLES_SQL) as unknown as Promise, this.queryFn(ctx.target.dbUrl, ROLES_SQL) as unknown as Promise, @@ -367,16 +397,19 @@ export class RolesCheck extends Check { this.queryFn(ctx.target.dbUrl, functionGrantsSql(ignore)) as unknown as Promise, this.queryFn(ctx.source.dbUrl, routinesSql(ignore)) as unknown as Promise, this.queryFn(ctx.target.dbUrl, routinesSql(ignore)) as unknown as Promise, + this.queryFn(ctx.source.dbUrl, schemaGrantsSql(ignore)) as unknown as Promise, + this.queryFn(ctx.target.dbUrl, schemaGrantsSql(ignore)) as unknown as Promise, + this.queryFn(ctx.source.dbUrl, schemasSql(ignore)) as unknown as Promise, ]) - const sourceObjects = [...(sourceRelations ?? []), ...(sourceRoutines ?? [])] + const sourceObjects = [...(sourceRelations ?? []), ...(sourceRoutines ?? []), ...(sourceSchemas ?? [])] const targetObjects = [...(targetRelations ?? []), ...(targetRoutines ?? [])] - const sourceAll = [...(sourceGrants ?? []), ...(sourceRoutineGrants ?? [])] - // A routine or sequence only the target has is the schema check's finding, - // and its grants go with it. Every routine grants EXECUTE to PUBLIC unless + const sourceAll = [...(sourceGrants ?? []), ...(sourceRoutineGrants ?? []), ...(sourceSchemaGrants ?? [])] + // A routine, sequence or schema only the target has is the schema check's + // finding, and its grants go with it. Every routine grants EXECUTE to PUBLIC unless // told otherwise, so each one present on the target alone was reported // again as an extra grant. const inSource = new Set(sourceObjects.map(objectKey)) - const targetAll = [...(targetGrants ?? []), ...(targetRoutineGrants ?? [])] + const targetAll = [...(targetGrants ?? []), ...(targetRoutineGrants ?? []), ...(targetSchemaGrants ?? [])] .filter(g => !g.object_kind || g.object_kind === 'table' || inSource.has(objectKey(g))) const comparable = comparablePrivileges(sourceVersion, targetVersion) const byDefault = grantedByDefault(targetObjects, targetDefaults ?? []) @@ -500,6 +533,7 @@ function grantKey(g: RoleGrant): string { function objectSql(g: RoleGrant): string { const name = `${quoteName(g.table_schema)}.${quoteName(g.table_name)}` switch (g.object_kind) { + case 'schema': return `SCHEMA ${quoteName(g.table_schema)}` case 'sequence': return `SEQUENCE ${name}` case 'function': return `FUNCTION ${name}(${g.args ?? ''})` case 'procedure': return `PROCEDURE ${name}(${g.args ?? ''})` @@ -511,6 +545,7 @@ function objectSql(g: RoleGrant): string { function describeObject(g: RoleGrant): string { const name = `${g.table_schema}.${g.table_name}` switch (g.object_kind) { + case 'schema': return `schema ${g.table_schema}` case 'sequence': return `sequence ${name}` case 'function': return `function ${name}(${g.args ?? ''})` case 'procedure': return `procedure ${name}(${g.args ?? ''})` @@ -548,7 +583,12 @@ const revokeSql = (g: RoleGrant) => `REVOKE ${grantTarget(g)} FROM ${granteeSql( */ export function comparablePrivileges(sourceVersion: number, targetVersion: number): (g: RoleGrant) => boolean { const both = Math.min(sourceVersion || Infinity, targetVersion || Infinity) - return g => g.privilege_type !== 'MAINTAIN' || both >= 170000 + // PostgreSQL 15 stopped granting CREATE on `public` to everyone. Across that + // line the difference is each version's default, and copying the old one + // into a newer server would undo its hardening. + const acrossFifteen = Boolean(sourceVersion && targetVersion) && (sourceVersion < 150000) !== (targetVersion < 150000) + return g => (g.privilege_type !== 'MAINTAIN' || both >= 170000) + && !(acrossFifteen && g.object_kind === 'schema' && g.table_schema === 'public' && g.grantee === 'PUBLIC' && g.privilege_type === 'CREATE') } /** Grants that differ the same way, on one table (or column) to one role, as one finding. */ diff --git a/packages/cli/test/checks/roles.test.ts b/packages/cli/test/checks/roles.test.ts index 1e31cf9..de95c3f 100644 --- a/packages/cli/test/checks/roles.test.ts +++ b/packages/cli/test/checks/roles.test.ts @@ -136,7 +136,7 @@ describe('RolesCheck', () => { expect(grantIssue!.sql?.up).toContain('REVOKE INSERT') }) - it('runs all 15 queries (roles, grants, column grants, routine grants, version, tables and routines per side; the target\'s default grants)', async () => { + it('runs all 18 queries (roles, grants, column grants, routine grants, schema grants, version, tables and routines per side; the source\'s schemas; the target\'s default grants)', async () => { const calls: string[] = [] const queryFn: QueryFn = async (_dbUrl, sql) => { calls.push(sql) @@ -144,7 +144,8 @@ describe('RolesCheck', () => { } const check = new RolesCheck(queryFn) await check.scan(mockContext()) - expect(calls).toHaveLength(15) + expect(calls).toHaveLength(18) + expect(calls.filter(s => s.includes('n.nspacl'))).toHaveLength(2) expect(calls.filter(s => s.includes('proacl'))).toHaveLength(2) expect(calls.filter(s => s.includes('server_version_num'))).toHaveLength(2) const roleQueries = calls.filter(s => s.includes('rolsuper')) @@ -404,7 +405,7 @@ describe('a new table keeps the source\'s grants, not the target\'s defaults', ( return source ? [] : ['anon', 'authenticated'].flatMap(grantee => ALL.map(privilege_type => ({ table_schema: 'public', grantee, privilege_type, is_grantable: false }))) } - if (sql.includes('attacl') || sql.includes('rolsuper') || sql.includes('pg_proc')) return [] + if (sql.includes('attacl') || sql.includes('rolsuper') || sql.includes('pg_proc') || sql.includes("'schema' AS object_kind")) return [] if (sql.includes('server_version_num')) return [{ v: '150008' }] if (sql.includes('relacl')) { return source ? [ @@ -528,3 +529,55 @@ describe('grants on a routine only the target has', () => { expect(issues).toEqual([]) }) }) + +// USAGE on a schema of its own is what lets a role reach the tables in it at +// all; it was not compared, so a target without it read as matching. +describe('RolesCheck: grants on schemas', () => { + const schemaGrant = (overrides: Record = {}) => + makeGrant({ table_schema: 'app', table_name: '', privilege_type: 'USAGE', object_kind: 'schema', ...overrides }) + const scanWith = (source: unknown[], target: unknown[], schemas = ['app']) => new RolesCheck(async (dbUrl, sql) => { + if (sql.includes('pg_roles')) return [makeRole()] + if (sql.includes('n.nspacl')) return (dbUrl.includes('source') ? source : target) as never + if (sql.includes("'schema' AS object_kind") && dbUrl.includes('source')) { + return schemas.map(table_schema => ({ table_schema, table_name: '', object_kind: 'schema' })) as never + } + return [] + }).scan(mockContext()) + + it('reports a schema grant the target lacks, with the GRANT that makes it', async () => { + const issues = await scanWith([schemaGrant()], []) + const missing = issues.find(i => i.id.startsWith('roles-grant-missing')) + expect(missing?.title).toContain('USAGE ON schema app TO app_readonly') + expect(missing?.sql?.up).toBe('GRANT USAGE ON SCHEMA "app" TO "app_readonly";') + expect(missing?.sql?.down).toBe('REVOKE USAGE ON SCHEMA "app" FROM "app_readonly";') + }) + + it('reports one only the target has, and revokes it', async () => { + const issues = await scanWith([], [schemaGrant({ privilege_type: 'CREATE' })]) + const extra = issues.find(i => i.id.startsWith('roles-grant-extra')) + expect(extra?.sql?.up).toBe('REVOKE CREATE ON SCHEMA "app" FROM "app_readonly";') + }) + + it('leaves the grants on a schema only the target has to the schema check', async () => { + expect(await scanWith([], [schemaGrant({ table_schema: 'old' })])).toEqual([]) + }) + + it('reports nothing when both match', async () => { + expect(await scanWith([schemaGrant()], [schemaGrant()])).toEqual([]) + }) +}) + +describe('comparablePrivileges: CREATE on public across PostgreSQL 15', () => { + const g = { grantee: 'PUBLIC', table_schema: 'public', table_name: '', privilege_type: 'CREATE', is_grantable: false, object_kind: 'schema' as const } + + it('leaves out the default that changed, in either direction', () => { + expect(comparablePrivileges(140010, 170000)(g)).toBe(false) + expect(comparablePrivileges(170000, 140010)(g)).toBe(false) + }) + + it('still compares it between servers on the same side of the change', () => { + expect(comparablePrivileges(140010, 140012)(g)).toBe(true) + expect(comparablePrivileges(150008, 180000)(g)).toBe(true) + expect(comparablePrivileges(140010, 170000)({ ...g, table_schema: 'app' })).toBe(true) + }) +}) From 4763e4e85c36968bf139bd39bc6b8b591b923591 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Sun, 18 Oct 2026 12:20:55 +0000 Subject: [PATCH 04/12] fix: a snapshot keeps grants on schemas and default privileges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit roles.sql held the grants on tables, columns and routines, and the schema layer is dumped without privileges, so a restore lost USAGE on the project's own schemas — leaving their tables out of reach — and every ALTER DEFAULT PRIVILEGES, so new tables were no longer granted what they were on the source. Both are captured now: a schema's grants as GRANT ... ON SCHEMA, and each role's defaults as the ALTER DEFAULT PRIVILEGES that make them, against PostgreSQL's built-in defaults for a global entry. The roles they name are created first on a target that lacks them, as for any grant. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/snapshot.ts | 87 ++++++++++++++++++++-- packages/cli/src/sql-deps.ts | 8 +- packages/cli/test/restore-coverage.test.ts | 6 ++ packages/cli/test/snapshot.test.ts | 40 ++++++++++ 4 files changed, 134 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/snapshot.ts b/packages/cli/src/snapshot.ts index cee6fdb..2023a51 100644 --- a/packages/cli/src/snapshot.ts +++ b/packages/cli/src/snapshot.ts @@ -700,6 +700,20 @@ async function captureRoleGrants( AND (a.grantee = 0 OR pg_get_userbyid(a.grantee) NOT LIKE 'pg\\_%') AND n.nspname NOT IN (${excluded}) UNION ALL + -- Schemas: USAGE on a schema of its own is what lets a role reach its tables. + SELECT CASE WHEN a.grantee = 0 THEN 'PUBLIC' ELSE pg_get_userbyid(a.grantee) END, + n.nspname, '', NULL::text, a.privilege_type, a.is_grantable, 'SCHEMA', NULL::text + FROM pg_namespace n + CROSS JOIN LATERAL aclexplode(n.nspacl) a + WHERE n.nspacl IS NOT NULL AND a.grantee <> n.nspowner + AND n.nspname NOT IN (${excluded}) AND n.nspname NOT LIKE 'pg\\_%' AND n.nspname <> 'information_schema' + AND NOT EXISTS (SELECT 1 FROM pg_depend d WHERE d.classid = 'pg_namespace'::regclass AND d.objid = n.oid AND d.deptype = 'e') + AND (a.grantee = 0 OR pg_get_userbyid(a.grantee) NOT IN ( + 'postgres','supabase_admin','authenticator','supabase_auth_admin', + 'supabase_storage_admin','dashboard_user','pgbouncer','supavisor' + )) + AND (a.grantee = 0 OR pg_get_userbyid(a.grantee) NOT LIKE 'pg\\_%') + UNION ALL SELECT CASE WHEN a.grantee = 0 THEN 'PUBLIC' ELSE pg_get_userbyid(a.grantee) END, n.nspname, c.relname, att.attname, a.privilege_type, a.is_grantable, '', NULL::text FROM pg_attribute att @@ -713,7 +727,7 @@ async function captureRoleGrants( `) as unknown as Array<{ grantee: string; table_schema: string; table_name: string column_name: string | null; privilege_type: string; is_grantable: boolean - /** SEQUENCE, FUNCTION or PROCEDURE; empty for a table or view. */ + /** SCHEMA, SEQUENCE, FUNCTION or PROCEDURE; empty for a table or view. */ keyword: string /** A routine's identity arguments. */ args: string | null @@ -732,23 +746,84 @@ async function captureRoleGrants( } const statements = [...grouped.values()].map(([row, ...rest]) => `GRANT ${[row, ...rest].map(r => r.privilege_type + (r.column_name ? ` (${quoteName(r.column_name)})` : '')).join(', ')}` - + ` ON ${row.keyword ? `${row.keyword} ` : ''}${quoteName(row.table_schema)}.${quoteName(row.table_name)}` - + (row.args !== null && row.keyword !== 'SEQUENCE' && row.keyword ? `(${row.args})` : '') + + ` ON ${row.keyword === 'SCHEMA' ? `SCHEMA ${quoteName(row.table_schema)}` + : `${row.keyword ? `${row.keyword} ` : ''}${quoteName(row.table_schema)}.${quoteName(row.table_name)}`}` + + (row.args !== null && row.keyword !== 'SEQUENCE' && row.keyword !== 'SCHEMA' && row.keyword ? `(${row.args})` : '') // PUBLIC is a keyword; quoted, it names a role that does not exist. + ` TO ${row.grantee === 'PUBLIC' ? 'PUBLIC' : quoteName(row.grantee)}` + (row.is_grantable ? ' WITH GRANT OPTION' : '') + ';') - const output = statements.length > 0 - ? `-- SupaForge Role Grants Snapshot\n-- ${rows.length} grant(s)\n\n${statements.join('\n')}\n` + const defaults = await defaultPrivilegeStatements(dbUrl, queryFn, excluded) + const all = [...statements, ...defaults] + const output = all.length > 0 + ? `-- SupaForge Role Grants Snapshot\n-- ${rows.length} grant(s), ${defaults.length} default privilege statement(s)\n\n${all.join('\n')}\n` : '-- No role grants found\n' await writeFile(join(dir, file), output) - return { captured: true, file, itemCount: rows.length } + return { captured: true, file, itemCount: rows.length + defaults.length } } catch (err) { return { captured: false, file, itemCount: 0, error: errMsg(err) } } } +/** What `ALTER DEFAULT PRIVILEGES` calls each kind pg_default_acl records. */ +const DEFAULT_ACL_KINDS: Record = { r: 'TABLES', S: 'SEQUENCES', f: 'FUNCTIONS', T: 'TYPES', n: 'SCHEMAS' } + +/** + * The `ALTER DEFAULT PRIVILEGES` that make each role's defaults what they + * are: what its new tables, sequences, functions, types or schemas are + * granted. One for a single schema adds to the global one; the global one + * replaces PostgreSQL's built-in default for the kind, so it is the GRANTs + * and REVOKEs between the two. A platform role's defaults are the platform's. + */ +async function defaultPrivilegeStatements(dbUrl: string, queryFn: QueryFn, excludedSchemas: string): Promise { + const platform = `'postgres','supabase_admin','authenticator','supabase_auth_admin','supabase_storage_admin','dashboard_user','pgbouncer','supavisor'` + const rows = await queryFn(dbUrl, ` + WITH d AS ( + SELECT d.*, n.nspname FROM pg_default_acl d LEFT JOIN pg_namespace n ON n.oid = d.defaclnamespace + WHERE pg_get_userbyid(d.defaclrole) NOT IN ('supabase_admin','supabase_auth_admin','supabase_storage_admin','dashboard_user') + AND (n.nspname IS NULL OR n.nspname NOT IN (${excludedSchemas})) + ), have AS ( + SELECT d.oid, a.* FROM d CROSS JOIN LATERAL aclexplode(d.defaclacl) a + ), builtin AS ( + SELECT d.oid, b.* FROM d CROSS JOIN LATERAL aclexplode(acldefault(d.defaclobjtype, d.defaclrole)) b + WHERE d.defaclnamespace = 0 + ) + SELECT pg_get_userbyid(d.defaclrole) AS role, d.nspname AS schema, d.defaclobjtype::text AS kind, x.action, + CASE WHEN x.grantee = 0 THEN 'PUBLIC' ELSE pg_get_userbyid(x.grantee) END AS grantee, + x.privilege_type, x.is_grantable + FROM d JOIN ( + SELECT h.oid, 'GRANT' AS action, h.grantee, h.privilege_type, h.is_grantable FROM have h + WHERE NOT EXISTS (SELECT 1 FROM builtin b WHERE b.oid = h.oid AND b.grantee = h.grantee + AND b.privilege_type = h.privilege_type AND b.is_grantable = h.is_grantable) + UNION ALL + SELECT b.oid, 'REVOKE', b.grantee, b.privilege_type, b.is_grantable FROM builtin b + WHERE NOT EXISTS (SELECT 1 FROM have h WHERE h.oid = b.oid AND h.grantee = b.grantee + AND h.privilege_type = b.privilege_type AND h.is_grantable = b.is_grantable) + ) x ON x.oid = d.oid + WHERE (x.grantee = 0 OR pg_get_userbyid(x.grantee) NOT IN (${platform}) OR x.grantee = d.defaclrole) + AND (x.grantee = 0 OR pg_get_userbyid(x.grantee) NOT LIKE 'pg\\_%') + ORDER BY 1, 2 NULLS FIRST, 3, 4, 5, 6 + `) as unknown as Array<{ role: string; schema: string | null; kind: string; action: 'GRANT' | 'REVOKE' + grantee: string; privilege_type: string; is_grantable: boolean }> + + const grouped = new Map() + for (const row of rows) { + if (!DEFAULT_ACL_KINDS[row.kind]) continue + const key = JSON.stringify([row.role, row.schema, row.kind, row.action, row.grantee, row.action === 'GRANT' && row.is_grantable]) + grouped.set(key, [...(grouped.get(key) ?? []), row]) + } + const grantee = (g: string) => g === 'PUBLIC' ? 'PUBLIC' : quoteName(g) + return [...grouped.values()].map(([row, ...rest]) => { + const head = `ALTER DEFAULT PRIVILEGES FOR ROLE ${quoteName(row.role)}${row.schema ? ` IN SCHEMA ${quoteName(row.schema)}` : ''}` + const privileges = [row, ...rest].map(r => r.privilege_type).join(', ') + const on = DEFAULT_ACL_KINDS[row.kind] + return row.action === 'GRANT' + ? `${head} GRANT ${privileges} ON ${on} TO ${grantee(row.grantee)}${row.is_grantable ? ' WITH GRANT OPTION' : ''};` + : `${head} REVOKE ${privileges} ON ${on} FROM ${grantee(row.grantee)};` + }) +} + async function captureExtensions( dir: string, dbUrl: string, diff --git a/packages/cli/src/sql-deps.ts b/packages/cli/src/sql-deps.ts index 9e59e90..168b6e4 100644 --- a/packages/cli/src/sql-deps.ts +++ b/packages/cli/src/sql-deps.ts @@ -657,7 +657,13 @@ function rolesOf({ kind, node }: Statement): string[] { (kind === 'GrantStmt' && node.is_grant) ? node.grantees ?? [] : (kind === 'GrantRoleStmt' && node.is_grant) ? node.grantee_roles ?? [] : (kind === 'CreatePolicyStmt' || kind === 'AlterPolicyStmt') ? node.roles ?? [] - : [] + // The roles whose new objects it is about (FOR ROLE), and those it grants to. + : kind === 'AlterDefaultPrivilegesStmt' ? [ + ...((node.options ?? []) as Array>) + .filter(o => o.DefElem?.defname === 'roles').flatMap(o => o.DefElem.arg?.List?.items ?? []), + ...(node.action?.is_grant ? node.action.grantees ?? [] : []), + ] + : [] return specs .map(spec => spec.RoleSpec) // PUBLIC, CURRENT_USER and the like are not role names. diff --git a/packages/cli/test/restore-coverage.test.ts b/packages/cli/test/restore-coverage.test.ts index 6b1cf88..0884d12 100644 --- a/packages/cli/test/restore-coverage.test.ts +++ b/packages/cli/test/restore-coverage.test.ts @@ -156,6 +156,12 @@ describe('previewSnapshotRestore: layer coverage', () => { * member of publication`. */ describe('rolesNamedBy', () => { + it('names the roles default privileges are for and grant to, not those they revoke from', () => { + expect(rolesNamedBy('ALTER DEFAULT PRIVILEGES FOR ROLE "owner" IN SCHEMA app GRANT SELECT ON TABLES TO reader, PUBLIC;')) + .toEqual(['owner', 'reader']) + expect(rolesNamedBy('ALTER DEFAULT PRIVILEGES REVOKE EXECUTE ON FUNCTIONS FROM gone;')).toEqual([]) + }) + it('names the role a grant needs', () => { expect(rolesNamedBy('GRANT SELECT ON "public"."orders" TO "anon";')).toEqual(['anon']) }) diff --git a/packages/cli/test/snapshot.test.ts b/packages/cli/test/snapshot.test.ts index ecfbc35..0f78344 100644 --- a/packages/cli/test/snapshot.test.ts +++ b/packages/cli/test/snapshot.test.ts @@ -542,6 +542,46 @@ describe('captureSnapshot: grants are captured one statement per role and object }) }) +// Grants on schemas, and default privileges, were left out of roles.sql: a +// restore lost USAGE on a schema of the project's own — so its tables were +// out of the Data API's reach — and what new tables are granted. +describe('roles.sql: schemas and default privileges', () => { + let dir: string + beforeEach(async () => { dir = await mkdtemp(join(tmpdir(), 'sf-snap-roles-')) }) + afterEach(async () => { await rm(dir, { recursive: true, force: true }) }) + + it('grants on a schema, and makes each role\'s defaults what they are', async () => { + const grants = [ + { grantee: 'reader', table_schema: 'app', table_name: '', column_name: null, privilege_type: 'USAGE', is_grantable: false, keyword: 'SCHEMA', args: null }, + ] + const defaults = [ + // One schema's entry adds to the global one. + { role: 'owner', schema: 'app', kind: 'r', action: 'GRANT', grantee: 'reader', privilege_type: 'SELECT', is_grantable: false }, + // The global one, against PostgreSQL's built-in default for functions. + { role: 'owner', schema: null, kind: 'f', action: 'REVOKE', grantee: 'PUBLIC', privilege_type: 'EXECUTE', is_grantable: false }, + { role: 'owner', schema: null, kind: 'S', action: 'GRANT', grantee: 'reader', privilege_type: 'SELECT', is_grantable: true }, + { role: 'owner', schema: null, kind: 'S', action: 'GRANT', grantee: 'reader', privilege_type: 'USAGE', is_grantable: true }, + ] + const queryFn = (async (_url: string, sql: string) => + sql.includes('pg_default_acl') ? defaults : sql.includes('nspacl') ? grants : []) as unknown as QueryFn + const result = await captureSnapshot({ + envName: 'prod', + env: { dbUrl: 'postgres://example' }, + config: { environments: { prod: { dbUrl: 'postgres://example' } } } as never, + outputDir: dir, + queryFn, + fetchFn: (async () => new Response('[]', { status: 200 })) as never, + }) + const lines = (await readFile(join(result.dir, 'roles.sql'), 'utf8')).split('\n').filter(l => /^(GRANT|ALTER)/.test(l)) + expect(lines).toEqual([ + 'GRANT USAGE ON SCHEMA "app" TO "reader";', + 'ALTER DEFAULT PRIVILEGES FOR ROLE "owner" IN SCHEMA "app" GRANT SELECT ON TABLES TO "reader";', + 'ALTER DEFAULT PRIVILEGES FOR ROLE "owner" REVOKE EXECUTE ON FUNCTIONS FROM PUBLIC;', + 'ALTER DEFAULT PRIVILEGES FOR ROLE "owner" GRANT SELECT, USAGE ON SEQUENCES TO "reader" WITH GRANT OPTION;', + ]) + }) +}) + describe('a snapshot without schema.sql', () => { const manifest = (schema: Record) => ({ timestamp: '20250101T120000Z', From 8a52a381a6b2b56d7b431f86c9fc43478bffbf7d Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Sun, 18 Oct 2026 12:52:12 +0000 Subject: [PATCH 05/12] fix: leave out a fix an older target cannot parse, and apply the rest A source newer than the target writes what the target has no syntax or feature for: UNIQUE NULLS NOT DISTINCT before PostgreSQL 15, a VIRTUAL generated column before 18, security_invoker before 15. The fix failed with a syntax error, and the whole apply rolled back with it. The apply now knows the source's version. On a target that is older, a syntax error, an unsupported feature or an unknown parameter means the target cannot hold that fix: it is left out and reported, as one needing a missing extension is, and the rest is applied. --prove rehearses it the same way. On a target as new as the source those are still real failures and roll everything back. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/commands/diff.ts | 1 + packages/cli/src/promote.ts | 56 +++++++++++++++++--- packages/cli/src/prove.ts | 2 +- packages/cli/test/promote-savepoints.test.ts | 11 ++++ 4 files changed, 61 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/commands/diff.ts b/packages/cli/src/commands/diff.ts index 35b1379..cb6b566 100644 --- a/packages/cli/src/commands/diff.ts +++ b/packages/cli/src/commands/diff.ts @@ -523,6 +523,7 @@ export default class Diff extends BaseCommand { const result = await promote({ dbUrl: targetEnv.dbUrl, + sourceUrl: config.environments[config.source!]?.dbUrl, scanResult, checks, dryRun, diff --git a/packages/cli/src/promote.ts b/packages/cli/src/promote.ts index 93f5bef..26a9b5b 100644 --- a/packages/cli/src/promote.ts +++ b/packages/cli/src/promote.ts @@ -22,6 +22,12 @@ export type FetchFn = (url: string, init?: RequestInit) => Promise export interface PromoteOptions { /** Target database connection string */ dbUrl: string + /** + * The source's connection string. Its PostgreSQL version tells a fix an + * older target cannot parse — a feature it does not have — from any other + * failure: that fix is left out, as one needing a missing extension is. + */ + sourceUrl?: string /** The scan result with SQL fixes to apply */ scanResult: ScanResult /** Only promote specific checks */ @@ -472,13 +478,15 @@ async function executeSql( statements: PlannedSql[], result: PromoteResult, transactional: boolean, + sourceMajor?: number, ): Promise { if (statements.length === 0) return const client = newPgClient(dbUrl) await client.connect() try { - if (transactional) await runInTransaction(client, statements, result) + const olderTarget = sourceMajor !== undefined && (await majorOf(client)) < sourceMajor + if (transactional) await runInTransaction(client, statements, result, olderTarget) else await runIndependently(client, statements, result) } finally { await client.end() @@ -492,12 +500,30 @@ async function executeSql( * this way, so it rehearses what the apply will do rather than a stricter * script that stopped at the first fix the server could not hold. */ -export async function runLikeApply(dbUrl: string, statements: PlannedSql[]): Promise { +export async function runLikeApply(dbUrl: string, statements: PlannedSql[], sourceMajor?: number): Promise { const result: PromoteResult = { applied: [], skipped: [], errors: [] } - await executeSql(dbUrl, statements, result, true) + await executeSql(dbUrl, statements, result, true, sourceMajor) return result } +/** A server's major version, from a client or a connection string. */ +async function majorOf(client: Pick): Promise { + const { rows: [row] } = await client.query<{ v: string }>(`SELECT current_setting('server_version_num') AS v`) + return Math.floor(Number(row?.v ?? 0) / 10000) +} + +export async function serverMajor(dbUrl: string): Promise { + const client = newPgClient(dbUrl) + try { + await client.connect() + return await majorOf(client) + } catch { + return undefined + } finally { + await client.end().catch(() => {}) + } +} + /** One statement at a time: a failure is recorded and the rest still run. */ async function runIndependently( client: pg.Client, @@ -556,6 +582,7 @@ async function runInTransaction( client: pg.Client, statements: PlannedSql[], result: PromoteResult, + olderTarget = false, ): Promise { const additions = statements.filter(s => isEnumValueAddition(s.sql)) const rest = statements.filter(s => !isEnumValueAddition(s.sql)) @@ -563,8 +590,8 @@ async function runInTransaction( const warning = lockBudgetWarning(estimateLocks(rest.map(s => s.sql)), await readLockBudget(client), 'This apply') if (warning) result.warnings = [...(result.warnings ?? []), warning] - if (additions.length > 0 && !(await runBatch(client, additions, result))) return - await runBatch(client, rest, result) + if (additions.length > 0 && !(await runBatch(client, additions, result, olderTarget))) return + await runBatch(client, rest, result, olderTarget) } /** @@ -574,9 +601,12 @@ async function runInTransaction( * Only "does not exist" failures qualify, and for `CREATE EXTENSION`, an * extension the server does not ship. */ -export function unmetDependency(sql: string, err: unknown): string | undefined { +export function unmetDependency(sql: string, err: unknown, olderTarget = false): string | undefined { const code = sqlState(err) if (!code) return undefined + if (olderTarget && OLDER_SERVER.has(code)) { + return "the target's PostgreSQL is older than the source's and does not support it" + } if (/^\s*CREATE\s+EXTENSION\b/i.test(sqlSkeleton(sql))) { return EXTENSION_UNAVAILABLE.has(code) ? 'this server does not ship it' : undefined } @@ -606,11 +636,20 @@ export function revokesFromAbsent(sql: string): boolean { return targets.size === 1 } +/** + * How an older server refuses what a newer one wrote: a syntax it does not + * know (`UNIQUE NULLS NOT DISTINCT` before 15, a `VIRTUAL` column before 18), + * a feature it lacks, a parameter it does not have (`security_invoker` before + * 15). Taken as the target lacking it only when the target is older. + */ +const OLDER_SERVER: ReadonlySet = new Set(['42601', '0A000', '22023']) + /** One transaction; true when it committed. */ async function runBatch( client: pg.Client, statements: PlannedSql[], result: PromoteResult, + olderTarget = false, ): Promise { if (statements.length === 0) return true const done: PromoteResult['applied'] = [] @@ -630,7 +669,7 @@ async function runBatch( await client.query('RELEASE SAVEPOINT sf_fix') done.push({ check: stmt.check, issueId: stmt.issueId, sql: stmt.sql }) } catch (err) { - const why = unmetDependency(stmt.sql, err) + const why = unmetDependency(stmt.sql, err, olderTarget) if (why && await client.query('ROLLBACK TO SAVEPOINT sf_fix').then(() => true, () => false)) { unmet.push({ stmt, err, why }) continue @@ -732,7 +771,8 @@ export async function promote(options: PromoteOptions): Promise { return result } - await executeSql(dbUrl, sqlStatements, result, transactional) + await executeSql(dbUrl, sqlStatements, result, transactional, + options.sourceUrl ? await serverMajor(options.sourceUrl) : undefined) // A rolled-back batch leaves the target untouched, so the API calls that // would have gone with it must not fire either — they are not transactional diff --git a/packages/cli/src/prove.ts b/packages/cli/src/prove.ts index 842d10b..dad7256 100644 --- a/packages/cli/src/prove.ts +++ b/packages/cli/src/prove.ts @@ -292,7 +292,7 @@ export async function proveConvergence(opts: { // The migration under test. let notApplied: ProofResult['notApplied'] if (opts.fixes) { - const replayed = await runLikeApply(cloneUrl, opts.fixes) + const replayed = await runLikeApply(cloneUrl, opts.fixes, sourceMajor) const unmet = replayed.errors.filter(e => / — not applied: /.test(e.error)) const failed = replayed.errors.filter(e => !unmet.includes(e)) if (failed.length > 0) { diff --git a/packages/cli/test/promote-savepoints.test.ts b/packages/cli/test/promote-savepoints.test.ts index 26840e9..e1a2fd1 100644 --- a/packages/cli/test/promote-savepoints.test.ts +++ b/packages/cli/test/promote-savepoints.test.ts @@ -146,6 +146,17 @@ describe('unmetDependency', () => { expect(unmetDependency('ALTER TABLE t ALTER COLUMN c TYPE int', err('0A000'))).toBeUndefined() }) + // A newer source writes what an older target cannot parse: UNIQUE NULLS NOT + // DISTINCT before 15, a VIRTUAL column before 18, security_invoker before + // 15. On such a target that fix is left out, like one needing an extension + // it lacks; on a target as new as the source it is still a real failure. + it('accepts a syntax, feature or parameter an older target does not have', () => { + for (const code of ['42601', '0A000', '22023']) { + expect(unmetDependency('CREATE TABLE t (c text UNIQUE NULLS NOT DISTINCT)', err(code), true), code).toContain('older') + expect(unmetDependency('CREATE TABLE t (c text UNIQUE NULLS NOT DISTINCT)', err(code), false), code).toBeUndefined() + } + }) + it('rejects every other failure', () => { expect(unmetDependency('CREATE TABLE t (id int)', err('42P07'))).toBeUndefined() expect(unmetDependency('ALTER TABLE t ADD CONSTRAINT u UNIQUE (id)', err('23505'))).toBeUndefined() From 537ba049b3d3dd83db28e8bf02f2ba55e627bf12 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Sun, 18 Oct 2026 12:52:12 +0000 Subject: [PATCH 06/12] fix: leave Supabase's own default privileges, and pgmq's tables, out of the schema check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dbdiff now compares default privileges, and between plain PostgreSQL and Supabase it offered to change those supabase_admin holds — the platform's, which postgres cannot alter — so the apply failed. Default privileges for a platform role are left out, as their grants are; postgres's own are the project's and still compared. pgmq's schema holds the queues' tables, which the queues check compares through pgmq; the schema check no longer reads them. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/dbdiff.ts | 18 +++++++++++++++++- packages/cli/src/defaults.ts | 4 +++- packages/cli/test/dbdiff-units.test.ts | 18 ++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/dbdiff.ts b/packages/cli/src/dbdiff.ts index fd24f8e..a14b1b6 100644 --- a/packages/cli/src/dbdiff.ts +++ b/packages/cli/src/dbdiff.ts @@ -8,6 +8,7 @@ import type { DriftIssue } from './types/drift' import { errMsg, friendlyDbError, DiagnosticError } from './utils/error' import { DBDIFF_EXEC_TIMEOUT_MS, DBDIFF_MAX_BUFFER } from './constants' import { schemaDiffIgnores } from './defaults' +import { PLATFORM_ROLES } from './checks/roles' import { parseStatements, names, relationName, type Statement } from './sql-ast.js' import { sqlSkeleton, policiesRemoved } from './sql-deps' import { escapeRegex } from './utils/strings' @@ -767,6 +768,20 @@ function unfoldedIssues( * " modified"; anything else by its leading statement, which for a * table change is its first ALTER TABLE rather than a view it stands aside. */ +/** + * Default privileges for a role the platform owns — supabase_admin's, the + * auth and storage admins' — are Supabase's, like their grants: they differ + * between a Supabase project and plain PostgreSQL or between Supabase + * versions, and `postgres` cannot change them, so offering to failed the + * whole apply. `postgres`'s own are the project's. + */ +function platformDefaultPrivileges(sql: string): boolean { + const role = /^\s*ALTER\s+DEFAULT\s+PRIVILEGES\s+FOR\s+ROLE\s+("(?:[^"]|"")+"|[\w$]+)/i.exec(sql)?.[1] + if (!role) return false + const name = role.startsWith('"') ? role.slice(1, -1).replace(/""/g, '"') : role.toLowerCase() + return name !== 'postgres' && PLATFORM_ROLES.includes(name) +} + function issuesFromUnits( upUnits: DbDiffUnit[], downUnits: DbDiffUnit[], @@ -775,7 +790,8 @@ function issuesFromUnits( ignoredSchemaTables?: Set, ): DriftIssue[] { const pairs = pairUnits(upUnits, downUnits).filter(({ up, down }) => - !(ignoreSchemas?.length && isCrossSchemaFkUnit(up, down, ignoreSchemas, ignoredSchemaTables))) + !(ignoreSchemas?.length && isCrossSchemaFkUnit(up, down, ignoreSchemas, ignoredSchemaTables)) + && !platformDefaultPrivileges(up.sql)) return pairs.map(({ up, down }, i) => { const upSql = up.sql diff --git a/packages/cli/src/defaults.ts b/packages/cli/src/defaults.ts index 581982a..d47bb5f 100644 --- a/packages/cli/src/defaults.ts +++ b/packages/cli/src/defaults.ts @@ -43,8 +43,10 @@ export const SUPABASE_PLATFORM_SCHEMAS = [ * schema is compared. */ export function schemaDiffIgnores(ignoreSchemas: readonly string[] = DEFAULT_IGNORE_SCHEMAS): string[] { + // pgmq's tables are its queues, which the queues check compares through + // pgmq itself. return [...new Set([...ignoreSchemas, ...SUPABASE_PLATFORM_SCHEMAS.filter(s => !DEFAULT_IGNORE_SCHEMAS.includes(s)), - '_realtime', '_analytics', 'pgsodium_masks'])] + '_realtime', '_analytics', 'pgsodium_masks', 'pgmq'])] } /** Hint text shown during `supaforge init` to guide users through the Supabase UI. */ diff --git a/packages/cli/test/dbdiff-units.test.ts b/packages/cli/test/dbdiff-units.test.ts index a846e74..380a696 100644 --- a/packages/cli/test/dbdiff-units.test.ts +++ b/packages/cli/test/dbdiff-units.test.ts @@ -325,3 +325,21 @@ describe('dbdiffNotices', () => { .toEqual(['Operator objects are not compared (source 1, target 0: ===) — check them by hand']) }) }) + +// supabase_admin's default privileges are Supabase's, and postgres cannot +// change them: offered as a fix, they failed the whole apply. +describe('default privileges a platform role holds', () => { + const defaults = (role: string) => units(['AlterDefaultPrivileges', `${role}.public.tables`, + `ALTER DEFAULT PRIVILEGES FOR ROLE "${role}" IN SCHEMA "public" REVOKE SELECT ON TABLES FROM "anon";`]) + + it('are left out', () => { + expect(sqlToIssues({ up: defaults('supabase_admin'), down: defaults('supabase_admin') }, 'schema')).toEqual([]) + }) + + it('but postgres\'s, and any other role\'s, are the project\'s', () => { + for (const role of ['postgres', 'app_owner']) { + expect(sqlToIssues({ up: defaults(role), down: defaults(role) }, 'schema').map(i => i.title)) + .toEqual([`Default privileges differ: ${role} in public on tables`]) + } + }) +}) From 3c3a0c95cbde241f9bb0c25bde55853eb8629d9f Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Sun, 18 Oct 2026 13:09:05 +0000 Subject: [PATCH 07/12] fix: offer only the extension changes the target can make MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A version mismatch was fixed with ALTER EXTENSION ... UPDATE TO the source's version whichever way it went. PostgreSQL has no way down a version, and a server may not ship the one asked for: between Supabase 15 and 17, pg_cron 1.6 against 1.6.4 failed, and the whole apply with it. The target's own update paths now decide: a version it can reach gets the ALTER, any other is reported with no fix and why. An extension in a schema of its own was created into it, which plain PostgreSQL may not have — Supabase's `extensions` — and the missing schema failed the apply. The schema is made first; an extension the server does not ship is still left out with it. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/checks/extensions.ts | 52 +++++++++++++++++---- packages/cli/test/checks/extensions.test.ts | 40 +++++++++++++++- 2 files changed, 82 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/checks/extensions.ts b/packages/cli/src/checks/extensions.ts index 4c791a4..32b72b5 100644 --- a/packages/cli/src/checks/extensions.ts +++ b/packages/cli/src/checks/extensions.ts @@ -18,12 +18,25 @@ export class ExtensionsCheck extends Check { } async scan(ctx: CheckContext): Promise { - const [source, target] = await Promise.all([ + const [source, target, reachable] = await Promise.all([ this.fetchExtensions(ctx.source.dbUrl), this.fetchExtensions(ctx.target.dbUrl), + this.fetchUpdates(ctx.target.dbUrl), ]) const ignored = new Set(ctx.config.ignoreExtensions ?? []) - return diffExtensions(source, target.filter(e => !ignored.has(e.name) || source.some(s => s.name === e.name))) + return diffExtensions(source, target.filter(e => !ignored.has(e.name) || source.some(s => s.name === e.name)), reachable) + } + + /** The versions each installed extension can be updated to on this server. */ + private async fetchUpdates(dbUrl: string): Promise> | undefined> { + try { + const rows = await this.queryFn(dbUrl, UPDATES_SQL) as unknown as Array<{ name: string; version: string }> + const out = new Map>() + for (const r of rows) out.set(r.name, (out.get(r.name) ?? new Set()).add(r.version)) + return out + } catch { + return undefined + } } private async fetchExtensions(dbUrl: string): Promise { @@ -40,7 +53,20 @@ const EXTENSIONS_SQL = ` ORDER BY e.extname ` -export function diffExtensions(source: PgExtension[], target: PgExtension[]): DriftIssue[] { +const UPDATES_SQL = ` + SELECT e.extname AS name, p.target AS version + FROM pg_extension e + CROSS JOIN LATERAL pg_extension_update_paths(e.extname) p + WHERE p.source = e.extversion AND p.path IS NOT NULL +` + +/** + * `reachable`: the versions each of the target's extensions can be updated + * to there. A version mismatch gets its ALTER EXTENSION only when the + * source's is one of them: PostgreSQL has no way down a version, and a + * server may not ship the one asked for, and either failed the whole apply. + */ +export function diffExtensions(source: PgExtension[], target: PgExtension[], reachable?: Map>): DriftIssue[] { const issues: DriftIssue[] = [] const sourceMap = new Map(source.map(e => [e.name, e])) const targetMap = new Map(target.map(e => [e.name, e])) @@ -56,7 +82,12 @@ export function diffExtensions(source: PgExtension[], target: PgExtension[]): Dr description: `Extension "${name}" (v${ext.version}) is enabled in source (schema: ${ext.schema}) but not in target.`, sourceValue: ext, sql: { - up: `CREATE EXTENSION IF NOT EXISTS ${quoteName(name)}${ext.schema !== 'public' ? ` SCHEMA ${quoteName(ext.schema)}` : ''};`, + // Into the schema it is in on the source, which plain PostgreSQL + // may not have — Supabase's `extensions`. Made first; if the server + // does not ship the extension, the fix is left out with it. + up: ext.schema !== 'public' + ? `CREATE SCHEMA IF NOT EXISTS ${quoteName(ext.schema)};\nCREATE EXTENSION IF NOT EXISTS ${quoteName(name)} SCHEMA ${quoteName(ext.schema)};` + : `CREATE EXTENSION IF NOT EXISTS ${quoteName(name)};`, down: `DROP EXTENSION IF EXISTS ${quoteName(name)};`, }, }) @@ -83,6 +114,7 @@ export function diffExtensions(source: PgExtension[], target: PgExtension[]): Dr if (!tgtExt) continue if (srcExt.version !== tgtExt.version) { + const updatable = !reachable || reachable.get(name)?.has(srcExt.version) issues.push({ id: `ext-version-${name}`, check: 'extensions', @@ -91,10 +123,14 @@ export function diffExtensions(source: PgExtension[], target: PgExtension[]): Dr description: `Extension "${name}" is v${srcExt.version} in source but v${tgtExt.version} in target.`, sourceValue: srcExt, targetValue: tgtExt, - sql: { - up: `ALTER EXTENSION ${quoteName(name)} UPDATE TO ${quoteLiteral(srcExt.version)};`, - down: `ALTER EXTENSION ${quoteName(name)} UPDATE TO ${quoteLiteral(tgtExt.version)};`, - }, + ...(updatable + ? { + sql: { + up: `ALTER EXTENSION ${quoteName(name)} UPDATE TO ${quoteLiteral(srcExt.version)};`, + down: `ALTER EXTENSION ${quoteName(name)} UPDATE TO ${quoteLiteral(tgtExt.version)};`, + }, + } + : { manualOnly: `the target cannot go from v${tgtExt.version} to v${srcExt.version}: its server has no update path there (an extension cannot be moved down a version)` }), }) } } diff --git a/packages/cli/test/checks/extensions.test.ts b/packages/cli/test/checks/extensions.test.ts index a6016a9..b1fcd35 100644 --- a/packages/cli/test/checks/extensions.test.ts +++ b/packages/cli/test/checks/extensions.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { ExtensionsCheck } from '../../src/checks/extensions.js' +import { ExtensionsCheck, diffExtensions } from '../../src/checks/extensions.js' import type { CheckContext } from '../../src/checks/base.js' import type { QueryFn } from '../../src/db.js' @@ -54,7 +54,8 @@ describe('ExtensionsCheck', () => { }) it('detects version mismatch', async () => { - const queryFn: QueryFn = async (dbUrl) => { + const queryFn: QueryFn = async (dbUrl, sql) => { + if (sql.includes('pg_extension_update_paths')) return [{ name: makeExt().name, version: '1.4' }] if (dbUrl.includes('source')) return [makeExt({ version: '1.4' })] return [makeExt({ version: '1.3' })] } @@ -103,3 +104,38 @@ describe('ExtensionsCheck', () => { expect(issues[0].sql?.up).not.toContain('SCHEMA') }) }) + +// The target's own server says which versions it can reach. PostgreSQL has +// no way down a version: ALTER EXTENSION pg_cron UPDATE TO '1.6' against a +// target on 1.6.4 failed, and with it the whole apply. +describe('ExtensionsCheck: versions the target cannot reach', () => { + const scan = (paths: Array<{ name: string; version: string }>) => new ExtensionsCheck(async (dbUrl, sql) => { + if (sql.includes('pg_extension_update_paths')) return paths as never + return [makeExt({ version: dbUrl.includes('source') ? '1.6' : '1.6.4' })] + }).scan(mockContext()) + + it('reports the mismatch with no fix, and why', async () => { + const [issue] = await scan([]) + expect(issue.title).toContain('version mismatch') + expect(issue.sql).toBeUndefined() + expect(issue.manualOnly).toContain('cannot go from v1.6.4 to v1.6') + }) + + it('still offers the update when the target can make it', async () => { + const [issue] = await scan([{ name: makeExt().name, version: '1.6' }]) + expect(issue.sql?.up).toContain("UPDATE TO '1.6'") + expect(issue.manualOnly).toBeUndefined() + }) +}) + +describe('diffExtensions: an extension in a schema of its own', () => { + it('makes the schema first, which plain PostgreSQL may not have', () => { + const [issue] = diffExtensions([{ name: 'pgjwt', version: '0.2.0', schema: 'extensions' }], []) + expect(issue.sql?.up).toBe('CREATE SCHEMA IF NOT EXISTS "extensions";\nCREATE EXTENSION IF NOT EXISTS "pgjwt" SCHEMA "extensions";') + }) + + it('needs no schema made in public', () => { + const [issue] = diffExtensions([{ name: 'pgcrypto', version: '1.3', schema: 'public' }], []) + expect(issue.sql?.up).toBe('CREATE EXTENSION IF NOT EXISTS "pgcrypto";') + }) +}) From b948099570f017612c485aa609bbf751a8c86143 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Sun, 18 Oct 2026 13:13:37 +0000 Subject: [PATCH 08/12] fix: restoring into an older server leaves out what it cannot hold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A snapshot of a newer PostgreSQL holds what an older one has no syntax or feature for — a VIRTUAL generated column before 18, UNIQUE NULLS NOT DISTINCT before 15 — and in one transaction that statement rolled the whole restore back. The snapshot's version is read from pg_dump's header in schema.sql; restoring into an older server, such a statement is left out and named, as one needing a missing extension is, and the rest is restored. Into a server as new, it still fails the restore. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/pg-errors.ts | 8 ++++++++ packages/cli/src/promote.ts | 9 +-------- packages/cli/src/restore.ts | 28 +++++++++++++++++++++++--- packages/cli/test/review-fixes.test.ts | 10 +++++++++ 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/pg-errors.ts b/packages/cli/src/pg-errors.ts index 0b1fb69..ec30d7e 100644 --- a/packages/cli/src/pg-errors.ts +++ b/packages/cli/src/pg-errors.ts @@ -15,3 +15,11 @@ export function sqlState(err: unknown): string | undefined { const code = (err as { code?: unknown } | null)?.code return typeof code === 'string' ? code : undefined } + +/** + * How an older server refuses what a newer one wrote: a syntax it does not + * know (`UNIQUE NULLS NOT DISTINCT` before 15, a `VIRTUAL` column before 18), + * a feature it lacks, a parameter it does not have (`security_invoker` before + * 15). Taken as the target lacking it only when the target is older. + */ +export const OLDER_SERVER: ReadonlySet = new Set(['42601', '0A000', '22023']) diff --git a/packages/cli/src/promote.ts b/packages/cli/src/promote.ts index 26a9b5b..b2f5d62 100644 --- a/packages/cli/src/promote.ts +++ b/packages/cli/src/promote.ts @@ -2,7 +2,7 @@ import pg from 'pg' import { newPgClient } from './db.js' import type { ScanResult, SyncAction } from './types/drift' import { errMsg } from './utils/error' -import { ABSENT_ON_TARGET, EXTENSION_UNAVAILABLE, UNDEFINED_COLUMN, sqlState } from './pg-errors.js' +import { ABSENT_ON_TARGET, EXTENSION_UNAVAILABLE, OLDER_SERVER, UNDEFINED_COLUMN, sqlState } from './pg-errors.js' import { destructiveReason } from './dbdiff' import { parseStatements } from './sql-ast' import { estimateLocks, lockBudgetWarning, readLockBudget } from './lock-budget.js' @@ -636,13 +636,6 @@ export function revokesFromAbsent(sql: string): boolean { return targets.size === 1 } -/** - * How an older server refuses what a newer one wrote: a syntax it does not - * know (`UNIQUE NULLS NOT DISTINCT` before 15, a `VIRTUAL` column before 18), - * a feature it lacks, a parameter it does not have (`security_invoker` before - * 15). Taken as the target lacking it only when the target is older. - */ -const OLDER_SERVER: ReadonlySet = new Set(['42601', '0A000', '22023']) /** One transaction; true when it committed. */ async function runBatch( diff --git a/packages/cli/src/restore.ts b/packages/cli/src/restore.ts index 8a85e9c..1db0962 100644 --- a/packages/cli/src/restore.ts +++ b/packages/cli/src/restore.ts @@ -17,7 +17,7 @@ import { dropUnsupportedSetStatements, knownParameters } from './prove' import { splitSqlStatements, isCommentOnly, isPsqlMetaCommand, stripPsqlMetaCommands } from './utils/sql-split' import { quoteIdent, quoteLiteral, quoteName } from './utils/sql.js' import { sqlSkeleton, statementSubject, rolesNamedBy } from './sql-deps.js' -import { ABSENT_ON_TARGET, EXTENSION_UNAVAILABLE } from './pg-errors.js' +import { ABSENT_ON_TARGET, EXTENSION_UNAVAILABLE, OLDER_SERVER } from './pg-errors.js' import { dataTablesInOrder, restoreBuckets, restoreTableRows } from './restore-data.js' import { replaceableSchemas, findExternalDependents, dropSchemaContents, recreateExternalDependents, @@ -88,6 +88,15 @@ export interface RestoreResult { warnings?: string[] } +/** Whether the snapshot's server, as pg_dump's header names it, is a newer major version than the target's. */ +async function snapshotIsNewer(snapshotDir: string, client: Pick): Promise { + const head = (await readFile(join(snapshotDir, 'schema.sql'), 'utf-8').catch(() => '')).slice(0, 2000) + const dumped = /Dumped from database version (\d+)/.exec(head)?.[1] + if (!dumped) return false + const { rows: [row] } = await client.query<{ v: string }>(`SELECT current_setting('server_version_num') AS v`) + return Number(dumped) > Math.floor(Number(row?.v ?? 0) / 10000) +} + /** About how many locks replaying the snapshot's SQL layers in one transaction holds. */ async function locksForSnapshot( snapshotDir: string, @@ -149,6 +158,10 @@ export async function restoreFromSnapshot(options: RestoreOptions): Promise, missing: ReadonlySet = new Set(), leftOut: ReadonlyMap = new Map(), + olderTarget = false, ): string | undefined { const code = (err as { code?: string } | null)?.code if (!code) return undefined const skeleton = sqlSkeleton(sql) + // A snapshot of a newer server holds what an older one has no syntax or + // feature for: a VIRTUAL generated column before 18, UNIQUE NULLS NOT + // DISTINCT before 15. Left out and named, as a missing extension is. + if (olderTarget && OLDER_SERVER.has(code)) { + return `this server's PostgreSQL is older than the snapshot's and does not support it: ${errMsg(err)}` + } + if (/^\s*CREATE\s+EXTENSION\b/i.test(skeleton)) { return EXTENSION_UNAVAILABLE.has(code) ? `extension not available on this server: ${errMsg(err)}` @@ -899,6 +920,7 @@ async function applyStatement( result: RestoreResult, missing: Set, leftOut: Map = new Map(), + olderTarget = false, ): Promise { if (transactional) await client.query('SAVEPOINT sf_restore_statement') try { @@ -920,7 +942,7 @@ async function applyStatement( if (transactional) await client.query('ROLLBACK TO SAVEPOINT sf_restore_statement') const { rows } = await client.query<{ nspname: string }>('SELECT nspname FROM pg_namespace') const targetSchemas = new Set(rows.map(r => r.nspname)) - const tolerated = tolerableFailure(sql, err, targetSchemas, missing, leftOut) + const tolerated = tolerableFailure(sql, err, targetSchemas, missing, leftOut, olderTarget) if (tolerated) { result.skipped.push({ type: 'sql', label: summarizeStatement(sql), reason: tolerated }) // What this would have created, and why it is not there, for whatever diff --git a/packages/cli/test/review-fixes.test.ts b/packages/cli/test/review-fixes.test.ts index d34191b..7029494 100644 --- a/packages/cli/test/review-fixes.test.ts +++ b/packages/cli/test/review-fixes.test.ts @@ -42,6 +42,16 @@ describe('platformOwnedObject by subject', () => { }) describe('tolerableFailure', () => { + // A snapshot of PostgreSQL 18 holds a VIRTUAL generated column, which 17 + // cannot parse: into an older server that statement is left out, and the + // restore goes on; into one as new it is still a real failure. + it('leaves out what an older server has no syntax for, only when it is older', () => { + const err = Object.assign(new Error('syntax error at or near ")"'), { code: '42601' }) + const sql = 'CREATE TABLE public.v (a integer, b integer GENERATED ALWAYS AS ((a * 2)));' + expect(tolerableFailure(sql, err, new Set(), new Set(), new Map(), true)).toContain('older than the snapshot') + expect(tolerableFailure(sql, err, new Set(), new Set(), new Map(), false)).toBeUndefined() + }) + const plain = new Set(['public', 'pg_catalog', 'information_schema']) const supabase = new Set([...plain, 'auth', 'storage', 'graphql', 'extensions']) const missing = (what: string) => ({ code: '3F000', message: `schema "${what}" does not exist` }) From 63c920679e3ff40680516f50f921384b658ed814 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Sun, 18 Oct 2026 13:21:13 +0000 Subject: [PATCH 09/12] feat: a scan reads each database as of one instant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The checks run many queries, concurrently, over seconds or minutes, and a change made meanwhile — a migration, a policy someone adds — was seen by some of them and not others: the report could describe a database that never existed. While a scan runs, one connection to each database holds a REPEATABLE READ, READ ONLY transaction that exports its snapshot, and each pooled connection joins it once. A query that fails ends that connection's transaction, which joins again next time; a definition rendered with a temporary object is undone to a savepoint. Both are let go when the scan ends, before anything is applied. A server that cannot export a snapshot is read as before. As with pg_dump, what the scan reads stays locked until it ends; --no-snapshot reads each query on its own. Co-Authored-By: Claude Opus 5.5 --- packages/cli/README.md | 1 + packages/cli/src/commands/diff.ts | 8 +- packages/cli/src/db.ts | 101 ++++++++++++++++++- packages/cli/src/scanner.ts | 20 +++- packages/cli/test/e2e/snapshot-reads.test.ts | 62 ++++++++++++ 5 files changed, 182 insertions(+), 10 deletions(-) create mode 100644 packages/cli/test/e2e/snapshot-reads.test.ts diff --git a/packages/cli/README.md b/packages/cli/README.md index 9e3c593..8870125 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -125,6 +125,7 @@ Useful `diff` flags: | `--allow-destructive` | Also apply drops and deletes | | `--apply-posture` | Also apply the target-only checks' fixes | | `--no-transaction` | Apply fix by fix, keeping what works | +| `--no-snapshot` | Read each query on its own. By default each database is read as of one instant, so a change made while the scan runs is seen by all of it or none; as with `pg_dump`, what it reads stays locked until the scan ends | | `--json` / `--ci` | Machine-readable output; CI annotations and exit codes | ## Applying fixes safely diff --git a/packages/cli/src/commands/diff.ts b/packages/cli/src/commands/diff.ts index cb6b566..048f602 100644 --- a/packages/cli/src/commands/diff.ts +++ b/packages/cli/src/commands/diff.ts @@ -176,6 +176,10 @@ export default class Diff extends BaseCommand { description: 'With --apply, print the fixes in the order they would run without executing any of them', default: false, }), + 'no-snapshot': Flags.boolean({ + description: 'Read each query on its own, rather than each database as of one instant (which holds what it reads locked until the scan ends, as pg_dump does)', + default: false, + }), 'no-transaction': Flags.boolean({ description: 'With --apply, run each fix on its own instead of rolling the whole set back on the first failure', aliases: ['continue-on-error'], @@ -396,7 +400,7 @@ export default class Diff extends BaseCommand { // the flag that performs one (issue #71). if (flags.apply || flags['dry-run']) { const onProgress = makeProgress() - const scanResult = await scan(registry, { config, checks, skip, tableFilter, onProgress, onDetail: makeDetail() }) + const scanResult = await scan(registry, { config, checks, skip, tableFilter, onProgress, onDetail: makeDetail(), snapshot: !flags['no-snapshot'] }) this.setCheckSummaries(scanResult.checks.map(c => ({ check: c.check, status: c.status, @@ -560,7 +564,7 @@ export default class Diff extends BaseCommand { // ── Scan mode (summary, detail, CI, or JSON) ──────────────────────────── const onProgress = makeProgress() - const result = await scan(registry, { config, checks, skip, tableFilter, onProgress, onDetail: makeDetail() }) + const result = await scan(registry, { config, checks, skip, tableFilter, onProgress, onDetail: makeDetail(), snapshot: !flags['no-snapshot'] }) this.setCheckSummaries(result.checks.map(c => ({ check: c.check, status: c.status, diff --git a/packages/cli/src/db.ts b/packages/cli/src/db.ts index 4dd6160..9c8fe1d 100644 --- a/packages/cli/src/db.ts +++ b/packages/cli/src/db.ts @@ -103,9 +103,95 @@ function poolFor(dbUrl: string): pg.Pool { return pool } +/** + * A scan reads each database as of one instant. + * + * The checks run many queries, concurrently, over seconds or minutes; a + * change made meanwhile — a migration, a policy someone adds — was seen by + * some and not others, and the report described a database that never + * existed. While a database is held (holdSnapshot), one connection keeps a + * REPEATABLE READ, READ ONLY transaction open that exports its snapshot, and + * each pooled connection joins it once, in a REPEATABLE READ transaction of + * its own — not read only, so a definition can still be rendered with a + * temporary object, inside a savepoint. A query that fails ends that + * connection's transaction; it is rolled back and joins again next time. + * + * As with pg_dump, what the reads lock stays locked until the scan ends, so + * releaseSnapshots() runs before anything is applied. + */ +const snapshots = new Map() +const joined = new WeakMap() + +/** Read `dbUrl` as of one instant until releaseSnapshots(). False, and nothing held, when the server will not. */ +export async function holdSnapshot(dbUrl: string): Promise { + if (snapshots.has(dbUrl)) return true + const anchor = newPgClient(dbUrl) + try { + await anchor.connect() + await anchor.query('BEGIN ISOLATION LEVEL REPEATABLE READ, READ ONLY') + const { rows: [row] } = await anchor.query<{ id: string }>('SELECT pg_export_snapshot() AS id') + snapshots.set(dbUrl, { id: row.id, anchor }) + return true + } catch { + await anchor.end().catch(() => undefined) + return false + } +} + +/** The snapshot `dbUrl` is read as of, while it is held. */ +export function heldSnapshot(dbUrl: string): string | undefined { + return snapshots.get(dbUrl)?.id +} + +/** + * Let every held database go. The pools reading them are closed with them: + * their connections sit in the snapshot's transactions. + */ +export async function releaseSnapshots(): Promise { + const held = [...snapshots.entries()] + snapshots.clear() + await Promise.all(held.map(async ([dbUrl, { anchor }]) => { + const pool = pools.get(dbUrl) + pools.delete(dbUrl) + await pool?.end().catch(() => undefined) + await anchor.end().catch(() => undefined) + })) +} + +/** A pooled connection, in the held snapshot's transaction when `dbUrl` is held. */ +async function connectionFor(dbUrl: string): Promise { + const client = await poolFor(dbUrl).connect() + const id = snapshots.get(dbUrl)?.id + if (id && joined.get(client) !== id) { + try { + await client.query(`BEGIN ISOLATION LEVEL REPEATABLE READ; SET TRANSACTION SNAPSHOT '${id.replace(/'/g, "''")}'`) + joined.set(client, id) + } catch (err) { + await client.query('ROLLBACK').catch(() => undefined) + client.release() + throw err + } + } + return client +} + export const pgQuery: QueryFn = async (dbUrl, sql, params) => { - const { rows } = await poolFor(dbUrl).query(sql, params) - return rows + if (!snapshots.has(dbUrl)) { + const { rows } = await poolFor(dbUrl).query(sql, params) + return rows + } + const client = await connectionFor(dbUrl) + try { + const { rows } = await client.query(sql, params) + return rows + } catch (err) { + // The transaction is over for this connection; it joins again next time. + await client.query('ROLLBACK').catch(() => undefined) + joined.delete(client) + throw err + } finally { + client.release() + } } /** @@ -117,13 +203,18 @@ export async function inRolledBackTransaction( dbUrl: string, fn: (client: pg.PoolClient) => Promise, ): Promise { - const client = await poolFor(dbUrl).connect() + const client = await connectionFor(dbUrl) + // Inside the held snapshot's transaction, a savepoint does what a + // transaction of its own would. + const [begin, undo] = joined.has(client) + ? ['SAVEPOINT sf_scratch', 'ROLLBACK TO SAVEPOINT sf_scratch'] + : ['BEGIN', 'ROLLBACK'] try { - await client.query('BEGIN') + await client.query(begin) try { return await fn(client) } finally { - await client.query('ROLLBACK').catch(() => undefined) + await client.query(undo).catch(() => undefined) } } finally { client.release() diff --git a/packages/cli/src/scanner.ts b/packages/cli/src/scanner.ts index b85bf36..ea29ede 100644 --- a/packages/cli/src/scanner.ts +++ b/packages/cli/src/scanner.ts @@ -1,3 +1,4 @@ +import { holdSnapshot, releaseSnapshots } from './db.js' import { policyOnlyKeys, sqlSkeleton } from './sql-deps.js' import { splitSqlStatements } from './utils/sql-split.js' import type { HookBus } from './hooks' @@ -32,6 +33,11 @@ export interface ScanOptions { * Merged with the config's own `checks.tables` / `checks.excludeTables`. */ tableFilter?: TableFilter + /** + * Read each database as of one instant (see holdSnapshot). On by default; + * what the scan reads stays locked until it ends, as with pg_dump. + */ + snapshot?: boolean } /** @@ -203,9 +209,17 @@ export async function scan( } } - await Promise.all( - Array.from({ length: Math.min(checkConcurrency(), total) }, () => worker()), - ) + // Each side read as of one instant, and let go before anything is applied. + if (options.snapshot !== false) { + await Promise.all([source?.dbUrl, target?.dbUrl].filter(Boolean).map(url => holdSnapshot(url as string))) + } + try { + await Promise.all( + Array.from({ length: Math.min(checkConcurrency(), total) }, () => worker()), + ) + } finally { + if (options.snapshot !== false) await releaseSnapshots() + } foldDuplicatePolicyFindings(results) foldDuplicateExtensionFindings(results) diff --git a/packages/cli/test/e2e/snapshot-reads.test.ts b/packages/cli/test/e2e/snapshot-reads.test.ts new file mode 100644 index 0000000..335f268 --- /dev/null +++ b/packages/cli/test/e2e/snapshot-reads.test.ts @@ -0,0 +1,62 @@ +/** + * A scan reads each database as of one instant, against a real server. + * + * A table made while the database is held is not seen by any read until it + * is let go; a read that fails, and a definition rendered with a temporary + * object, leave the reads after them working. + */ +import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest' +import { PgHarness } from '../harness/PgHarness.js' +import { describeWithContainers } from '../harness/containers.js' +import { holdSnapshot, heldSnapshot, inRolledBackTransaction, pgQuery, releaseSnapshots } from '../../src/db.js' + +const describeE2E = describeWithContainers() + +describeE2E('e2e: reads as of one instant', () => { + let h: PgHarness + + beforeAll(async () => { + h = new PgHarness({ verbose: !!process.env.E2E_VERBOSE }) + await h.up() + }, 300_000) + + afterEach(async () => { await releaseSnapshots() }) + afterAll(async () => { await h?.down() }, 120_000) + + const count = async (url: string, name: string) => + Number((await pgQuery(url, 'SELECT count(*) AS n FROM pg_class WHERE relname = $1', [name]))[0].n) + + it('does not see a table made while the database is held, until it is let go', async () => { + const url = h.connectionString('source') + expect(await holdSnapshot(url)).toBe(true) + expect(heldSnapshot(url)).toBeTruthy() + await count(url, 'made_meanwhile') + + await h.applySql('source', 'CREATE TABLE public.made_meanwhile (id int);') + + // Several reads, on several pooled connections, all as of the instant held. + expect(await Promise.all([1, 2, 3, 4, 5, 6].map(() => count(url, 'made_meanwhile')))).toEqual([0, 0, 0, 0, 0, 0]) + + await releaseSnapshots() + expect(heldSnapshot(url)).toBeUndefined() + expect(await count(url, 'made_meanwhile')).toBe(1) + }, 120_000) + + it('keeps reading after a read that fails and a rendering with a temporary object', async () => { + const url = h.connectionString('target') + await h.applySql('target', 'CREATE TABLE public.kept (id int);') + expect(await holdSnapshot(url)).toBe(true) + await h.applySql('target', 'CREATE TABLE public.later (id int);') + + await expect(pgQuery(url, 'SELECT * FROM no_such_table')).rejects.toThrow(/no_such_table/) + const rendered = await inRolledBackTransaction(url, async client => { + await client.query('CREATE TEMP TABLE sf_probe (x int CHECK (x > 0))') + return (await client.query(`SELECT pg_get_constraintdef(oid) AS def FROM pg_constraint WHERE conrelid = 'sf_probe'::regclass`)).rows[0].def + }) + expect(rendered).toBe('CHECK ((x > 0))') + + expect(await count(url, 'kept')).toBe(1) + expect(await count(url, 'later')).toBe(0) + expect(await count(url, 'sf_probe')).toBe(0) + }, 120_000) +}) From a61663c5cb0c8b2b1f1d21d512a3df1cbe3b8ff9 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Sun, 18 Oct 2026 13:23:32 +0000 Subject: [PATCH 10/12] fix: an extension in a system schema is created without making the schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pg_cron is in pg_catalog on Supabase, and creating the schema an extension goes in first wrote CREATE SCHEMA IF NOT EXISTS "pg_catalog", which PostgreSQL refuses ("unacceptable schema name") — failing the whole apply from Supabase to plain PostgreSQL. A system schema is always there; only another one is made first. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/checks/extensions.ts | 11 +++++++---- packages/cli/test/checks/extensions.test.ts | 5 +++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/checks/extensions.ts b/packages/cli/src/checks/extensions.ts index 32b72b5..0af2448 100644 --- a/packages/cli/src/checks/extensions.ts +++ b/packages/cli/src/checks/extensions.ts @@ -84,10 +84,13 @@ export function diffExtensions(source: PgExtension[], target: PgExtension[], rea sql: { // Into the schema it is in on the source, which plain PostgreSQL // may not have — Supabase's `extensions`. Made first; if the server - // does not ship the extension, the fix is left out with it. - up: ext.schema !== 'public' - ? `CREATE SCHEMA IF NOT EXISTS ${quoteName(ext.schema)};\nCREATE EXTENSION IF NOT EXISTS ${quoteName(name)} SCHEMA ${quoteName(ext.schema)};` - : `CREATE EXTENSION IF NOT EXISTS ${quoteName(name)};`, + // does not ship the extension, the fix is left out with it. A + // system schema (pg_cron's pg_catalog) is always there, and cannot + // be made. + up: ext.schema === 'public' + ? `CREATE EXTENSION IF NOT EXISTS ${quoteName(name)};` + : `${ext.schema.startsWith('pg_') ? '' : `CREATE SCHEMA IF NOT EXISTS ${quoteName(ext.schema)};\n`}` + + `CREATE EXTENSION IF NOT EXISTS ${quoteName(name)} SCHEMA ${quoteName(ext.schema)};`, down: `DROP EXTENSION IF EXISTS ${quoteName(name)};`, }, }) diff --git a/packages/cli/test/checks/extensions.test.ts b/packages/cli/test/checks/extensions.test.ts index b1fcd35..f6af96b 100644 --- a/packages/cli/test/checks/extensions.test.ts +++ b/packages/cli/test/checks/extensions.test.ts @@ -134,6 +134,11 @@ describe('diffExtensions: an extension in a schema of its own', () => { expect(issue.sql?.up).toBe('CREATE SCHEMA IF NOT EXISTS "extensions";\nCREATE EXTENSION IF NOT EXISTS "pgjwt" SCHEMA "extensions";') }) + it('makes no system schema, which is always there and cannot be made', () => { + const [issue] = diffExtensions([{ name: 'pg_cron', version: '1.6', schema: 'pg_catalog' }], []) + expect(issue.sql?.up).toBe('CREATE EXTENSION IF NOT EXISTS "pg_cron" SCHEMA "pg_catalog";') + }) + it('needs no schema made in public', () => { const [issue] = diffExtensions([{ name: 'pgcrypto', version: '1.3', schema: 'public' }], []) expect(issue.sql?.up).toBe('CREATE EXTENSION IF NOT EXISTS "pgcrypto";') From f74f278fae22aec95d1766d55b45d764acce75f7 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Sun, 18 Oct 2026 13:30:07 +0000 Subject: [PATCH 11/12] fix: an extension the target does not ship is left out, schema and all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An extension in a schema of its own makes that schema first, so its fix no longer began with CREATE EXTENSION, and one the target's server does not ship — pg_graphql on plain PostgreSQL — was no longer recognised as such: it failed the whole apply instead of being left out. The fix is now recognised by a CREATE EXTENSION anywhere in it. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/promote.ts | 3 ++- packages/cli/test/promote-savepoints.test.ts | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/promote.ts b/packages/cli/src/promote.ts index b2f5d62..d3c10d8 100644 --- a/packages/cli/src/promote.ts +++ b/packages/cli/src/promote.ts @@ -607,7 +607,8 @@ export function unmetDependency(sql: string, err: unknown, olderTarget = false): if (olderTarget && OLDER_SERVER.has(code)) { return "the target's PostgreSQL is older than the source's and does not support it" } - if (/^\s*CREATE\s+EXTENSION\b/i.test(sqlSkeleton(sql))) { + // Anywhere in the fix: one into a schema of its own makes the schema first. + if (/(?:^|;)\s*CREATE\s+EXTENSION\b/i.test(sqlSkeleton(sql))) { return EXTENSION_UNAVAILABLE.has(code) ? 'this server does not ship it' : undefined } if (ABSENT_ON_TARGET.has(code) || code === UNDEFINED_COLUMN) { diff --git a/packages/cli/test/promote-savepoints.test.ts b/packages/cli/test/promote-savepoints.test.ts index e1a2fd1..ce3e6a6 100644 --- a/packages/cli/test/promote-savepoints.test.ts +++ b/packages/cli/test/promote-savepoints.test.ts @@ -141,6 +141,9 @@ describe('unmetDependency', () => { it('accepts an extension the server does not ship, only for CREATE EXTENSION', () => { expect(unmetDependency('CREATE EXTENSION IF NOT EXISTS vector', err('0A000'))).toContain('does not ship') + // Into a schema of its own, which the fix makes first. + expect(unmetDependency('CREATE SCHEMA IF NOT EXISTS "graphql";\nCREATE EXTENSION IF NOT EXISTS "pg_graphql" SCHEMA "graphql";', err('0A000'))) + .toContain('does not ship') expect(unmetDependency('CREATE EXTENSION vector', err('58P01'))).toContain('does not ship') // 0A000 is "feature not supported" in general: anywhere else it is a real failure. expect(unmetDependency('ALTER TABLE t ALTER COLUMN c TYPE int', err('0A000'))).toBeUndefined() From f598f1e8532c7cd6390f1cd339b112d355fd30df Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Sun, 18 Oct 2026 13:49:31 +0000 Subject: [PATCH 12/12] fix: --prove matches a NOT NULL constraint by its column, not its name From PostgreSQL 18 a NOT NULL is a constraint with a name: a table or column renamed keeps the old one, and one made afresh gets a default. The proof compares the migrated copy with the source's structure made anew on the target's server, so a rename from an older source read as its NOT NULLs missing and others unexpected, and the migration was refused. They are now matched by the column they hold for. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/state-diff.ts | 5 ++++- packages/cli/test/state-diff.test.ts | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/state-diff.ts b/packages/cli/src/state-diff.ts index 9cbdca5..ccb4832 100644 --- a/packages/cli/src/state-diff.ts +++ b/packages/cli/src/state-diff.ts @@ -100,8 +100,11 @@ function compareTable(out: string[], subject: string, a: StateTable, b: StateTab compareKeyed(out, 'column', a.columns, b.columns, c => `${prefix}.${c.name}`, (o, s, x, y) => compareColumn(o, s, x, y)) + // A NOT NULL, a constraint from PostgreSQL 18, by the column it holds for: + // its name is a table's or column's old one after a rename, or one the + // server picked, and an older source has none to compare. compareKeyed(out, `constraint on ${prefix}`, a.constraints, b.constraints, - c => c.name, + c => c.type === 'not_null' ? c.definition : c.name, (o, s, x, y) => { compareField(o, s, 'definition', x.definition, y.definition) compareField(o, s, 'validated', x.validated, y.validated) diff --git a/packages/cli/test/state-diff.test.ts b/packages/cli/test/state-diff.test.ts index 18b7e01..cf7b440 100644 --- a/packages/cli/test/state-diff.test.ts +++ b/packages/cli/test/state-diff.test.ts @@ -104,4 +104,20 @@ describe('diffState', () => { 'sequence public.free_seq: unexpected', ]) }) + + // A table renamed on PostgreSQL 18 keeps its NOT NULLs' old names; one made + // afresh, or on an older server, has others or none. Matched by column. + it('matches a NOT NULL constraint by its column, not its name', () => { + const nn = (name: string, column: string) => + ({ name, type: 'not_null', definition: `NOT NULL ${column}`, validated: true, deferrable: false, deferred: false }) + const a = empty(); a.tables = [table({ constraints: [nn('config_key_not_null', 'key')] }) as never] + const b = empty(); b.tables = [table({ constraints: [nn('settings_key_not_null', 'key')] }) as never] + expect(diffState(a, b)).toEqual([]) + + const c = empty(); c.tables = [table({ constraints: [nn('config_key_not_null', 'value')] }) as never] + expect(diffState(a, c)).toEqual([ + 'constraint on public.t NOT NULL key: missing', + 'constraint on public.t NOT NULL value: unexpected', + ]) + }) })