Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 34 additions & 31 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,36 @@
## Listing/groups review follow-ups (from PR #2046)

The shared driver this PR landed now gives these a single seam: the two edge
writers (`setListingChildrenWithPackageCheckTx` / `addParentEdgesWithPackageCheckTx`)
both delegate their transaction-local recheck to
`guardEdgeWriteTx` in `src/shared/db/listing-edge-write.ts` — one declared check
list (existence, nesting, package) running against current tx state. The items
below are the natural **next entries** in that declarative check list, not
separate hand-rolled guards; each still needs the tx-scoped read it mentions.
The remaining ones are transaction-race hardening on already-rare windows, so
they were deferred from the PR rather than implemented there.
writers (`setListingChildrenWithPackageCheckTx` /
`addParentEdgesWithPackageCheckTx`) both delegate their transaction-local
recheck to `guardEdgeWriteTx` in `src/shared/db/listing-edge-write.ts` — one
declared check list (existence, nesting, package) running against current tx
state. The items below are the natural **next entries** in that declarative
check list, not separate hand-rolled guards; each still needs the tx-scoped read
it mentions. The remaining ones are transaction-race hardening on already-rare
windows, so they were deferred from the PR rather than implemented there.

- **Xh_QZ — Redirect vanished-group failures to a live page.**
`handleAddListingsToGroup` in `src/features/admin/groups.ts` redirects every
`assignListingsToGroup` error to `/admin/groups/${group.id}`. When the group is
deleted after the handler loads it but before the write, `assignListingsToGroup`
returns `t("error.selected_group_deleted")` and that redirect lands on a 404.
Route that one result to `/admin/groups` (the live list). Reasoning for defer:
the group must vanish between the handler's load and the write — a window not
reachable from a single-request test without fragile cache manipulation, so
it shipped without coverage.
`assignListingsToGroup` error to `/admin/groups/${group.id}`. When the group
is deleted after the handler loads it but before the write,
`assignListingsToGroup` returns `t("error.selected_group_deleted")` and that
redirect lands on a 404. Route that one result to `/admin/groups` (the live
list). Reasoning for defer: the group must vanish between the handler's load
and the write — a window not reachable from a single-request test without
fragile cache manipulation, so it shipped without coverage.

- **XiL8J / XiL8L — Revalidate edge fields inside the write transaction.**
`guardEdgeWriteTx` rechecks existence, nesting, and package membership in the
tx, but if another admin changes a parent's or a selected child's type, renewal
tier, duration, or day prices after `validateChildEdges`/`validateParentEdges`
runs, it commits a relationship `edgeFieldError` would now reject. Fix: load
both endpoints' current edge columns (and day prices) through `tx` and rerun
`edgeFieldError` as the next entry in `guardEdgeWriteTx`'s check list. Note
`name` and some fields are encrypted (PII), so the read must select only the
plain edge columns `edgeFieldError` reasons over rather than decrypt under the
write lock. All sibling recheck guards were already implemented.
tx, but if another admin changes a parent's or a selected child's type,
renewal tier, duration, or day prices after
`validateChildEdges`/`validateParentEdges` runs, it commits a relationship
`edgeFieldError` would now reject. Fix: load both endpoints' current edge
columns (and day prices) through `tx` and rerun `edgeFieldError` as the next
entry in `guardEdgeWriteTx`'s check list. Note `name` and some fields are
encrypted (PII), so the read must select only the plain edge columns
`edgeFieldError` reasons over rather than decrypt under the write lock. All
sibling recheck guards were already implemented.

