Skip to content

fix(security): apply the transactions INSERT lockdown to cloud and make migration drift detectable (#541) - #552

Merged
guillermoscript merged 3 commits into
masterfrom
fix/cloud-lockdown-541
Jul 26, 2026
Merged

fix(security): apply the transactions INSERT lockdown to cloud and make migration drift detectable (#541)#552
guillermoscript merged 3 commits into
masterfrom
fix/cloud-lockdown-541

Conversation

@guillermoscript

@guillermoscript guillermoscript commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Closes #541. Part of EPIC #540, §1.1.

What was wrong

20260725180000_transactions_insert_lockdown.sql (#538) was never applied to cloud. Verified by querying the live catalog before touching anything:

has_table_privilege('authenticated','public.transactions','INSERT')  ->  true
INSERT policy with_check  ->  the #528 shape, no settlement_* pins

So the #538 scenario was open on the only production database this project has: open a pending row quoting settlement_base: 1 against a priced product, pay 1 lamport, and /api/payments/solana/verify — which verifies the transfer against the row's own settlement_base (lib/payments/solana-reconcile.ts:104) — flips it to successful on the service-role client and grants the entitlement.

What this does

1. Applied the lockdown to cloud. REVOKE INSERT ... FROM authenticated, anon plus the INSERT policy re-created with all four settlement_* IS NULL pins. Confirmed by query afterwards:

assertion before after
authenticated INSERT true false
anon INSERT false false
service_role INSERT true true (untouched — every webhook/reconciler/cron depends on it)
INSERT policy settlement pins none all four present

Nothing depended on the grant — PR #539 had already moved both user-scoped inserts onto createAdminClient(). The two user-scoped .update() calls remaining in app/api/payments/checkout/route.ts only touch provider_subscription_id and status, both inside the #528 three-column UPDATE grant, so they still work.

2. Repaired the drifted ledger (20260726005843_repair_migration_ledger_541.sql). Cloud now matches the repo exactly: 171 files, 171 stamps, zero pending, zero orphans.

3. npm run verify:cloud — a drift check that queries the live database and exits non-zero on drift.

4. docs/MIGRATIONS.md now leads with the push-only rule, the reason, and the incident table.

Root cause

Cloud stamps had drifted from repo filenames — four migrations applied via MCP apply_migration, which stamps a fresh timestamp:

Repo file Cloud stamp before now
20260721120000_add_binance_personal_provider 20260725213441
20260725110000_transaction_split_snapshot_backstop 20260725213508
20260725160000_entitlement_gated_enrollment_inserts 20260725213610
20260725170000_transactions_column_hardening 20260725192946
20260725180000_transactions_insert_lockdown never applied

All five read as "pending" against the ledger, so the genuinely missing one hid among four false positives. Note the issue named three drifted stamps; there were fouradd_binance_personal_provider had drifted too. Each of the four was confirmed genuinely applied by querying what its DDL created (the binance_personal CHECK value, has_course_access in the enrollments INSERT policy, both snapshot triggers, the 3-column UPDATE grant) before being treated as a ledger-only repair, so the repair re-runs no DDL.

The repair migration is committed under the stamp apply_migration assigned it, so it is not itself an orphan. It is idempotent — on a fresh supabase db reset the four UPDATEs match nothing and the INSERT no-ops.

npm run verify:cloud

SUPABASE_ACCESS_TOKEN=sbp_... npm run verify:cloud

Against cloud right now — 10/10 PASS:

PASS  every repo migration is applied to cloud                          all 171 applied
PASS  cloud carries no migration stamp that matches no repo file        ledger matches repo filenames exactly
PASS  authenticated holds no INSERT on transactions (#538)              revoked
PASS  anon holds no INSERT on transactions (#538)                       revoked
PASS  no column-level INSERT grant to authenticated survives            (none)
PASS  service_role retains INSERT                                       granted
PASS  authenticated UPDATE grant is exactly (...)                       matches
PASS  INSERT policy pins status + all four settlement columns (#538)    all pins present
PASS  trigger before_transaction_split_snapshot_insert ... enabled      enabled
PASS  trigger before_transaction_split_snapshot_update ... enabled      enabled

It asserts ledger integrity and the #512/#528/#538 payment invariants against catalog state rather than migration text — so a later migration re-widening things, or a schema dump re-applying the original GRANT ALL ON TABLE transactions TO authenticated, fails it too.

It uses the Management API over HTTPS rather than a Postgres connection because the pooler on port 5432 is unreachable from this network (supabase migration listtls error … i/o timeout), the same reason docs/MIGRATIONS.md already documents an HTTPS fallback.

On the "prove it fails by re-granting" criterion

I did not re-grant INSERT on the live payments table. Re-opening a write grant on production to watch a script go red isn't a trade worth making.

Instead the failure modes are proven as repeatable tests in tests/unit/verify-cloud-schema.test.ts (13 tests), driven from a healthy fixture that is the exact catalog state read back from cloud — including how Postgres renders the policy expression back ('pending'::transaction_status), which is the part a hand-written fixture gets wrong. Covered: INSERT re-granted to authenticated, to anon, a column-level INSERT grant reintroduced, the pre-fix #528 policy shape, the policy dropped, the UPDATE grant widened, service_role losing INSERT, a trigger missing or disabled, a migration never applied, an orphan stamp, and the real pre-#541 ledger.

Known gap

supabase db push --dry-run (an acceptance criterion) is unverified — the CLI cannot reach the pooler from this network at all. The equivalent was verified by querying schema_migrations directly and diffing against supabase/migrations/: 171 vs 171, no pending, no orphans. Worth a confirming run from a network with port 5432 open.

QA

npm run typecheck            # clean
npm run test:unit            # 383 passed (370 baseline + 13 new)
npx eslint scripts/verify-cloud-schema.ts scripts/lib/ tests/unit/verify-cloud-schema.test.ts   # clean
npm run build                # succeeds
SUPABASE_ACCESS_TOKEN=sbp_... npm run verify:cloud   # 10/10 PASS

🤖 Generated with Claude Code

https://claude.ai/code/session_01HPFyHjqzSR6Ku1gyWjeQie

guillermoscript and others added 2 commits July 26, 2026 01:43
…ke 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
Applied to cloud. Re-stamps the four migrations that had been applied through
the MCP apply_migration tool (which stamps a fresh timestamp instead of the
migration's filename) and records 20260725180000, whose DDL was applied earlier
in this issue:

  20260721120000_add_binance_personal_provider        was 20260725213441
  20260725110000_transaction_split_snapshot_backstop  was 20260725213508
  20260725160000_entitlement_gated_enrollment_inserts was 20260725213610
  20260725170000_transactions_column_hardening        was 20260725192946
  20260725180000_transactions_insert_lockdown         was absent

Each of the four was confirmed genuinely live by querying what its DDL created
before being treated as a ledger-only repair, so this re-runs no DDL.

The file is committed under the stamp apply_migration assigned it
(20260726005843) so the repair is not itself an orphan — the ledger now matches
the repo exactly: 171 files, 171 stamps, zero pending, zero orphans.

Idempotent: on a fresh `supabase db reset` the four UPDATEs match nothing and
the INSERT no-ops, since the CLI has already stamped 20260725180000 by the time
this file runs.

verify:cloud now reports 10/10 PASS.

Gates: typecheck clean, test:unit 383 passed.

Refs #540

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HPFyHjqzSR6Ku1gyWjeQie
@guillermoscript
guillermoscript marked this pull request as ready for review July 26, 2026 01:13
…elative (#541)

The two ledger checks compare cloud against the migrations in the CURRENT
checkout, so on a feature branch a migration another in-flight branch has
already applied reads as an orphan, and one on this branch not yet applied
reads as pending. Neither is drift.

Found while confirming #541: PRs #553 (#542) and #554 (#543) had applied
20260726013256, 20260726015858, 20260726100000 and 20260726110000 to cloud,
none of which exist on this branch — so verify:cloud run here reports four
orphans that are not drift at all.

Left as guidance rather than logic. Filtering by "is this stamp on some other
branch" would need a remote ref walk and would silently excuse the exact
condition the check exists to catch. The orphan detail line now names the
possibility and says to re-check on master; the payment-invariant checks are
branch-independent and meaningful anywhere.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HPFyHjqzSR6Ku1gyWjeQie
@guillermoscript

Copy link
Copy Markdown
Owner Author

Correction to the "10/10 PASS" figure above

That run was accurate when taken, but is now stale, and the reason is worth recording.

Since then PRs #553 (#542) and #554 (#543) applied their own migrations to cloud — 20260726013256, 20260726015858, 20260726100000, 20260726110000. None of those files exist on this branch, so verify:cloud run from this branch now reports them as orphan stamps: 9/10 PASS, 1 FAIL.

That is not drift. The two ledger checks compare cloud against the migrations in the current checkout, which makes them branch-relative:

  • a migration another in-flight branch has already applied → reads as an orphan
  • a migration on your branch not yet applied → reads as pending

Fixed as guidance rather than logic (a5a921b): the orphan detail line now names the possibility and says to re-check on master, and docs/MIGRATIONS.md says to run it from master after merging. Filtering by "is this stamp on some other branch" would need a remote ref walk and would silently excuse the exact condition the check exists to catch — so it stays loud.

The eight payment-invariant checks are branch-independent and pass everywhere. The security outcome this PR is actually about is unaffected:

authenticated INSERT on transactions  ->  revoked (postgres + service_role only)
INSERT policy                          ->  all four settlement_* IS NULL pins present

Once #553 and #554 merge, master will hold every file and the ledger reads clean again.

@guillermoscript
guillermoscript merged commit 2e4678e into master Jul 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant