2288419a | ci: run the Supabase RLS/stack integration suite on every PR | Amalanand Muthukumaran | 2026-07-25 | ↗ GitHub |
commit body The gated stack tests (backend/src/__tests__/integration/*.supabase.test.ts)
prove the deny-all RLS firewall and the auth<->API contract against a real
local Supabase stack, but no CI trigger ever set the SUPABASE_TEST_* env
vars, so they silently self-skipped on every PR. Add a workflow that boots
the stack on the runner (supabase CLI pinned, minimal service set: db, auth,
rest, kong), bootstraps the database the way backend/scripts/test-stack.sh
does, and runs the suite. No secrets: everything is local to the runner.
The bootstrap loads schema.sql and then applies every dated migration on
top in filename order, which doubles as a schema-drift smoke test: the
snapshot and the migrations must apply cleanly together. Running it
surfaced five migrations that could not apply on top of the current
schema.sql - three whose backfills read documents columns that later
migrations moved to document_versions, and two overview RPCs whose return
row type `create or replace` cannot change. Guard the backfills on the
historical columns' existence and drop-before-create the RPCs (the pattern
20260703_02 already uses); behavior on era deployments is unchanged, and
the full sequence now applies cleanly end to end (verified locally: fresh
stack, schema + 44 migrations, 5/5 stack tests green).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
ab716822 | ci: bootstrap the stack from schema.sql only; restore historical migrations | Amal | 2026-08-05 | ↗ GitHub |
commit body Addresses willchen96's review on PR #256.
WHY THIS MATTERS
The previous revision of this workflow built the CI database as:
empty database -> current schema.sql -> every dated migration
and called it a "schema-drift smoke test". But that sequence is not a real
installation path. Per README.md, this repo has two deliberately separate
database artifacts:
- backend/schema.sql -> the COMPLETE shape for FRESH databases
- backend/migrations/ -> incremental steps that move OLDER, already-
deployed databases forward from the version
they are on
A fresh install runs schema.sql and stops. An existing deployment runs only
the migrations dated after its version. Nobody ever replays the full
migration history on top of the current snapshot - so when five old
migrations "failed" under that replay, they were not broken; the harness
was. They had run correctly on the era-appropriate schemas they were
written for.
WHAT IS MIGRATION IMMUTABILITY
Once a migration has shipped and real deployments have executed it, the
file becomes a historical record of "the change that was required at that
point in time". Editing it afterwards cannot help any database that already
ran it - it only makes the repo's history diverge from what production
actually executed, which is exactly the record you need intact when
debugging a deployment later. (Tools like Flyway enforce this with
checksums: a modified applied migration is a hard error.) The previous
revision rewrote five historical migrations to satisfy the artificial
replay; worse, the column-existence guards it added made those backfills
silently no-op on unexpected schemas - converting the loud failure a drift
check exists to produce into a silent skip.
HOW THIS COMMIT FIXES IT
1. The five historical migrations are restored byte-for-byte to their
state on main (20260424_01, 20260427_01, 20260602_01, 20260613_02,
20260613_05).
2. The workflow's bootstrap step now loads schema.sql only - the same
thing backend/scripts/test-stack.sh does locally and the same thing the
README documents for a fresh deployment. The suite therefore tests the
real contract: "a fresh Mike database enforces deny-all RLS and the
auth<->API contract."
Real drift protection (does baseline-plus-migrations equal the current
snapshot?) needs a pinned baseline dump from an older release, the
migrations dated after it, and a pg_dump schema diff against a
schema.sql-built database. That is a separate change, designed separately.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
4e136af6 | ci: prove fresh installs and upgraded deployments converge on one schema | Amal | 2026-08-05 | ↗ GitHub |
commit body The follow-up promised in PR #256's review thread: a real drift check,
designed the way willchen96 described - a pinned baseline from an older
point in history, only the migrations added since, and a schema comparison
against a fresh install. It also fixes the first drift it caught.
WHY THIS MATTERS
This repo maintains the database shape twice, on purpose: schema.sql is
what a FRESH install runs; backend/migrations/ is what an EXISTING
deployment applies to move forward. They are edited by hand, in parallel,
and nothing forces them to agree. When they disagree, the two classes of
real deployment silently diverge - and no ordinary test notices, because
test databases are always built fresh.
This is not hypothetical. Commit b2dbb39 ("narrow service role schema
grants") tightened service_role from GRANT ALL to select/insert/update/
delete - in schema.sql only, with no migration. Every deployment created
before 2026-07-23 and upgraded by the book still lets service_role
TRUNCATE any table, create TRIGGERs, and reset sequences. Fresh installs
do not. Same codebase, two different security postures.
HOW THE CHECK WORKS
The new "Schema drift" workflow builds both REAL installation paths in one
disposable Supabase stack and demands they converge:
upgraded: schema.sql as of a pinned baseline commit (9a1277b, the
commit that introduced dated migrations) + only the
migrations git says were ADDED since (git diff
--diff-filter=A) - the documented upgrade path, exactly
fresh: today's schema.sql
Each build is reduced to a canonical fingerprint
(backend/scripts/schema-fingerprint.sql): tables, columns, constraints,
indexes, RLS policies, function definitions, triggers, views, enums, and
exploded per-privilege ACLs, every section totally ordered.
WHY A FINGERPRINT INSTEAD OF DIFFING pg_dump
Two reasons, both lessons from PR #256:
1. Column order. Migrations append columns; schema.sql may declare them
anywhere. Upgraded and fresh databases therefore differ in physical
column order forever - a benign difference a raw pg_dump diff would
flag on every table. The fingerprint sorts columns by name: benign
difference ignored, every real difference kept.
2. Grant order. An ACL array is ordered by GRANT execution order, which
legitimately differs between the two paths. Exploding to one row per
(object, grantee, privilege) and sorting compares the meaning, not the
history.
And unlike the replay-history approach reverted in PR #256, this never
runs an old migration against a schema from its future, and it can never
be "fixed" by editing shipped migrations - the failure message explicitly
forbids that.
THE ACCOMPANYING MIGRATION
20260805_01_narrow_service_role_grants.sql is the forward fix for the
b2dbb39 drift: it revokes service_role's excess table/sequence privileges
so upgraded deployments land on the same least-privilege grants a fresh
install gets. Note the difference from what PR #256 reverted: adding a NEW
dated migration to move deployments forward is exactly what migrations are
for; editing already-shipped ones is what they must never suffer.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
128464e1 | fix: cast "char" catalog columns to text in the schema fingerprint | Amal | 2026-08-05 | ↗ GitHub |
commit body First CI run of the drift check confirmed the whole upgrade path works -
baseline schema plus all twelve since-added migrations applied cleanly -
and then died inside the fingerprint itself:
ERROR: operator is not unique: text || "char"
WHAT IS "char" (WITH QUOTES)
Postgres catalog columns like pg_class.relkind, pg_attribute.attidentity,
and pg_default_acl.defaclobjtype use the internal single-byte type "char"
(quoted) - a different type from char(1). Concatenating text with "char"
is ambiguous: the parser finds more than one usable || operator via
implicit casts and refuses to guess. An explicit ::text cast picks one.
Only the "char" columns needed casts; booleans and reals concatenate fine
(anynonarray || text is unique for them).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
2f667486 | fix: fold workflow_open_source_submissions into schema.sql | Amal | 2026-08-05 | ↗ GitHub |
commit body The drift check's first complete run found exactly the class of bug it was
built for - and it is a real one affecting every fresh install today.
WHAT THE CHECK FOUND
Migration 20260629_01 created workflow_open_source_submissions, and commit
a5fe6d6 (2026-07-04) merged it - but the table was never folded into
backend/schema.sql (git log -S confirms it has never appeared there). The
two installation paths therefore diverge:
upgraded deployment: has the table (migration applied)
fresh install: does not - schema.sql never creates it
This is not dead weight: backend/src/routes/workflows.ts,
lib/userDataCleanup.ts, and lib/userDataExport.ts all query the table. On
a fresh install, submitting a workflow to the open-source queue, exporting
user data, or cleaning up a user hits "relation does not exist".
THE FIX
Add the table to schema.sql verbatim from the migration - same columns,
check constraints, the three indexes, RLS enabled - placed with the other
workflow tables, plus its row in the deny-all revoke block. The file's
closing "grant select, insert, update, delete on all tables" already
covers service_role, matching what the migration's era grants produce on
the upgraded path, so the ACL fingerprints converge too.
With this fold-in, the fingerprint diff between "baseline + migrations
since" and "current schema.sql" should be empty - the check's green state.
WHY THIS DIRECTION AND NOT A MIGRATION
The failure message offers two legitimate fixes: fold a missing change
into schema.sql, or ship a new dated migration. Here the migration already
exists and upgraded deployments are correct; it is the snapshot that is
missing the change. So the snapshot gets the fix - the mirror image of the
service_role grants drift fixed in the previous commit, where schema.sql
was right and deployments needed a new migration.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
317c5559 | fix: apply the service_role grant migration in one transaction | Amal | 2026-08-05 | ↗ GitHub |
commit body WHY THIS MATTERS
This migration REVOKEs every privilege service_role holds on the
application's tables and sequences, then GRANTs back the narrower set the
backend actually needs. On a live deployment those two steps were not
atomic: CI (and the documented upgrade path) apply migrations with
psql --set ON_ERROR_STOP=1 --file <migration>
and plain psql, without the -1/--single-transaction flag, runs each SQL
statement in its own autocommitted transaction. That means there was a
real moment - after `revoke all ... from service_role` committed and
before the following `grant select, insert, update, delete` committed -
when service_role had ZERO privileges on every table. Any backend query
racing through that window fails with "permission denied", i.e. a brief
production outage caused by a security-hardening migration.
WHAT IS AUTOCOMMIT VS. AN EXPLICIT TRANSACTION
PostgreSQL always runs statements inside transactions. If you do not open
one yourself, each statement gets its own ("autocommit"), and its effects
become visible to every other session the instant it completes:
revoke all privileges on all tables ... ; -- visible immediately!
-- <-- other sessions now see service_role with no privileges
grant select, insert, update, delete ... ; -- visible only now
Wrapping the statements in `begin; ... commit;` changes when other
sessions see the effects: nothing is visible until COMMIT, and then
everything is visible at once. DDL and privilege changes are fully
transactional in PostgreSQL (unlike some other databases), so this is a
supported and standard pattern:
begin;
revoke all privileges on all tables ... ;
grant select, insert, update, delete ... ;
commit;
Concurrent queries either see the old grants (before commit) or the final
narrowed grants (after commit) - never the empty in-between state.
HOW THE FIX WORKS
The migration file now opens with `begin;` and ends with `commit;`. The
revoke+regrant pairs for tables and for sequences all sit inside that one
transaction, so applying the file with plain psql is atomic. A bonus:
with ON_ERROR_STOP=1, a failure partway through now rolls the whole file
back instead of leaving service_role stripped of privileges with no
re-grant applied.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
66877127 | fix: use the sequence ACL default for sequences in the fingerprint | Amal | 2026-08-05 | ↗ GitHub |
commit body WHY THIS MATTERS
The schema-drift fingerprint explodes each relation's ACL into one line
per (object, grantee, privilege) so CI can diff a fresh install against
an upgraded deployment. When a relation has never had an explicit GRANT
or REVOKE, its pg_class.relacl is NULL - PostgreSQL stores nothing and
applies built-in defaults - so the script synthesizes the effective ACL
with acldefault(). The relation query admits sequences (relkind 'S'),
but it called acldefault('r', ...) - the TABLE default - for every row.
A sequence with a NULL relacl would therefore be fingerprinted as
holding INSERT, DELETE, TRUNCATE, REFERENCES and TRIGGER: privileges a
sequence cannot hold at all. Today no sequence in the schema has a NULL
relacl, so the check passes either way - but the first time one does,
the failure diff would show phantom table privileges on a sequence,
sending whoever debugs the drift down a false trail.
WHAT IS acldefault()
acldefault(objtype, ownerid) answers "what ACL does PostgreSQL treat a
NULL acl column as meaning for this kind of object?". The first argument
is a one-character object-type code, and each code maps to a different
default privilege set:
acldefault('r', owner) -- 'r'elation: SELECT, INSERT, UPDATE,
-- DELETE, TRUNCATE, REFERENCES, TRIGGER
acldefault('s', owner) -- 's'equence: USAGE, SELECT, UPDATE
acldefault('f', owner) -- 'f'unction: EXECUTE (incl. PUBLIC)
Note the code is about the object KIND, not pg_class.relkind: relkind
'S' (a sequence row in pg_class) corresponds to acldefault kind 's'.
Passing 'r' for a sequence does not error - it just fabricates a
table-shaped ACL the sequence could never actually have.
HOW THE FIX WORKS
The lateral aclexplode now picks the acldefault kind per row:
aclexplode(coalesce(c.relacl,
acldefault((case when c.relkind = 'S' then 's' else 'r' end)::"char",
c.relowner)))
Sequences get the sequence default (USAGE, SELECT, UPDATE); tables,
partitioned tables, views and materialized views - everything else the
WHERE clause admits - keep the relation default. The explicit ::"char"
cast matches acldefault's parameter type (the internal one-byte "char",
not char(1)). Verified against a live Postgres 15: a fresh sequence with
NULL relacl now fingerprints as exactly SELECT|UPDATE|USAGE, and the
full drift check (baseline schema + migrations vs. current schema.sql)
still reports identical fingerprints.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
2d442b15 | Merge branch 'main' into olp-pr/schema-drift-check | Will Chen | 2026-08-06 | ↗ GitHub |