Skip to content

fix(export): complete the portability promise — missing tables, SQL dump, real PKs (#8) - #74

Open
keithfawcett wants to merge 2 commits into
mainfrom
fix/export-portability
Open

fix(export): complete the portability promise — missing tables, SQL dump, real PKs (#8)#74
keithfawcett wants to merge 2 commits into
mainfrom
fix/export-portability

Conversation

@keithfawcett

Copy link
Copy Markdown
Contributor

Closes #8 from docs/audit-remaining-work.md. Not money-risky, but it's the one architectural promise the repo makes on its front page.

What wasn't true

CLAUDE.md principle #2 and the README promise every table exports to CSV + JSON + SQL, and a hosted export re-imports into self-host. Three gaps:

  1. The exported set omitted PartnerProgram, PartnerCommission and Coupon. A restored instance had the partner roster but no program grants, no record of what any partnership was agreed at, and no coupon → (partner, program) mapping — so coupon-driven conversions couldn't be re-attributed.
  2. No SQL dump existed, though the contract names it as one of the three formats.
  3. Import hardcoded id as the conflict target, but PartnerCommission is keyed on partnerId — adding it naively would have thrown on every re-import.

Each table now carries its real primary key in EXPORT_TABLES, and that list doubles as the FK-safe import order.

Two more the round-trip test caught (both live bugs)

  • A JS array coming out of select * is ambiguous: Program.commissionRule is a jsonb array, Program.categories is a text[]. The driver renders a bare array as a Postgres array literal, so importing any program with compound commission rules failed with invalid input syntax for type json. Both the import and the dump now read the live column types and render each correctly ('[{"type":"percent"}]' vs '{"saas","devtools"}').
  • Same ambiguity in the dump: a text[] column was being emitted with ::jsonb.

The SQL dump

GET /export.sql (full bundle) and GET /export/<Table>.sql. Portable by default — rows are written under a psql variable, so one file restores into any instance:

psql "$DATABASE_URL" -v tenant_id=default -f openpartner-export.sql

It opens a transaction, set_config('app.tenant_id', …) so it works on the RLS-scoped app role as well as the privileged one, emits tables in import order, and every statement is ON CONFLICT (pk) DO NOTHING — idempotent and resumable, same guarantee as the JSON path. ?tenantId=<id> bakes a literal instead, for clients that don't run psql meta-commands.

Versioning

schemaVersion 1 → 2. POST /import still accepts v1 bundles — an export sitting on someone's disk must not become worthless because we added tables. SUPPORTED_IMPORT_VERSIONS only ever grows.

Tests

export-roundtrip.test.ts (11): seed one row in every exportable table with the FK chain intact → export → wipe → restore → compare. Once through JSON, once through the SQL dump actually executed against Postgres, and once through the portable dump put through psql's own substitution. Plus idempotency on both paths (the case that used to throw on PartnerCommission), v1 acceptance, PK/order invariants, and escaping of apostrophes, jsonb arrays, text[] and nulls.

Full suite: 274 passing. pnpm typecheck + pnpm lint clean. No migration.

Also

The portal export page listed 9 of the 14 tables and offered no SQL — now complete, with a SQL column and a SQL bundle button. docs/data-portability.md is the format contract: bundle shape, the table list with its keys, both restore procedures, the type-fidelity note, and the checklist for adding a table.

🤖 Generated with Claude Code

…ump, real PKs (#8)

CLAUDE.md and the README promise every table exports to CSV + JSON + SQL
with a re-importable round-trip. Three ways it wasn't true:

1. The exported set omitted PartnerProgram, PartnerCommission and Coupon.
   A restored instance had the partner roster but no program grants, no
   record of what any partnership was agreed at, and no coupon →
   (partner, program) mapping — so coupon-driven conversions could not be
   re-attributed. Added, in FK-safe positions.
2. No SQL dump existed at all, though the contract names it.
3. Import hardcoded `id` as the conflict target, but PartnerCommission is
   keyed on partnerId — adding it naively would have thrown on every
   re-import.

Each table now carries its real primary key in EXPORT_TABLES, and the
list doubles as the import order (parents first).

The round-trip test found two more, both live bugs:

- A JS array coming out of `select *` is ambiguous: Program.commissionRule
  is a jsonb array, Program.categories is a text[]. The driver renders a
  bare array as a Postgres array literal, so importing ANY program with
  compound commission rules failed with "invalid input syntax for type
  json". Both paths now read the live column types and render each
  correctly.
- Same ambiguity in the SQL dump: a text[] column got `::jsonb`.

New: GET /export.sql (full bundle) and GET /export/<Table>.sql. The dump
is portable by default — rows are written under a psql variable, so one
file restores anywhere:

    psql "$DATABASE_URL" -v tenant_id=default -f openpartner-export.sql

It opens a transaction, sets app.tenant_id so it works on the RLS-scoped
app role as well as the privileged one, and every statement is ON CONFLICT
DO NOTHING so a restore is idempotent and resumable. `?tenantId=<id>`
bakes a literal instead, for clients that don't run psql meta-commands.

schemaVersion 1 → 2. POST /import still accepts v1 bundles: an export
sitting on someone's disk must not become worthless because we added
tables. SUPPORTED_IMPORT_VERSIONS only ever grows.

Tests (export-roundtrip.test.ts, 11): seed one row in every exportable
table → export → wipe → restore → compare, once through JSON and once
through the SQL dump actually executed against Postgres (including the
portable form put through psql's own substitution). Plus idempotency on
both paths, v1 acceptance, PK/order invariants, and escaping of quotes,
jsonb arrays, text[] and nulls.

Also: the portal export page listed 9 of the 14 tables and offered no
SQL; docs/data-portability.md is the format contract (bundle shape, table
list with keys, restore procedures, and how to add a table).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
keithfawcett added a commit that referenced this pull request Aug 9, 2026
Turns the handoff brief into a status record: what each item was, what
actually shipped, and — the part that is still open — the staging
exercises that have to pass before either money path is trusted.

Item A (#10, PR #73): planner/executor split with a durable payout intent
and a frozen commission set. Item B (#12, PR #75): the three funding
races plus a live-Stripe backstop for missed refund/reversal webhooks.
Item C (#8, PR #74): the three missing tables, per-table primary keys, a
portable SQL dump, and two array round-trip bugs the test found.

No code left on any of the three; the remaining work is the staging
checklists in docs/direct-connect-payouts.md and section H of the funding
staging runbook, plus the two post-merge prod actions for #62 and #63.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… id, string mode

Codex review of #74 found three real defects. All three are in the part
of the change that generates a file someone else later executes, which is
exactly where I should have been most careful.

1. COMMAND INJECTION via `?tenantId=`. The parameter was accepted
   verbatim and interpolated into a `--` header comment. psql
   meta-commands are line-oriented, so a value containing a newline and
   `\! <shell command>` produced a dump that runs that command on the
   machine doing the restore — the documented workflow. The values were
   escaped; the comment was not.
   The id is now VALIDATED, not escaped (`[A-Za-z0-9_-]{1,64}`, refused
   with a 400), header comments strip newlines regardless, and
   buildSqlDump asserts the same rule so no future caller can reintroduce
   it. I wrote exactly this guard for Stripe ids on another branch and
   missed it here.

2. THE DOCUMENTED RESTORE COULD NEVER HAVE WORKED. The fallback was
   `\set tenant_id 'default'` and the docs said `-v tenant_id=default` —
   but `default` is the tenant SLUG. `tenantId` is a foreign key to
   `Tenant.id`, whose seeded value is `01J0000000DEFAULTTENANT0000`, so
   the first row failed the FK and rolled the whole restore back. The
   fallback is now DEFAULT_TENANT_ID, and the docs/README/portal say
   "destination tenant id", not a slug.
   The old test hid this by substituting the real id before executing;
   there is now a test that runs the dump the way `psql -f` with NO -v
   does, using the file's own fallback.

3. The escaping assumed `standard_conforming_strings` is on (the default
   since PG 9.1) but never said so; with it off, a backslash in exported
   data escapes the quote doubling and breaks out of the literal. The
   dump now pins the setting in its preamble.

Also from the same review: normalizeRow revived ANY ISO-shaped string as
a Date, so a partner named "2026-08-09T00:00:00" was corrupted on import
and a lookalike that isn't a valid date could fail it. Now that the
importer reads column types, the coercion is gated on the column actually
being a timestamp.

Tests: no -v restore path, string-mode pin, tenant-id refusal (incl. the
`\!` payload), header-comment containment, backslash/quote round-trip
through the dump, and timestamp-lookalike text.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@keithfawcett

Copy link
Copy Markdown
Contributor Author

Adversarial review pass (Codex, xhigh) — three real defects fixed in 2f4df82

All three were in the generated-artifact path, i.e. a file someone else executes:

  1. Command injection via ?tenantId= (critical). The parameter was interpolated verbatim into a -- header comment. psql meta-commands are line-oriented, so ?tenantId=x%0A%5C!%20cmd%0A-- produced a dump containing a line-leading \! cmd, which psql runs as a shell command on the machine doing the restore. The row values were escaped; the comment wasn't. Now validated ([A-Za-z0-9_-]{1,64}, 400 on anything else), comments strip newlines regardless, and buildSqlDump asserts the same rule so no future caller can reintroduce it.

  2. The documented restore could never have worked (high). The fallback was \set tenant_id 'default' and the docs said -v tenant_id=default — but default is the slug; tenantId is an FK to Tenant.id = 01J0000000DEFAULTTENANT0000. The first row failed the FK and rolled the restore back. Fixed everywhere (dump fallback, docs, README, portal). My original test masked it by substituting the real id before executing; there's now a test that runs the dump the way psql -f with no -v does.

  3. standard_conforming_strings assumption (medium). The escaping assumed it's on (default since 9.1) without saying so; with it off a backslash in exported data breaks out of its literal. The dump now pins it in the preamble.

Plus: normalizeRow revived any ISO-shaped string as a Date, so a partner named 2026-08-09T00:00:00 was corrupted on import. Now that the importer reads column types, the coercion is gated on the column actually being a timestamp.

New tests: no--v restore path · string-mode pin · tenant-id refusal (including the \! payload) · header-comment containment · backslash/quote round-trip through the dump · timestamp-lookalike text.

⚠️ Merge order: this PR must land after #62

Exports rely entirely on RLS for tenant scoping (exportTable is an unfiltered select *). PartnerProgram — newly added to the export set here — has no RLS policy and no app-role grant on main: the rename migration never created them under the new name and packages/db/scripts/ensure-app-role.ts:33 still lists PartnerCampaign. Merging this first would make /export.json either 500 (no grant) or return every tenant's grants (grant, no policy). #62 adds exactly that policy + grant.

Known, not fixed here (follow-ups)

  • ON CONFLICT (pk) doesn't cover natural keys — Link(tenantId,linkKey), Identity(clickId,userId), Event.externalEventId — so importing into a non-empty database throws instead of skipping. The round-trip test always wipes first.
  • Config, PartnerPostback and BrandAsset are customer-owned data and still aren't exported, against a doc that says "every table".
  • exportAll + buildSqlDump build the whole tenant in memory; a multi-million-row tenant needs streaming.
  • Sub-millisecond timestamptz precision is lost through both paths (pg returns JS Date).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant