From 7a5d7ca2ac8551c51dc09b087062aa850793d5a8 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Wed, 21 Oct 2026 23:27:20 +0000 Subject: [PATCH] feat: migrate create writes the grants on what it makes A migration file is replayed where the default privileges may differ from the target it was written against: a new project, a preview branch, a local reset. Supabase no longer grants the Data API roles on new tables in public by default, so a table whose access came from the target's defaults would arrive without it. migrate create now writes every grant the source holds on the objects it creates, even those the target's defaults would give, and revokes them in the DOWN. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/checks/roles.ts | 10 +++++++-- packages/cli/src/commands/migrate/create.ts | 17 +++++++++++--- packages/cli/test/checks/roles.test.ts | 11 +++++++++ packages/cli/test/e2e/lifecycle.test.ts | 25 ++++++++++++++++++++- 4 files changed, 57 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/checks/roles.ts b/packages/cli/src/checks/roles.ts index 36148a4..f511377 100644 --- a/packages/cli/src/checks/roles.ts +++ b/packages/cli/src/checks/roles.ts @@ -368,7 +368,13 @@ function extraGrantSeverity(grantee: string): 'critical' | 'warning' { export class RolesCheck extends Check { readonly name = 'roles' as const - constructor(private queryFn: QueryFn = pgQuery) { + /** + * `explicit`: report a grant the source has even where the target's + * default privileges would make it. A migration file is replayed on + * databases whose defaults differ — Supabase stopped granting the Data API + * roles on new tables in `public` — so it must say every grant itself. + */ + constructor(private queryFn: QueryFn = pgQuery, private options: { explicit?: boolean } = {}) { super() } @@ -412,7 +418,7 @@ export class RolesCheck extends Check { 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 ?? []) + const byDefault = this.options.explicit ? () => false : grantedByDefault(targetObjects, targetDefaults ?? []) return [ ...diffRoles(sourceRoles, targetRoles), diff --git a/packages/cli/src/commands/migrate/create.ts b/packages/cli/src/commands/migrate/create.ts index 1e85d60..6732d1c 100644 --- a/packages/cli/src/commands/migrate/create.ts +++ b/packages/cli/src/commands/migrate/create.ts @@ -3,6 +3,9 @@ import { join } from 'node:path' import { Flags } from '@oclif/core' import { BaseCommand } from '../../base-command.js' import { runDbDiff, isDestructiveSql } from '../../dbdiff.js' +import { RolesCheck } from '../../checks/roles.js' +import type { CheckContext } from '../../checks/base.js' +import { pgQuery } from '../../db.js' import { DEFAULT_IGNORE_SCHEMAS } from '../../defaults.js' import { ok, warn, dim, bold, cmd } from '../../ui.js' import { splitSqlStatements } from '../../utils/sql-split.js' @@ -63,7 +66,14 @@ export default class MigrateCreate extends BaseCommand { ignoreSchemas, }) - if (!result.up.trim()) { + // The grants on what it makes, written out: the file is replayed where + // default privileges differ — a new project, a preview branch, a local + // reset — and Supabase no longer grants the Data API roles on new tables. + const grants = (await new RolesCheck(pgQuery, { explicit: true }).scan({ + source: sourceEnv, target: targetEnv, config, + } as CheckContext)).filter(i => i.id.startsWith('roles-grant-') && i.sql?.up) + + if (!result.up.trim() && grants.length === 0) { this.log(`${ok('No schema drift detected.')} Nothing to write. ✓`) return } @@ -91,15 +101,16 @@ export default class MigrateCreate extends BaseCommand { : []), '', result.up, + ...(grants.length > 0 ? ['', '-- Grants', ...grants.map(i => i.sql!.up)] : []), ] - if (result.down.trim()) { + if (result.down.trim() || grants.length > 0) { lines.push( '', '-- ==================== DOWN ====================', '-- To revert this migration, run the SQL below manually:', '--', - ...result.down.split('\n').map(l => `-- ${l}`), + ...[...grants.map(i => i.sql?.down ?? '').filter(Boolean).reverse(), result.down].join('\n').split('\n').map(l => `-- ${l}`), ) } diff --git a/packages/cli/test/checks/roles.test.ts b/packages/cli/test/checks/roles.test.ts index de95c3f..49409b6 100644 --- a/packages/cli/test/checks/roles.test.ts +++ b/packages/cli/test/checks/roles.test.ts @@ -444,6 +444,17 @@ describe('a new table keeps the source\'s grants, not the target\'s defaults', ( it('says nothing about a table the target already has', async () => { expect((await issues()).some(i => i.id.endsWith('public.shared'))).toBe(false) }) + + // A migration file is replayed where defaults differ — Supabase no longer + // grants the Data API roles on new tables — so it writes every grant. + it('writes out what the defaults would grant, when asked to be explicit', async () => { + const scan = async (explicit: boolean) => (await new RolesCheck( + (async (dbUrl: string, sql: string) => target(sql, dbUrl)) as unknown as QueryFn, { explicit }, + ).scan(mockContext())).filter(i => i.id.startsWith('roles-grant-missing') && i.id.includes('ro_items')) + expect(await scan(false)).toEqual([]) + const explicit = await scan(true) + expect(explicit.map(i => i.sql?.up).join('\n')).toMatch(/GRANT .*SELECT.* ON "public"\."ro_items" TO "anon";/) + }) }) describe('an extra grant to a Data API role is not a footnote', () => { diff --git a/packages/cli/test/e2e/lifecycle.test.ts b/packages/cli/test/e2e/lifecycle.test.ts index 69ab21a..3e0f2e1 100644 --- a/packages/cli/test/e2e/lifecycle.test.ts +++ b/packages/cli/test/e2e/lifecycle.test.ts @@ -15,7 +15,7 @@ * containers and workspaces up for inspection after a failure. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest' -import { readdir, mkdtemp, writeFile } from 'node:fs/promises' +import { readdir, mkdtemp, readFile, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { PgHarness } from '../harness/PgHarness.js' @@ -166,6 +166,29 @@ describeE2E('e2e: real-database lifecycle', () => { expect(r.stdout + r.stderr).toMatch(/referral|migration/i) }, 300_000) + // A migration is replayed where default privileges differ — Supabase no + // longer grants the Data API roles on new tables — so a table it makes + // carries its grants in the file, even where the target's defaults would + // have given them. + it('writes the grants on a new table into the file', async () => { + const role = "DO $$ BEGIN CREATE ROLE sf_api_reader NOLOGIN; EXCEPTION WHEN duplicate_object THEN NULL; END $$;" + await h.applySql('source', role + ' CREATE TABLE public.sf_granted (id int PRIMARY KEY); GRANT SELECT ON public.sf_granted TO sf_api_reader;') + await h.applySql('target', role + ' ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT SELECT ON TABLES TO sf_api_reader;') + const dir = await mkdtemp(join(tmpdir(), 'sf-migrations-')) + const mws = await h.workspace({ checks: { migrations: { dir } } }) + try { + const r = await h.cli(['migrate', 'create', '-n', 'granted_table'], { cwd: mws }) + expect(r.code, r.stdout + r.stderr).toBe(0) + const [file] = await readdir(dir) + const sql = await readFile(join(dir, file), 'utf8') + expect(sql).toContain('CREATE TABLE public.sf_granted') + expect(sql).toMatch(/GRANT SELECT ON "public"\."sf_granted" TO "sf_api_reader";/) + } finally { + await h.applySql('source', 'DROP TABLE IF EXISTS public.sf_granted;') + await h.applySql('target', 'ALTER DEFAULT PRIVILEGES IN SCHEMA public REVOKE SELECT ON TABLES FROM sf_api_reader;') + } + }, 300_000) + // `migrate run` was the one writing command that executed when run bare; // every other previews until given --apply. it('previews pending migrations unless given --apply', async () => {