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 () => {