- **Xig83 / XiqKh — Recheck add-on reachability inside the write transaction.**
Same race-revalidation family as the edge-field entry, extended to optional
Expand All @@ -42,17 +43,19 @@ they were deferred from the PR rather than implemented there.
entry to `guardEdgeWriteTx`'s check list, resolving scope against
transaction-local modifier + `group_listings` state (a tx-scoped variant of
`modifier-resolve`'s live resolver). CodeRabbit's `XiqKh` is the combined view
of this and the edge-field entry, scoped to `api-listing-joins.ts:
persistListingJoins`; `Xig83` is Codex's form/API view.
of this and the edge-field entry, scoped to
`api-listing-joins.ts:
persistListingJoins`; `Xig83` is Codex's form/API
view.

- **XjDI9 — Read prior package flags on the transaction connection.**
`requirePackageGuardsTx` in `src/shared/db/groups/membership.ts` computes
`wasHiddenPackage` from the caller-supplied `existing` snapshot, which predates
the write transaction. If another request makes a visible group hidden and a
checkout sells it after the snapshot but before this transaction, a stale edit
can clear `is_package` without running `hasPackageBookingsTx`, exposing sold
hidden member names. The blocker: `writeRowInTransaction` runs the
`afterWrite` hooks after the UPDATE, so the hook can't read the pre-update
`wasHiddenPackage` from the caller-supplied `existing` snapshot, which
predates the write transaction. If another request makes a visible group
hidden and a checkout sells it after the snapshot but before this transaction,
a stale edit can clear `is_package` without running `hasPackageBookingsTx`,
exposing sold hidden member names. The blocker: `writeRowInTransaction` runs
the `afterWrite` hooks after the UPDATE, so the hook can't read the pre-update
flags from the DB (documented at the `PackageRow` definition). A correct fix
reads the current `is_package`/`hide_package_listings` on the transaction
connection _before_ the UPDATE and validates under the same lock — an
Expand Down
1 change: 1 addition & 0 deletions deno.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"backup": "deno run --env-file --allow-env --allow-read --allow-write --allow-net --allow-sys --allow-ffi scripts/backup.ts",
"restore": "deno run --allow-env --allow-read --allow-write --allow-net --allow-sys --allow-ffi scripts/restore.ts",
"snapshot": "deno run --allow-env --allow-read --allow-write --allow-net --allow-sys --allow-ffi scripts/database-snapshot.ts",
"migration-verify": "deno run --allow-env --allow-read --allow-write --allow-net --allow-sys --allow-ffi scripts/migration-verify.ts",
"migrate:turso": "deno run --allow-env --allow-read --allow-write --allow-net --allow-sys --allow-ffi scripts/turso-migration.ts",
"migrate:sites": "deno run --allow-env --allow-read --allow-write --allow-net --allow-sys --allow-ffi scripts/site-migration.ts",
"cli:tui": "deno run --allow-env --allow-read --allow-run cli/tui.ts",
Expand Down
200 changes: 200 additions & 0 deletions scripts/migration-verify-deps.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
/**
* Production wiring for the migration-readiness verifier.
*
* Builds the database-backed reader and the owner-key provider that
* `runMigrationVerifyCli` (in `migration-verify-lib.ts`) drives. The reader
* keyset-paginates the legacy payment tables so a large database never trips
* libsqld's "Response is too large" cap; the owner-key provider derives the
* site's private key from an owner-authenticated password and decrypts attendee
* PII and merge-reference charges in-process. Nothing here writes to the
* database — every read is read-only migration input.
*/

import type { InValue } from "@libsql/client";
import type {
MigrationVerifyOwnerKey,
MigrationVerifyReader,
} from "#scripts/migration-verify-lib.ts";
import {
decryptWithOwnerKey,
HYBRID_PREFIX,
unwrapKey,
} from "#shared/crypto/keys.ts";
import {
deriveOwnerKek,
privateKeyFromDataKey,
} from "#shared/crypto/owner-kek.ts";
import type { OwnerKeyEncrypted } from "#shared/crypto/sealed.ts";
import { ATTENDEE_KIND } from "#shared/db/attendees/kind.ts";
import { decryptPiiBlob } from "#shared/db/attendees/pii.ts";
import { queryAll } from "#shared/db/client.ts";
import { settings } from "#shared/db/settings.ts";
import {
decryptAdminLevel,
getUserByUsername,
verifyUserPassword,
} from "#shared/db/users.ts";
import type {
AttendeePiiSource,
CheckoutStageRow,
ProcessedPaymentRow,
SumupCheckoutRow,
} from "#shared/migration-readiness/readiness.ts";
import { CONFIG_KEYS } from "#shared/settings/keys.ts";

const DEFAULT_VERIFY_PAGE_SIZE = 500;

/** Read every row of a table as keyset pages, so no single libsql response
* exceeds its payload cap. `whereClause` narrows the read (e.g. real-audience
* PII); the cursor advances past the previous page's last primary key. */
const keysetRows = async <T>(
sqlPrefix: string,
whereClause: string | null,
pkColumn: string,
pageSize: number,
): Promise<T[]> => {
const rows: T[] = [];
let after: InValue = null;
for (;;) {
const conds: string[] = [];
const args: InValue[] = [];
if (whereClause) conds.push(whereClause);
if (after !== null) {
conds.push(`${pkColumn} > ?`);
args.push(after);
}
const where = conds.length ? ` WHERE ${conds.join(" AND ")}` : "";
const page = await queryAll<T>(
`${sqlPrefix}${where} ORDER BY ${pkColumn} LIMIT ?`,
[...args, pageSize],
);
if (page.length === 0) break;
rows.push(...page);
after = (page[page.length - 1] as Record<string, unknown>)[
pkColumn
] as InValue;
Comment thread
chobble-opencode-vm[bot] marked this conversation as resolved.
Comment thread
chobble-opencode-vm[bot] marked this conversation as resolved.
if (page.length < pageSize) break;
}
return rows;
};

/** The legacy payment tables the verifier reads, in the order its reports list
* them. Each read selects only the columns the readiness rules use. */
export const createMigrationVerifyReader = (
pageSize: number = DEFAULT_VERIFY_PAGE_SIZE,
): MigrationVerifyReader => ({
readAttendeeIds: () => {
// Only real attendees hold payments; servicing rows (vans/crews) are never
// valid payment targets, so exclude them from the live-attendee set.
const ids = keysetRows<{ id: number }>(
"SELECT id FROM attendees",
Comment thread
chobble-opencode-vm[bot] marked this conversation as resolved.
`kind = '${ATTENDEE_KIND}'`,
"id",
pageSize,
);
return ids.then((rows) => new Set(rows.map((row) => row.id)));
},
readAttendeePii: () =>
keysetRows<AttendeePiiSource>(
"SELECT id, pii_blob FROM attendees",
"kind = 'attendee' AND pii_blob != ''",
"id",
pageSize,
),
readCheckoutStages: () =>
keysetRows<CheckoutStageRow>(
"SELECT payment_session_id, attendee_id, provider, state, created_at FROM checkout_stages",
null,
"payment_session_id",
pageSize,
),
readProcessedPayments: () =>
keysetRows<ProcessedPaymentRow>(
"SELECT payment_session_id, attendee_id, processed_at, payment_reference, provider_refunded_at, failure_data FROM processed_payments",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Verify stored ticket-token ciphertext

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 👍 / 👎.

null,
"payment_session_id",
pageSize,
),
readSumupCheckouts: () =>
keysetRows<SumupCheckoutRow>(
"SELECT reference_index, sumup_id, created_at FROM sumup_checkouts",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Read the encrypted SumUp staging fields

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 👍 / 👎.

null,
"reference_index",
pageSize,
),
});

/** Whether an owner-key-encrypted payment reference decrypts under the key. An
* empty value is nothing to verify. A non-hybrid value is a legacy plaintext
* payment_reference (development builds wrote the column in the clear — see
* `payment-references.ts`), so it is treated as decryptable. A hybrid
* ciphertext that throws on decrypt, or decrypts to an empty string (an
* encrypted-but-empty charge is corrupt), is not. Returns no plaintext. */
const paymentReferenceDecrypts = async (
value: OwnerKeyEncrypted | "",
key: CryptoKey,
): Promise<boolean> => {
if (value === "" || !value.startsWith(HYBRID_PREFIX)) return true;
try {
const plaintext = await decryptWithOwnerKey(
value as OwnerKeyEncrypted,
key,
);
return plaintext !== "";
} catch {
return false;
}
};

/**
* The owner-key provider: an owner-authenticated step that derives the site
* private key from an owner password, then proves it can decrypt (and parse)
* every attendee PII blob and every payment reference. A wrong password, a
* non-owner account, a missing wrapped-data key, or an absent wrapped private
* key returns null — the caller then blocks rather than skipping the encrypted
* charges. PII plaintext never leaves this step; only ids/keys that failed are
* returned.
*/
export const createMigrationVerifyOwnerKey = (): MigrationVerifyOwnerKey => ({
derive: async (username, password) => {
const user = await getUserByUsername(username);
if (!user?.wrapped_data_key) return null;
const passwordHash = await verifyUserPassword(user, password);
if (!passwordHash) return null;
Comment thread
chobble-opencode-vm[bot] marked this conversation as resolved.
// The private key protects attendee PII for the whole site, so only an
// owner-level account may derive it through this command.
const adminLevel = await decryptAdminLevel(user);
if (adminLevel !== "owner") return null;
await settings.loadKeys([CONFIG_KEYS.WRAPPED_PRIVATE_KEY]);
if (!settings.wrappedPrivateKey) return null;
const kek = await deriveOwnerKek(password, passwordHash, user.kek_version);
const dataKey = await unwrapKey(user.wrapped_data_key, kek);
return privateKeyFromDataKey(dataKey, settings.wrappedPrivateKey);
Comment on lines +171 to +172

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Return null when owner-key unwrap fails

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 👍 / 👎.

},
verify: async (key, inputs) => {
const undecryptablePii = new Set<number>();
const undecryptablePaymentReferences = new Set<string>();
for (const { id, pii_blob } of inputs.attendees) {
// readAttendeePii filters pii_blob != '', so every blob here is
// non-empty hybrid ciphertext (or corrupt). Decrypt AND parse: a blob
// that decrypts to malformed JSON or one missing required PII fields
// would fail the real attendee readers, so it must fail readiness too.
// Non-hybrid blobs throw here (PII has no legacy plaintext fallback),
// catching corrupt plaintext PII.
try {
await decryptPiiBlob(pii_blob as OwnerKeyEncrypted, key, true);
} catch {
undecryptablePii.add(id);
Comment thread
chobble-opencode-vm[bot] marked this conversation as resolved.
}
}
for (const {
payment_reference,
payment_session_id,
} of inputs.paymentReferences) {
if (!(await paymentReferenceDecrypts(payment_reference, key))) {
undecryptablePaymentReferences.add(payment_session_id);
}
}
return { undecryptablePaymentReferences, undecryptablePii };
},
});
Loading