Add read-only migration-readiness verifier (PR 13 slice) - #2056
Add read-only migration-readiness verifier (PR 13 slice)#2056chobble-opencode-vm[bot] wants to merge 7 commits into
Conversation
Operators can run `deno task migration-verify` against the configured database (a live one, or one freshly restored from an old backup into the current application) to prove the legacy payment tables, attendee PII, and merge references are safe to migrate before a later release changes payment history — without writing anything. The pure rules in `src/shared/migration-readiness/readiness.ts` group one provider payment before pagination, convert old timestamps, and surface bounded contradictions across processed_payments, checkout_stages, sumup_checkouts, attendee PII, and attendee-merge references. The owner-key control for encrypted attendee PII is enforced: the verifier derives the site private key from an owner-authenticated password and blocks (rather than skipping) the encrypted charges when the key is unavailable. Shares one database-config validator with the restore CLI (dedup) in `src/shared/db/database-config.ts`, and one owner-KEK dispatch plus a private-key-from-data-key helper in `src/shared/crypto/owner-kek.ts` (used by the verifier; the login flow keeps its inline dispatch). Touches no provider adapters, payment callback/classification/refund paths, PR3_PLAN.md, or aggregate runtime.
d933802 to
005fe6a
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d933802133
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Good-citizen fix: TODO.md (last touched by #2046) broke the `deno fmt --check` gate on main — its prose exceeded 80 columns. Pure reformatting (verified by empty word-diff), no content change. Unblocks the PR 13 verifier's CI checks job, which runs `lint:ci`.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54426664b6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ord echo Six Codex P2 findings, all verified against the codebase and fixed with regression tests: 1. Owner key unavailable now also blocks on encrypted merge-reference charges, not only attendee PII — no silent skip when no key is supplied (PLAN.md PR 13: do not silently skip charges). 2. `--page-size` now flows to the database reader (factory `createReader`), so the keyset page size that bounds each read matches what the operator asked for (previously the reader always used the default 500). 3. Removed the false-positive `merge_reference_without_source_attendee` and the redundant `merge_reference_without_target_attendee` checks: `applyAttendeeMerge` deletes the source attendee in the same batch, so the source id is historical and the row's attendee_id (the merge target) is already covered by the general `processed_payment_without_attendee` check. 4. Read `failure_data`; exempt terminal handled failures (attendee NULL + failure_data set) from the missing-attendee check — only unresolved stuck reservations (attendee NULL + failure_data '') block. 5. Owner password read from non-tty stdin without echo (or `MIGRATION_VERIFY_PASSWORD` env var); `prompt()` remains the interactive fallback with a documented echo caveat. 6. Added `checkout_stage_without_attendee`: a stage whose own attendee_id points at a deleted attendee is now flagged even when the session shares a processed payment. readiness.ts mutation 100% (125/125, 1 provably-equivalent suppressed).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1605658487
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…e refs, timestamps, servicing, dead-split removal Seven more Codex P2 findings, all verified and fixed: 1. (deps) A non-empty pii_blob that is not hybrid ciphertext is no longer counted as verified — decryptPiiBlob is used (decrypt AND parse), so a corrupt-plaintext or malformed-JSON blob fails readiness. 2. (readiness) convertLegacyTimestamp now catches epoch-millis values outside Date's representable range (returns null instead of throwing). Required timestamps (processed_at, checkout_stages.created_at, sumup_checkouts. created_at) that are empty now contradict; only provider_refunded_at is optional. 3. (deps) --owner now requires an owner-level account: derive returns null for any non-owner (manager/agent/editor) carrying the site data key, so a non-owner cannot make the verifier decrypt PII. 4. (lib) Every processed_payments row with a non-empty payment_reference is verified (not only legacy-merge handoffs); a corrupt regular captured charge is reported as undecryptable_payment_reference. 5. (deps) PII blobs are parsed through the real PII schema after decrypt, so malformed-but-decryptable blobs (missing payment_id etc.) fail readiness. 6. (readiness) Removed the dead paymentsExceedingPage / payment_split_across_page check: payment_session_id is a primary key, so within one table a payment appears at most once and a keyset page can never split it (split detection is a PR 14 copy-cursor concern, not this read-only verifier). 7. (deps) readAttendeeIds filters kind = 'attendee', so a payment pointing at a servicing (van/crew) row blocks instead of reading as a valid attendee. readiness.ts mutation 100% (127/127, no survivors, no suppressions needed).
Addresses two Codex round-3 P2 findings on file size: - Move formatReadinessReport + CONTRADICTION_PHRASES out of readiness.ts into src/shared/migration-readiness/format.ts (readiness.ts: 486 -> 421 lines; the remaining content is the pure diagnosis rules). - Move the formatReadinessReport test suite into test/shared/migration-readiness/format.test.ts, with shared fixtures (enc/stage/processed/goodInput/mergeRefNoOwnerInput) extracted into fixtures.ts so neither test file duplicates them (readiness.test.ts: 567 -> 425; format.test.ts: 118). readiness.ts mutation stays 100% (90/90); format.ts mutation 100% (37/37). The two 'reject null keys before advancing the cursor' comments need no code change (explained in review replies): every paged table uses ORDER BY pk ASC, and SQLite sorts NULLs first, so the cursor (last row of each page) is never NULL and pk > NULL truncation cannot occur; NULL-pk rows are read, not skipped.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e12744b9f7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…mpty-decrypted charge Three Codex round-4 P2 findings, all verified and fixed: 1. (readiness) A session whose checkout_stages row and processed_payments row name different live attendees is now a contradiction (checkout_stage_attendee_mismatch) — previously passed because the stage and processed attendee checks were independent. 2. (readiness) hasEncryptedPaymentReference now keys on the hybrid ciphertext prefix, so a database with only legacy plaintext payment_reference rows no longer blocks when --owner is omitted (plaintext charges do not need the owner key). 3. (deps) paymentReferenceDecrypts rejects a hybrid payment_reference that decrypts to an empty string (an encrypted-but-empty charge is corrupt). Integration tests: 'blocks on a hybrid payment_reference that decrypts to an empty string' and unit: mismatch + plaintext-ref-doesn't-block. readiness.ts mutation 100% (101/101).
…files) - Remove the dead empty-blob skip in verify (readAttendeePii filters pii_blob != '', so empty blobs never reach it) — was an uncovered, impossible branch. - Add CLI tests for --page-size parsing: a numeric value reaches the reader with that page size, and a non-numeric value exits 2 with usage. Extract expectUsageExit to dedup the two usage cases. - Add an integration test for a legacy plaintext (non-hybrid) payment reference with the owner key (covers paymentReferenceDecrypts' non-hybrid branch). All changed source now at 100% line + branch coverage (migration-verify-deps, migration-verify-lib, readiness, format, database-config, owner-kek).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c41d0818a1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ), | ||
| readProcessedPayments: () => | ||
| keysetRows<ProcessedPaymentRow>( | ||
| "SELECT payment_session_id, attendee_id, processed_at, payment_reference, provider_refunded_at, failure_data FROM processed_payments", |
There was a problem hiding this comment.
When a finalized processed_payments row has non-empty ticket_tokens that no longer decrypts, this reader never loads that column, so diagnoseReadiness can still print ready even though the current replay path decrypts processed_payments.ticket_tokens in alreadyProcessedResult and the forward migration has to preserve that ticket state. Include and verify non-empty ticket-token ciphertext before claiming the legacy payment row is safe.
Useful? React with 👍 / 👎.
| return Deno.stdin.isTerminal() | ||
| ? prompt(message) |
There was a problem hiding this comment.
Fresh evidence in the current code is that the terminal branch still calls prompt(message), and the adjacent comment states that this echoes; when an operator runs deno task migration-verify --owner ... from an interactive terminal without piping stdin or setting MIGRATION_VERIFY_PASSWORD, the password protecting attendee PII is still typed visibly. Use a non-echoing TTY reader for this branch instead of making the secure path opt-in.
Useful? React with 👍 / 👎.
| const dataKey = await unwrapKey(user.wrapped_data_key, kek); | ||
| return privateKeyFromDataKey(dataKey, settings.wrappedPrivateKey); |
There was a problem hiding this comment.
When a restored database has a corrupt or mismatched wrapped_data_key or wrapped_private_key, these awaits reject instead of returning null, so assessOwnerKey never reaches its bounded “owner key not supplied” verdict and the operator gets an unhandled error for a source-readiness condition. Catch unwrap/import failures in derive and return null so encrypted PII and payment references block normally.
Useful? React with 👍 / 👎.
| ), | ||
| readSumupCheckouts: () => | ||
| keysetRows<SumupCheckoutRow>( | ||
| "SELECT reference_index, sumup_id, created_at FROM sumup_checkouts", |
There was a problem hiding this comment.
When a sumup_checkouts row has an empty wrapped_key or metadata value, this SELECT discards both fields and the readiness rules only check sumup_id, so the report can say ready even though the current getSumupCheckout path requires those encrypted fields to recover the staged booking metadata. Include those columns in the input and block empty values before certifying the row as migratable.
Useful? React with 👍 / 👎.
| * (attendee null + failure_data ''), a terminal failure is a normal stored | ||
| * outcome and must not block migration. */ | ||
| const isTerminalFailure = (row: ProcessedPaymentRow): boolean => | ||
| row.attendee_id === null && row.failure_data !== ""; |
There was a problem hiding this comment.
When a row has attendee_id NULL and any non-empty but corrupt failure_data, this predicate exempts it as a handled terminal failure, so readiness can pass without proving the encrypted failure JSON still decrypts and carries the refund facts the migration must preserve. Validate that blob at the boundary or report a contradiction before treating the row as safe.
Useful? React with 👍 / 👎.
1 participant
Current-system value
Operators can now run
deno task migration-verifyagainst the configureddatabase — a live one, or one freshly restored from an old backup into the
current application — and prove whether the legacy payment tables, attendee
PII, and merge references are safe to migrate, without writing anything.
Before this, an operator had no way to check migration readiness ahead of the
forward copy.
The command exits
0(ready) or1(blocked) and prints a bounded verdict:source counts, the owner-key verdict, and each contradiction in plain language.
Exit
2covers a usage or source-read failure.Production / operator caller
deno task migration-verify [--owner <username>] [--page-size <n>]— wired asa
deno.jsontask. Entry:scripts/migration-verify.ts; orchestration:scripts/migration-verify-lib.ts; DB + crypto wiring:scripts/migration-verify-deps.ts. The pure rules live insrc/shared/migration-readiness/readiness.tsand the operator report rendererin
src/shared/migration-readiness/format.ts; both unit-tested directly.What it does (PR 13 scope, one coherent read-only slice)
processed_payments(incl.failure_data),checkout_stages,sumup_checkouts(non-secret columns), attendee PII blobs,and attendee-merge references (
legacy-merge:<sourceId>rows).--page-size(default 500); groups one provider payment(
buildPaymentGroups) for a coherent report. (payment_session_idis theprimary key, so a keyset page can never split a single payment — the split
risk is a PR 14 copy-cursor concern.)
…sssZ(ISO-8601 or old whole-epoch-millis), rejecting impossible dates (Feb 30) and epoch overflow.
Required timestamps (
processed_at,checkout_stages.created_at,sumup_checkouts.created_at) that are empty contradict; onlyprovider_refunded_atis optional.payments pointing at deleted attendees, a stage and processed payment that
disagree on attendee, terminal handled failures exempt (
attendee_id NULL+failure_dataset), unconvertible/empty-required timestamps, sumup checkoutswithout a recorded id, undecryptable PII / payment references (hybrid
ciphertext that fails to decrypt or decrypts to an empty charge).
private key from an owner-level account's password (non-owners are
refused) and decrypts (and parses, via the real PII schema) every attendee
PII blob and every hybrid
payment_reference(regular charges and merge-reference handoffs; legacy plaintext refs don't need the key). If no owner
key is supplied (or derivation fails) and encrypted PII or hybrid payment
references exist, the verdict blocks — it never silently skips charges.
The owner password is read from non-tty stdin (or
MIGRATION_VERIFY_PASSWORD)without echo;
prompt()is the interactive fallback. PII plaintext neverleaves the decrypt step.
Codex review — all 20 findings addressed (7 commits)
Round 1 (
160565848): owner-key-unavailable blocks on merge-ref charges;--page-sizeflows to the reader; removed false-positive merge-source-attendeecheck;
failure_dataexempts terminal failures; non-tty password read;checkout_stage_without_attendeecheck.Round 2 (
10ac3c4c0): non-hybrid/malformed PII fails (decrypt AND parse);epoch-overflow catch + empty-required-timestamp contradictions;
--ownerrestricted to owner-level; all
payment_references verified (not just mergerefs) →
undecryptable_payment_reference; removed deadpayment_split_across_page(PK uniqueness);readAttendeeIdsfilterskind='attendee'(excludes servicing rows).Round 3 (
e12744b9f): splitformatReadinessReportintoformat.ts(readiness.ts 486→421) and the format tests into
format.test.ts(readiness.test.ts 567→425) with shared
fixtures.ts. The two "reject nullkeys before advancing the cursor" comments need no code change —
ORDER BY pk ASCsorts NULLs first, so the page cursor is never NULL andpk > NULLtruncation cannot occur.
Round 4 (
32579fe70): block sessions where the stage and processedattendees disagree (
checkout_stage_attendee_mismatch);hasEncryptedPaymentReferencekeys on the hybrid prefix so plaintext-only DBs don't block without
--owner;reject hybrid
payment_referencethat decrypts to""(corrupt).Coverage (
c41d0818a): removed the dead empty-blob skip inverify(never reached —
readAttendeePiifilters empty blobs); added CLI tests for--page-sizeparsing and an integration test for a legacy plaintext charge.All changed source now at 100% line + branch.
Shared helpers (dedup)
src/shared/db/database-config.ts— one DB-config validator used by bothdeno task restoreanddeno task migration-verify.src/shared/crypto/owner-kek.ts—deriveOwnerKek(v1/v2 KEK dispatch) +privateKeyFromDataKey, used by the verifier (login flow keeps its inlinedispatch as a documented follow-up).
Good-citizen fix (recorded separately)
TODO.md(last touched by #2046) broke thedeno fmt --checkgate on main —commit
54426664bapplies the mechanicaldeno fmtwrapping (empty word-diff:no content change). Unblocks the PR's
checksCI job and main.Source and total line counts
src/insertions + deletions vsmain: 630 (limit: 800).mutants entry, and the TODO.md reformat).
keys.tsandauth.tsare unchanged vsmain.005fe6aslice,5442666TODO fmt,1605658Codex r1,10ac3c4Codex r2,e12744bCodex r3 split,32579feCodex r4,c41d081coverage).Database and provider call budget
Full-Deno CLI (not an edge isolate), so not bound by Bunny's 50-subrequest
budget. Reads are keyset-paginated. Zero provider calls — only legacy DB
reads + owner-key decryption. One
settings.loadKeysread for the wrappedprivate key.
Tests and mutation
test/shared/migration-readiness/readiness.test.ts— pure rules.test/shared/migration-readiness/format.test.ts— report rendering.test/shared/migration-readiness/fixtures.ts— shared fixtures.test/shared/db/database-config.test.ts— config validator.test/shared/crypto/owner-kek.test.ts— KEK dispatch + key recovery.test/scripts/migration-verify.test.ts— CLI orchestration (fakes).test/integration/migration-verify.test.ts(16) — end-to-end against asetup-complete test DB (real owner-key derivation, keyset pagination, terminal
failures, merge + regular + plaintext + empty-decrypted charge verification,
non-owner refusal, servicing-row exclusion,
--page-sizeflow).Coverage: all changed source at 100% line + branch.
Targeted mutation (
--jobs 1 --timeout 60000):readiness.ts100% (101/101),format.ts100% (37/37),database-config.ts100% (27/27),owner-kek.ts100% (4/4).keys.ts/auth.tsunchanged vsmain(outside the diff, not mutated).scripts/files aren't mutation-gated (same convention asrestore-lib.ts).Known PLAN.md faults addressed
A read-only readiness check against the legacy reader tables — addresses PR 13's
inspect-losslessly / group-before-pagination / convert-timestamps / surface-
contradictions / owner-key-control requirements, proving readiness before PR
14 changes payment history. It detects (does not fix) the conditions that
would trigger PR 14 copy faults.
Deferred PR 13 / follow-up work
deriveOwnerKek(kept inline to avoid pullingkeys.ts/auth.tswhole-file mutation into this slice).deterministic in-process test) — the non-tty/env paths are the testable
no-echo routes.
readiness.tsis 421 lines (just over the soft ~400 target); the formatsplit removed the renderer, leaving cohesive diagnosis logic further
splitting would fragment.
PR 3 paths untouched
No provider adapters, payment callback/classification/refund paths,
PR3_PLAN.md, or aggregate-runtime code modified. The verifier only reads thelegacy tables and decrypts PII with the owner key. (
keys.tsandauth.tshave zero net diff vs
main.)