Skip to content

Commit 53689ae

Browse files
fix(security): apply the transactions INSERT lockdown to cloud and make drift detectable (#541)
The #538 lockdown (20260725180000) was never applied to the cloud project. Until this commit, `authenticated` still held INSERT on `public.transactions` there and the INSERT policy was the older #528 shape, so a caller could open a pending row quoting `settlement_base: 1` against a priced product, pay that on-chain, and have /api/payments/solana/verify — which verifies against the row's own settlement_base — flip it to successful and grant the entitlement. Applied to cloud (verified by querying the live catalog, not by reading files): - REVOKE INSERT ON transactions FROM authenticated, anon INSERT is now held only by postgres and service_role. - INSERT policy re-created with all four settlement_* IS NULL pins. Nothing depended on the grant: PR #539 had already moved both user-scoped inserts onto createAdminClient(). The two user-scoped .update() calls that remain in the checkout route only touch provider_subscription_id and status, both inside the #528 three-column UPDATE grant, so they are unaffected. WHY IT WAS LOST, AND WHAT NOW CATCHES IT Cloud migration stamps had drifted from repo filenames: four migrations were applied through the MCP apply_migration tool, which stamps a fresh timestamp instead of the filename. That makes "what is missing from cloud?" unanswerable — the re-stamped four read as pending, and the genuinely missing fifth was indistinguishable from them. npm run verify:cloud queries the live database and exits non-zero on drift. It asserts ledger integrity (every repo migration stamped under its own filename; no stamp matching no file) and the #512/#528/#538 payment invariants against catalog state rather than migration text, so a later re-widening fails it too. The rules are split into scripts/lib/verify-cloud-schema-checks.ts and driven from both healthy and drifted fixtures in tests/unit/verify-cloud-schema.test.ts (13 tests). The healthy fixture is the exact state read back from cloud, so the policy-pin matcher is tested against how Postgres actually renders the expression. That is also how the "fails if you re-grant INSERT" criterion is demonstrated: as a repeatable test, rather than by briefly re-opening a write grant on the payments table of the only production database this project has. docs/MIGRATIONS.md now states the push-only rule and why, with the incident table, and reframes the Management API fallback so the schema_migrations stamp reads as the thing that makes it safe rather than optional bookkeeping. Gates: typecheck clean, test:unit 383 passed (370 baseline + 13), eslint clean on new files, build succeeds. Refs #540 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPFyHjqzSR6Ku1gyWjeQie
1 parent 146a8cf commit 53689ae

5 files changed

Lines changed: 674 additions & 8 deletions

File tree

docs/MIGRATIONS.md

Lines changed: 101 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,79 @@ Edit the generated file with your SQL. Always make migrations **idempotent** whe
1818

1919
## Applying Migrations
2020

21+
### The rule: cloud changes go through `supabase db push`
22+
23+
**Never apply a migration to cloud with the MCP `apply_migration` tool, the SQL
24+
editor, or a raw Management API call unless you also stamp it with the version
25+
from its filename.** This is not style preference — it is what keeps drift
26+
detectable.
27+
28+
`supabase_migrations.schema_migrations` is the only record of what cloud has
29+
run. `supabase db push` stamps each migration with **the timestamp from its
30+
filename**. Every ad-hoc path stamps a **fresh** timestamp instead. Once the
31+
ledger holds a stamp that matches no file in `supabase/migrations/`, the
32+
question "what is missing from cloud?" stops having a usable answer: the
33+
re-stamped migrations show up as *pending* (false positives), and a genuinely
34+
missing migration is indistinguishable from them.
35+
36+
That is not hypothetical. It is how issue #541 happened:
37+
38+
| Repo file | Cloud stamp before #541 |
39+
|---|---|
40+
| `20260721120000_add_binance_personal_provider` | `20260725213441` |
41+
| `20260725110000_transaction_split_snapshot_backstop` | `20260725213508` |
42+
| `20260725160000_entitlement_gated_enrollment_inserts` | `20260725213610` |
43+
| `20260725170000_transactions_column_hardening` | `20260725192946` |
44+
| `20260725180000_transactions_insert_lockdown` | **never applied** |
45+
46+
Four migrations were applied through `apply_migration` and re-stamped. The
47+
fifth — the `transactions` INSERT lockdown from #538, a `severity:critical`
48+
payments fix — was never applied at all, and hid among the four false
49+
positives for a day. Diffing the repo against the ledger reported all five as
50+
pending, so the real gap looked like more of the same noise.
51+
52+
If you do have to apply SQL out of band, **immediately** repair the stamp:
53+
54+
```bash
55+
# Preferred — the CLI's purpose-built command for this
56+
supabase migration repair --status applied <VERSION_FROM_FILENAME>
57+
supabase migration repair --status reverted <WRONG_FRESH_STAMP>
58+
```
59+
60+
Then run `npm run verify:cloud` (below) and confirm it is clean before moving on.
61+
62+
### Verifying cloud matches this repo
63+
64+
```bash
65+
SUPABASE_ACCESS_TOKEN=sbp_... npm run verify:cloud
66+
```
67+
68+
Queries the **live** database and exits non-zero on drift. It asserts both
69+
halves of the problem above:
70+
71+
- **Ledger integrity** — every repo migration is stamped on cloud under its own
72+
filename, and cloud carries no stamp that matches no file.
73+
- **Payment invariants** (#512 / #528 / #538) — `authenticated` and `anon` hold
74+
no INSERT on `transactions`, no column-level INSERT grant survives,
75+
`service_role` still has INSERT, the `authenticated` UPDATE grant is exactly
76+
`(status, provider_subscription_id, stripe_payment_intent_id)`, the INSERT
77+
policy pins `status` plus all four `settlement_*` columns to NULL, and both
78+
`before_transaction_split_snapshot_*` triggers exist and are enabled.
79+
80+
It reads catalog state, not migration text, so it also catches a *later*
81+
migration re-widening something — including a schema dump re-applying the
82+
original `GRANT ALL ON TABLE transactions TO authenticated`.
83+
84+
The token is a [personal access token](https://supabase.com/dashboard/account/tokens).
85+
It uses the Management API over HTTPS rather than a Postgres connection because
86+
port 5432 is blocked on some networks this project is developed from — the same
87+
reason Option 2 below exists.
88+
89+
> Unit tests cannot do this job. `tests/unit/verify-cloud-schema.test.ts` and
90+
> `tests/unit/transaction-split-snapshot-backstop.test.ts` prove the *rules* are
91+
> right; neither opens a database connection. A green `npm run test:unit` says
92+
> nothing about whether a migration was ever applied. Only `verify:cloud` does.
93+
2194
### Option 1: Supabase CLI (preferred)
2295

2396
```bash
@@ -30,7 +103,10 @@ This connects via the Supabase connection pooler and applies all pending migrati
30103

31104
### Option 2: Management API (fallback)
32105

33-
When the CLI can't connect, push migrations via the Supabase Management API:
106+
When the CLI can't connect, push migrations via the Supabase Management API.
107+
**This is the path that creates drift** — the `INSERT INTO schema_migrations`
108+
line below is not optional bookkeeping, it is the entire reason this fallback is
109+
safe to use:
34110

35111
```bash
36112
# 1. Get your access token (stored in macOS keychain by the CLI)
@@ -91,16 +167,22 @@ process.stdout.write(JSON.stringify({query: 'SELECT version, name FROM supabase_
91167

92168
### What's pending locally?
93169

94-
Compare local files against remote records:
170+
```bash
171+
SUPABASE_ACCESS_TOKEN=sbp_... npm run verify:cloud
172+
```
173+
174+
Use this rather than eyeballing a diff. The manual comparison —
95175

96176
```bash
97-
# Local migration versions
98177
ls supabase/migrations/*.sql | sed 's|.*/||' | sed 's/_.*//'
99-
100-
# Remote versions (use one of the methods above)
101178
```
102179

103-
Any local version not in the remote list needs to be applied.
180+
— is only trustworthy when the ledger is clean. If any migration was ever
181+
applied out of band, a local version missing from the remote list means *either*
182+
"never applied" *or* "applied under a different stamp", and nothing in the diff
183+
tells you which. `verify:cloud` reports the two cases separately (`not on cloud`
184+
vs `orphan stamps`) and additionally checks that the payment invariants actually
185+
hold, which no filename comparison can do.
104186

105187
## Running the Production Seed
106188

@@ -158,15 +240,26 @@ DELETE FROM courses WHERE author_id NOT IN (SELECT id FROM profiles);
158240

159241
### Migration version mismatch
160242

161-
If the remote `schema_migrations` table has different version numbers than local filenames (e.g. migrations were applied via dashboard with auto-generated timestamps):
243+
If the remote `schema_migrations` table has different version numbers than local filenames (e.g. migrations were applied via dashboard or `apply_migration` with auto-generated timestamps), prefer the CLI's purpose-built command:
244+
245+
```bash
246+
supabase migration repair --status applied <LOCAL_VERSION>
247+
supabase migration repair --status reverted <REMOTE_VERSION>
248+
```
249+
250+
If the CLI cannot reach the pooler, the equivalent write is:
162251

163252
```sql
164-
-- Update the remote record to match the local filename
253+
-- Re-stamp the remote record to match the local filename.
254+
-- Verify FIRST that the DDL really is live (query the grants/policies/triggers
255+
-- it created) — this only repairs the ledger, it does not apply anything.
165256
UPDATE supabase_migrations.schema_migrations
166257
SET version = '<LOCAL_VERSION>'
167258
WHERE version = '<REMOTE_VERSION>' AND name = '<NAME>';
168259
```
169260

261+
Confirm with `npm run verify:cloud` afterwards.
262+
170263
### Prepared statement errors (port 6543)
171264

172265
If you try the session pooler (port 6543) and get `prepared statement already exists`, use port 5432 or the Management API instead. The session pooler doesn't work well with the Supabase CLI's connection handling.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
"db:reset": "supabase db reset",
1818
"db:push": "supabase db push",
1919
"db:types": "supabase gen types typescript --linked --schema public > lib/database.types.ts",
20+
"verify:cloud": "tsx scripts/verify-cloud-schema.ts",
2021
"mcp:build": "cd mcp-server && npm run build",
2122
"backup-course": "tsx scripts/backup-english-course.ts",
2223
"restore-course": "tsx scripts/restore-english-course.ts",
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
/**
2+
* Issue #541 — the assertion half of `npm run verify:cloud`, kept pure.
3+
*
4+
* Split out from `scripts/verify-cloud-schema.ts` so the rules can be exercised
5+
* without a network round-trip or a live database. That matters here for a
6+
* specific reason: the acceptance criterion for #541 is "the check FAILS if you
7+
* re-grant INSERT", and the honest way to demonstrate that is a test that feeds
8+
* in a re-granted fixture — not briefly re-opening a write grant on the
9+
* payments table of the only production database this project has.
10+
*
11+
* `tests/unit/verify-cloud-schema.test.ts` drives every rule below from both a
12+
* healthy fixture and a drifted one.
13+
*/
14+
15+
export type MigrationLedger = {
16+
/** Version prefixes of `supabase/migrations/*.sql`, e.g. `20260725180000`. */
17+
repoVersions: string[]
18+
/** `version` column of `supabase_migrations.schema_migrations` on cloud. */
19+
cloudVersions: string[]
20+
}
21+
22+
export type TransactionPrivileges = {
23+
auth_insert: boolean
24+
anon_insert: boolean
25+
service_insert: boolean
26+
/** Comma-joined column list, or null when no column-level grant exists. */
27+
auth_col_insert: string | null
28+
auth_col_update: string | null
29+
}
30+
31+
export type InsertPolicy = { policyname: string; with_check: string | null }
32+
export type TriggerState = { tgname: string; enabled: boolean }
33+
34+
export type CloudFacts = {
35+
ledger: MigrationLedger
36+
priv: TransactionPrivileges
37+
insertPolicies: InsertPolicy[]
38+
triggers: TriggerState[]
39+
}
40+
41+
export type Check = { name: string; ok: boolean; detail: string }
42+
43+
/** The exact `authenticated` UPDATE column grant #528 established. */
44+
export const EXPECTED_UPDATE_COLS =
45+
'provider_subscription_id, status, stripe_payment_intent_id'
46+
47+
/** What the #538 INSERT policy must pin. Rendered form, after normalisation. */
48+
export const REQUIRED_POLICY_PINS = [
49+
"status = 'pending'",
50+
'settlement_currency IS NULL',
51+
'settlement_base IS NULL',
52+
'settlement_mint IS NULL',
53+
'settlement_sol_usd IS NULL',
54+
]
55+
56+
export const SPLIT_SNAPSHOT_TRIGGERS = [
57+
'before_transaction_split_snapshot_insert',
58+
'before_transaction_split_snapshot_update',
59+
]
60+
61+
/**
62+
* Strip the catalog's type annotations and collapse whitespace, so a pin written
63+
* `status = 'pending'` matches how Postgres renders it back
64+
* (`status = 'pending'::transaction_status`).
65+
*/
66+
export function normalisePolicyExpr(expr: string): string {
67+
return expr.replace(/::[a-z_]+/g, '').replace(/\s+/g, ' ')
68+
}
69+
70+
export function evaluateChecks(facts: CloudFacts): Check[] {
71+
const checks: Check[] = []
72+
const add = (name: string, ok: boolean, detail: string) =>
73+
checks.push({ name, ok, detail })
74+
75+
// -------------------------------------------------------------------------
76+
// A. Ledger integrity — what makes any future drift detectable at all.
77+
// -------------------------------------------------------------------------
78+
const cloudSet = new Set(facts.ledger.cloudVersions)
79+
const repoSet = new Set(facts.ledger.repoVersions)
80+
const pending = facts.ledger.repoVersions.filter((v) => !cloudSet.has(v))
81+
const orphans = facts.ledger.cloudVersions.filter((v) => !repoSet.has(v))
82+
83+
add(
84+
'every repo migration is applied to cloud',
85+
pending.length === 0,
86+
pending.length
87+
? `not on cloud: ${pending.join(', ')}`
88+
: `all ${facts.ledger.repoVersions.length} applied`
89+
)
90+
91+
// An orphan stamp is the failure mode that hid #538: it matches no file, so it
92+
// pads the ledger and makes the pending list above untrustworthy.
93+
add(
94+
'cloud carries no migration stamp that matches no repo file',
95+
orphans.length === 0,
96+
orphans.length
97+
? `orphan stamps (applied ad hoc, not via db push): ${orphans.join(', ')}`
98+
: 'ledger matches repo filenames exactly'
99+
)
100+
101+
// -------------------------------------------------------------------------
102+
// B. The #528 / #538 payment invariants, against live catalog state.
103+
// -------------------------------------------------------------------------
104+
add(
105+
'authenticated holds no INSERT on transactions (#538)',
106+
facts.priv.auth_insert === false,
107+
facts.priv.auth_insert
108+
? 'GRANT IS PRESENT — a caller can open a self-priced row'
109+
: 'revoked'
110+
)
111+
112+
add(
113+
'anon holds no INSERT on transactions (#538)',
114+
facts.priv.anon_insert === false,
115+
facts.priv.anon_insert ? 'GRANT IS PRESENT' : 'revoked'
116+
)
117+
118+
// A table-level revoke leaves no column grants, but a later migration could
119+
// add one and reopen the path without restoring the table grant.
120+
add(
121+
'no column-level INSERT grant to authenticated survives',
122+
facts.priv.auth_col_insert === null,
123+
facts.priv.auth_col_insert ? `columns: ${facts.priv.auth_col_insert}` : '(none)'
124+
)
125+
126+
// The lockdown must not have taken the server-side writers with it.
127+
add(
128+
'service_role retains INSERT (webhooks, reconcilers, crons, admin routes)',
129+
facts.priv.service_insert === true,
130+
facts.priv.service_insert
131+
? 'granted'
132+
: 'REVOKED — every payment write path is broken'
133+
)
134+
135+
add(
136+
`authenticated UPDATE grant is exactly (${EXPECTED_UPDATE_COLS}) (#528)`,
137+
facts.priv.auth_col_update === EXPECTED_UPDATE_COLS,
138+
`actual: ${facts.priv.auth_col_update ?? '(none)'}`
139+
)
140+
141+
// The policy is unreachable while the grant is revoked. It is asserted anyway
142+
// because the revoke is one `GRANT INSERT` away from being undone, and these
143+
// pins are what keep the #538 under-quote impossible if that ever happens.
144+
if (facts.insertPolicies.length === 0) {
145+
add(
146+
'transactions INSERT policy pins status + all four settlement columns (#538)',
147+
false,
148+
'no INSERT policy found on public.transactions'
149+
)
150+
} else {
151+
for (const p of facts.insertPolicies) {
152+
const normalised = normalisePolicyExpr(p.with_check ?? '')
153+
const missing = REQUIRED_POLICY_PINS.filter((pin) => !normalised.includes(pin))
154+
add(
155+
`INSERT policy "${p.policyname}" pins status + all four settlement columns (#538)`,
156+
missing.length === 0,
157+
missing.length ? `missing pins: ${missing.join('; ')}` : 'all pins present'
158+
)
159+
}
160+
}
161+
162+
for (const name of SPLIT_SNAPSHOT_TRIGGERS) {
163+
const row = facts.triggers.find((t) => t.tgname === name)
164+
add(
165+
`trigger ${name} exists and is enabled (#512)`,
166+
row?.enabled === true,
167+
!row ? 'MISSING' : row.enabled ? 'enabled' : 'DISABLED'
168+
)
169+
}
170+
171+
return checks
172+
}

0 commit comments

Comments
 (0)