diff --git a/docs/database-locks.md b/docs/database-locks.md index c55bcacd..08505623 100644 --- a/docs/database-locks.md +++ b/docs/database-locks.md @@ -43,7 +43,7 @@ lists (see the pool row below). | Pool row | `core.resource_pools` row of one pool | `FOR UPDATE`; `FOR KEY SHARE` in a plan act's listing batch | the functions `core.confer`, `core.end_conferral`, `core.align_conferral_shape`, `core.update_conferral_bounds` and `core.settle_obligation`; `LockPools` (through the `LockResourcePool` query), called by `RevokeGrantTx`, `ManageGrantTx`, `ResumeReplacedGrantsTx`, `settlePool`, `runPlanAct` (`AddTier`, `RemoveTier` and `EnactObligation`, a default change's move), and `runPlanAct` for a reorder's move (`EnactObligation`); `LockPoolOfGrant` (through the `LockGrantPool` query), called by `ExpireGrantTx` and `ExtendWaitingGrantTx`; and `core.write_plan_act_obligations`, which takes the pool rows of one listing batch `FOR KEY SHARE` | Everything that changes a pool's positions or entitlements: conferral, endings, grant acts, tier removal, a default change's move, rule-change settlement and grant resumption. A listing batch's key-share lock conflicts with `FOR UPDATE`, so a batch and a move or a conferral on one of its pools wait for each other, and two batches of one act share the rows. | | Entitlement set row and rule row | `core.entitlement_sets` row and `core.entitlement_set_rules` row | `FOR UPDATE` | `core.commit_rule_change` (the set row also by the `LockEntitlementSet` query, once per batch) | A rule change against a writer that changes the set or the rule without holding the rendezvous. | | Change obligation row | `core.entitlement_set_change_obligations` row of one (change, pool) | `FOR UPDATE` | `core.settle_obligation` | Two passes settling one owed pool, so each effect is written once. | -| Plan act obligation row | `core.plan_act_obligations` row of one (act, pool) | `FOR UPDATE` by a move, a listing batch's supersession and Retry's requeue; `UPDATE` by the failure recorder | the `LockPlanActObligation` query (a move, after the pool row), `core.write_plan_act_obligations` (the open obligations of earlier acts on the batch's pools), `core.requeue_plan_act_obligations` (the retried act's failed obligations of the types it still owns) and `core.record_plan_act_obligation_failure` | Two passes settling one obligation, and a settlement against a supersession or a requeue. A batch and a Retry take several rows in ascending `pool_id` order and touch disjoint rows: a batch supersedes the open rows of earlier acts on the types its act owns, a Retry requeues the failed rows of the types its act still owns, and both hold the shared rendezvous, so no default change can move the ownership while either runs. | +| Plan act obligation row | `core.plan_act_obligations` row of one (act, pool) | `FOR UPDATE` by a move, a listing batch's supersession and Retry's requeue; `UPDATE` by the failure recorder | the `LockPlanActObligation` query (a move, after the pool row), which calls `core.lock_plan_act_obligation`; the function takes the row `FOR UPDATE`, `core.write_plan_act_obligations` (the open obligations of earlier acts on the batch's pools), `core.requeue_plan_act_obligations` (the retried act's failed obligations of the types it still owns) and `core.record_plan_act_obligation_failure` | Two passes settling one obligation, and a settlement against a supersession or a requeue. A batch and a Retry take several rows in ascending `pool_id` order and touch disjoint rows: a batch supersedes the open rows of earlier acts on the types its act owns, a Retry requeues the failed rows of the types its act still owns, and both hold the shared rendezvous, so no default change can move the ownership while either runs. | | Org type row | `core.org_types` row of one type | `FOR NO KEY UPDATE` | the `LockOrgType` query, run by the default change's T0 | The T0 against the demonstration seed and an edit of the type's other columns, which write the row without the rendezvous. Every other writer of the default already waits on the exclusive rendezvous. `FOR NO KEY UPDATE` does not conflict with the `FOR KEY SHARE` an organization insert takes on its type's row. | | Grant lineage head | the `core.grants` row no other grant extends | `FOR UPDATE OF g` | the `GetGrantLineageHeadForUpdate` query, run by `RevokeGrantTx`, `ExtendWaitingGrantTx` and `ManageGrantTx` | Acts on one lineage, so an extension that committed while another act waited is visible to it. | | Usage row | `core.numeric_entitlement_usage` row of one (pool, resource key) | `FOR UPDATE` | the `LockNumericUsage` query, run by `usage.Recount` and by the slot gate, after the site rows the transaction writes and before the request rows it finishes (the hold rows have no place in the order, see rule 5); the count is a statement of its own after the lock | Two recounts of one pool, so each count reads every change committed before its lock was granted; two slot gates for one pool, so the second counts the first's hold and two creates cannot both take the last slot. | @@ -250,9 +250,10 @@ transaction timeout before the rendezvous statement. benign: the next pass lists the batch again. - **One organization's move** (`EnactObligation`, through `runPlanAct`): the shared rendezvous, the pool row (`LockPools`), then the obligation row - (`FOR UPDATE`, with the type's owner read in the same statement; a move - skips an obligation that is not pending or whose act no longer owns the - type), then the reads (the type's default and the pool's positions) and the + (`FOR UPDATE`, taken by `core.lock_plan_act_obligation`, which the + `LockPlanActObligation` query calls; the function reads the type's owner in + the same statement; a move skips an obligation that is not pending or whose + act no longer owns the type), then the reads (the type's default and the pool's positions) and the bucket's work (`ReapplyDefaultsIfVacant`, `ConferGrantTx` for a grandfathered incumbent, or `core.end_conferral`, `ReapplyDefaultsIfVacant` and materialization for a migration), which take the same pool row again, diff --git a/docs/database-management.md b/docs/database-management.md index 5c98c7d0..c6263980 100644 --- a/docs/database-management.md +++ b/docs/database-management.md @@ -161,6 +161,14 @@ there is no privilege wiring performed from outside a schema's own migration stream, apart from the console's grant of `console_runtime` to the runtime user at start and `console_reader`, below. +The objects the migrations create belong to the user that runs the +migrations, with two exceptions. `core_owner` owns the `SECURITY DEFINER` +functions of the core schema, and it owns `core.webhook_events` with its +partitions, because creating a partition requires the owner of the parent table +and `core.ensure_webhook_event_partitions`, which `console_runtime` alone is +granted, runs as `core_owner`. The runtime user owns no object in the schemas +and does not own the database. + ## The `console_reader` role `scripts/db-reader-role.sql` creates and maintains `console_reader`, a login diff --git a/docs/models/payments-billing.md b/docs/models/payments-billing.md index 111ee412..a7085eca 100644 --- a/docs/models/payments-billing.md +++ b/docs/models/payments-billing.md @@ -29,7 +29,7 @@ Database objects sit in the `core` schema (defined in `internal/db/migrations/00 | `core.subscription_scheduled_changes` | The intent ledger: future plan switches, quantity changes, and cancellations, with when they fire and their lifecycle status | `00001_init.sql` | | `core.invoices`, `core.invoice_line_items`, `core.payments`, `core.payment_methods` | The money projections: amounts in integer minor units (cents) with a per-row currency; payment methods store safe card descriptors (brand, last4, expiry), never card data. `core.invoices.invoice_number` [db] is the platform's own reference number (upstream data model v16.2; identifier policy arbitration 2026-08-29): a zero-padded decimal of at least four digits (e.g. `0042`), assigned per billing account from `core.accounts.next_invoice_number` — a transactional, gapless counter, never a Postgres sequence — in the same transaction that projects the invoice; unique per billing account where present, immutable once assigned, required for every non-draft row (`chk_invoices_issued_have_number`). Rows numbered before this scheme (v16.2) keep the Stripe-format string they were quoted with verbatim, so one account may show both forms; Stripe's own customer-facing number moves to `stripe.invoice_mappings.stripe_invoice_number` as an external reference, shown on the invoice detail as "Stripe invoice ..." and never treated as the invoice's identity. `created_at` on these rows is when the console wrote them, set by the database and never backdated; the times Stripe reports have columns of their own: an invoice's `issued_at` (Stripe's date of issue, or its finalization; required for every non-draft row, `chk_invoices_issued_have_issued_at`), `paid_at` and `voided_at`, and a payment's `paid_at` and `failed_at`. Every invoice list sorts by `issued_at` and every payment list by `paid_at`, else `failed_at`, newest first, with the id breaking ties | `00001_init.sql`; `00013_invoice_numbers.sql`; `00022_invoice_issued_payment_paid.sql`; `internal/integrations/stripe/store/migrations/00003_invoice_number.sql` | | `core.webhook_events` | The inbound event store all integrations share (Discourse uses it too): every verified Stripe event lands here before processing; partitioned by month, meaning each month's rows live in their own physical sub-table; partitions are created ahead of need at boot and by a recurring maintenance schedule | `00001_init.sql` | -| Partition maintenance | The idempotent ensure pass that creates upcoming monthly partitions, run once at boot and on a 24h Temporal schedule (`webhook-partition-ensure`) | `internal/db/partitions.go`, `internal/workflows/maintenance/` | +| Partition maintenance | The idempotent ensure pass that creates upcoming monthly partitions, run once at boot and on a 24h Temporal schedule (`webhook-partition-ensure`) | `internal/db/migrations/00031_webhook_partition_function.sql`, `internal/db/partitions.go`, `internal/workflows/maintenance/` | | `core.outbox` | The transactional outbox for outbound work: rows enqueue in the same database transaction as the domain write that needs them, and a drainer executes them against Stripe | `00001_init.sql` | | `stripe` schema | Eight mapping tables (customer, product, price, subscription, subscription item, invoice, payment, payment method) — each links one console UUID to one Stripe id, both unique, with a `sync_status` that starts `pending` for objects the console pushes out and `synced` for objects projected in. A payment is one attempt, so its Stripe id is the charge (`stripe_charge_id`, unique); the payment intent the attempts share (`stripe_payment_intent_id`) and the Stripe payment method the charge tried (`stripe_payment_method_id`) sit beside it. A payment without a Stripe charge is one paid outside Stripe or for an invoice with nothing due; the reconcile creates it for a paid invoice with no succeeded charge. An invoice mapping also keeps the Stripe subscription the invoice belongs to (`stripe_subscription_id`, so the link can be filled once that subscription is mapped) and `read_back_at`, the last time the invoice reconcile read the invoice back and reached an outcome repeating would not change | `internal/integrations/stripe/store/migrations/00001_init.sql`; `00005_invoice_read_back.sql` | | Webhook receiver | Verifies the Stripe signature, scrubs personal data from the payload, and stores the event | `internal/integrations/stripe/web/webhook.go` | diff --git a/internal/db/integration_delivery_migration_test.go b/internal/db/integration_delivery_migration_test.go index 382cfaae..41b8c396 100644 --- a/internal/db/integration_delivery_migration_test.go +++ b/internal/db/integration_delivery_migration_test.go @@ -141,23 +141,25 @@ func assertDeliverySchemaPlacement(t *testing.T, database *sql.DB) { // columns with their types, defaults, generation and column privileges // (not their attnum, which a dropped and re-added column does not keep), // constraints, indexes, triggers, functions with their definitions, owners, -// security and privileges, and comments. The lines are sorted, so two +// security and privileges, and comments. Every ACL is rendered with its +// entries sorted, so a re-granted entry appended at the end of the stored ACL +// compares equal to the same set in another order. The lines are sorted, so two // snapshots compare with slices.Equal. func schemaSnapshot(t *testing.T, ctx context.Context, database *sql.DB) []string { t.Helper() const userSchemas = `n.nspname NOT LIKE 'pg\_%' AND n.nspname <> 'information_schema'` queries := []string{ - `SELECT 'schema ' || n.nspname || ' owner=' || pg_get_userbyid(n.nspowner) || ' acl=' || coalesce(n.nspacl::text, '') + `SELECT 'schema ' || n.nspname || ' owner=' || pg_get_userbyid(n.nspowner) || ' acl=' || coalesce((SELECT string_agg(x::text, ',' ORDER BY x::text) FROM unnest(n.nspacl) x), '') FROM pg_namespace n WHERE ` + userSchemas, `SELECT 'relation ' || n.nspname || '.' || c.relname || ' kind=' || c.relkind::text || ' owner=' || pg_get_userbyid(c.relowner) - || ' acl=' || coalesce(c.relacl::text, '') + || ' acl=' || coalesce((SELECT string_agg(x::text, ',' ORDER BY x::text) FROM unnest(c.relacl) x), '') FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace WHERE ` + userSchemas + ` AND c.relkind IN ('r', 'p', 'v', 'm', 'S', 'f')`, `SELECT 'column ' || n.nspname || '.' || c.relname || '.' || a.attname || ' ' || format_type(a.atttypid, a.atttypmod) || ' notnull=' || a.attnotnull || ' default=' || coalesce(pg_get_expr(d.adbin, d.adrelid), '') || ' generated=' || a.attgenerated::text || ' identity=' || a.attidentity::text - || ' acl=' || coalesce(a.attacl::text, '') + || ' acl=' || coalesce((SELECT string_agg(x::text, ',' ORDER BY x::text) FROM unnest(a.attacl) x), '') FROM pg_attribute a JOIN pg_class c ON c.oid = a.attrelid JOIN pg_namespace n ON n.oid = c.relnamespace @@ -179,7 +181,7 @@ func schemaSnapshot(t *testing.T, ctx context.Context, database *sql.DB) []strin JOIN pg_namespace n ON n.oid = c.relnamespace WHERE ` + userSchemas + ` AND NOT tg.tgisinternal`, `SELECT 'function ' || p.oid::regprocedure::text || ' owner=' || pg_get_userbyid(p.proowner) - || ' secdef=' || p.prosecdef || ' acl=' || coalesce(p.proacl::text, '') || E'\n' + || ' secdef=' || p.prosecdef || ' acl=' || coalesce((SELECT string_agg(x::text, ',' ORDER BY x::text) FROM unnest(p.proacl) x), '') || E'\n' || CASE WHEN p.prokind IN ('f', 'p', 'w') THEN pg_get_functiondef(p.oid) ELSE '' END FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace WHERE ` + userSchemas, diff --git a/internal/db/migrations/00001_init.sql b/internal/db/migrations/00001_init.sql index e9eed7c5..d5fd5791 100644 --- a/internal/db/migrations/00001_init.sql +++ b/internal/db/migrations/00001_init.sql @@ -1047,21 +1047,24 @@ CREATE TABLE core.webhook_events ( -- +goose StatementBegin -- Create initial partitions: current month + next 2 months. Future --- partitions are created by application boot or a scheduler. +-- partitions are created by core.ensure_webhook_event_partitions (00031). The +-- month is the UTC month and the bounds are UTC instants, whatever the +-- session's TimeZone, as that function computes them; the naming and bounds +-- must match it. DO $$ DECLARE - m_start DATE; - m_end DATE; + m_start TIMESTAMP; -- a UTC wall-clock time + m_end TIMESTAMP; part TEXT; BEGIN FOR i IN 0..2 LOOP - m_start := DATE_TRUNC('month', NOW()) + (i || ' months')::INTERVAL; - m_end := m_start + '1 month'::INTERVAL; + m_start := DATE_TRUNC('month', NOW() AT TIME ZONE 'UTC') + make_interval(months => i); + m_end := m_start + INTERVAL '1 month'; part := 'core.webhook_events_' || TO_CHAR(m_start, 'YYYY_MM'); EXECUTE FORMAT( 'CREATE TABLE IF NOT EXISTS %s PARTITION OF core.webhook_events FOR VALUES FROM (%L) TO (%L)', - part, m_start, m_end + part, m_start AT TIME ZONE 'UTC', m_end AT TIME ZONE 'UTC' ); END LOOP; END $$; diff --git a/internal/db/migrations/00031_webhook_partition_function.sql b/internal/db/migrations/00031_webhook_partition_function.sql new file mode 100644 index 00000000..0ef3f5b4 --- /dev/null +++ b/internal/db/migrations/00031_webhook_partition_function.sql @@ -0,0 +1,107 @@ +-- SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial +-- SPDX-FileCopyrightText: 2025-2026 Christian Galo + +-- +goose Up +-- --------------------------------------------------------------------------- +-- Partitions of core.webhook_events are created by a function (openspec change +-- runtime-database-user, design D3 and D4). The console serves from a user +-- that owns nothing, and CREATE TABLE ... PARTITION OF requires the owner of +-- the parent table. core_owner therefore owns core.webhook_events and each of +-- its partitions, and core.ensure_webhook_event_partitions, a SECURITY DEFINER +-- function owned by core_owner, creates the partitions for the caller. +-- ALTER TABLE ... OWNER TO on the parent does not change its partitions' +-- owners, so the loop below moves each partition too; the parent's indexes and +-- identity sequence move with it. A partition the function creates belongs to +-- core_owner because the function runs as core_owner. +-- +-- EXECUTE goes to console_runtime alone: creating a partition is a privilege +-- of the process that serves the console. The window is capped at 12 months +-- ahead, so one call creates at most 13 tables. +-- +-- The naming and the bounds must match the baseline's initial-partition DO +-- block (00001_init.sql) and partitionTableName in partitions_test.go: the table +-- core.webhook_events_YYYY_MM of the UTC month, bounded by that month's first +-- instant and the next month's, in UTC whatever the session's TimeZone. +-- +-- The Down drops the function and gives the parent and each partition back to +-- the user running the Down. The baseline's GRANT ALL ON ALL TABLES IN SCHEMA +-- core TO core_owner left core_owner an explicit entry on the parent and on +-- each baseline partition; the Up's OWNER TO core_owner merges that entry into +-- the owner's, and OWNER TO CURRENT_USER would leave core_owner with none. The +-- Down therefore re-grants ALL to core_owner on each table, so the database is +-- back where the baseline left it. +-- --------------------------------------------------------------------------- + +-- +goose StatementBegin +DO $$ +DECLARE + rel RECORD; +BEGIN + ALTER TABLE core.webhook_events OWNER TO core_owner; + FOR rel IN + SELECT n.nspname AS schema_name, c.relname AS table_name + FROM pg_catalog.pg_inherits i + JOIN pg_catalog.pg_class c ON c.oid = i.inhrelid + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + WHERE i.inhparent = 'core.webhook_events'::pg_catalog.regclass + LOOP + EXECUTE pg_catalog.format('ALTER TABLE %I.%I OWNER TO core_owner', rel.schema_name, rel.table_name); + END LOOP; +END $$; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE FUNCTION core.ensure_webhook_event_partitions(p_now TIMESTAMPTZ, p_months_ahead INTEGER) +RETURNS VOID +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = core, pg_temp +AS $$ +DECLARE + month_start TIMESTAMP; -- a UTC wall-clock time +BEGIN + IF p_now IS NULL THEN + RAISE EXCEPTION 'p_now must not be null' USING ERRCODE = 'invalid_parameter_value'; + END IF; + IF p_months_ahead IS NULL OR p_months_ahead NOT BETWEEN 0 AND 12 THEN + RAISE EXCEPTION 'months ahead must be between 0 and 12, got %', p_months_ahead + USING ERRCODE = 'invalid_parameter_value'; + END IF; + FOR i IN 0..p_months_ahead LOOP + month_start := date_trunc('month', p_now AT TIME ZONE 'UTC') + make_interval(months => i); + EXECUTE format( + 'CREATE TABLE IF NOT EXISTS core.%I PARTITION OF core.webhook_events FOR VALUES FROM (%L) TO (%L)', + 'webhook_events_' || to_char(month_start, 'YYYY_MM'), + month_start AT TIME ZONE 'UTC', + (month_start + INTERVAL '1 month') AT TIME ZONE 'UTC'); + END LOOP; +END +$$; +-- +goose StatementEnd + +ALTER FUNCTION core.ensure_webhook_event_partitions(TIMESTAMPTZ, INTEGER) OWNER TO core_owner; +REVOKE EXECUTE ON FUNCTION core.ensure_webhook_event_partitions(TIMESTAMPTZ, INTEGER) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION core.ensure_webhook_event_partitions(TIMESTAMPTZ, INTEGER) TO console_runtime; + +-- +goose Down +DROP FUNCTION core.ensure_webhook_event_partitions(TIMESTAMPTZ, INTEGER); + +-- +goose StatementBegin +DO $$ +DECLARE + rel RECORD; +BEGIN + ALTER TABLE core.webhook_events OWNER TO CURRENT_USER; + GRANT ALL ON core.webhook_events TO core_owner; + FOR rel IN + SELECT n.nspname AS schema_name, c.relname AS table_name + FROM pg_catalog.pg_inherits i + JOIN pg_catalog.pg_class c ON c.oid = i.inhrelid + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + WHERE i.inhparent = 'core.webhook_events'::pg_catalog.regclass + LOOP + EXECUTE pg_catalog.format('ALTER TABLE %I.%I OWNER TO CURRENT_USER', rel.schema_name, rel.table_name); + EXECUTE pg_catalog.format('GRANT ALL ON %I.%I TO core_owner', rel.schema_name, rel.table_name); + END LOOP; +END $$; +-- +goose StatementEnd diff --git a/internal/db/migrations/00032_lock_plan_act_obligation.sql b/internal/db/migrations/00032_lock_plan_act_obligation.sql new file mode 100644 index 00000000..24871b80 --- /dev/null +++ b/internal/db/migrations/00032_lock_plan_act_obligation.sql @@ -0,0 +1,45 @@ +-- SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial +-- SPDX-FileCopyrightText: 2025-2026 Christian Galo + +-- +goose Up +-- --------------------------------------------------------------------------- +-- The plan-act obligation lock goes through a function (openspec change +-- runtime-database-user, design D19). EnactObligation locks the obligation of +-- one (act, pool) after the pool row, to read in one statement whether the act +-- still owns the obligation's org type and who put it in motion. Postgres +-- requires UPDATE on a table for a locking read, and the enclosure (00028) +-- withholds UPDATE from core_writer so that every status change goes through +-- a function. This SECURITY DEFINER function owned by core_owner takes the +-- lock for the caller; it writes nothing. +-- +-- The function reads only the obligation row, as the query it replaces did: a +-- join would let Postgres's recheck of a row that a Retry requeued while the +-- statement waited drop the row. The lock, its mode (FOR UPDATE) and its order +-- (the pool row, then the obligation row) are those of the query it replaces. +-- A function with a SET clause is never inlined, so the definer's privileges +-- apply. No row is returned for a missing obligation. +-- --------------------------------------------------------------------------- + +-- +goose StatementBegin +CREATE FUNCTION core.lock_plan_act_obligation(p_act_id UUID, p_pool_id UUID) +RETURNS TABLE (act_id UUID, pool_id UUID, org_type VARCHAR, org_id UUID, status VARCHAR, + owned BOOLEAN, motion_act_id UUID) +LANGUAGE sql +SECURITY DEFINER +SET search_path = core, pg_temp +AS $$ + SELECT ob.act_id, ob.pool_id, ob.org_type, ob.org_id, ob.status, + core.plan_act_owns_org_type(ob.act_id, ob.org_type)::boolean, + COALESCE(ob.requeued_by_act_id, ob.act_id)::uuid + FROM core.plan_act_obligations ob + WHERE ob.act_id = p_act_id AND ob.pool_id = p_pool_id + FOR UPDATE OF ob; +$$; +-- +goose StatementEnd + +ALTER FUNCTION core.lock_plan_act_obligation(UUID, UUID) OWNER TO core_owner; +REVOKE EXECUTE ON FUNCTION core.lock_plan_act_obligation(UUID, UUID) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION core.lock_plan_act_obligation(UUID, UUID) TO core_writer; + +-- +goose Down +DROP FUNCTION core.lock_plan_act_obligation(UUID, UUID); diff --git a/internal/db/partitions.go b/internal/db/partitions.go index b4d32ba2..1347910b 100644 --- a/internal/db/partitions.go +++ b/internal/db/partitions.go @@ -13,7 +13,7 @@ import ( // DefaultWebhookPartitionMonthsAhead is the default lookahead window for // EnsureWebhookEventPartitions: the current month plus this many months // ahead. One more than the baseline migration's initial 0..2 loop -// (migrations/00001_init.sql:1054), so a fresh install immediately gains +// (the initial-partition block of migrations/00001_init.sql), so a fresh install immediately gains // headroom (design.md D3). const DefaultWebhookPartitionMonthsAhead = 3 @@ -22,48 +22,29 @@ const DefaultWebhookPartitionMonthsAhead = 3 // plus monthsAhead (inclusive) — e.g. monthsAhead=3 ensures four partitions: // the current month and the next three. // -// It mirrors the baseline migration's partition-creation DO block -// (migrations/00001_init.sql:1048-1064) exactly: the same -// core.webhook_events_YYYY_MM naming (zero-padded four-digit year, two-digit -// month) and the same month-start/next-month-start boundaries. That parity -// is load-bearing — partitions created here must never overlap the ones the -// migration creates, whichever runs first — so any change to the naming or -// boundary computation here MUST be mirrored back into that DO block, and -// vice versa. +// The partitions are created by core.ensure_webhook_event_partitions +// (migrations/00031_webhook_partition_function.sql), a SECURITY DEFINER +// function owned by core_owner, because the runtime user owns nothing and +// creating a partition requires the owner of the parent table. The function +// refuses a null time and a window outside 0 to 12 months with SQLSTATE 22023. // -// Idempotent via CREATE TABLE IF NOT EXISTS: running it twice, or racing the -// migration's own initial-partition creation, is safe. A concurrent-create -// race between two runners surfaces as an error to whichever loses; the -// caller logs it and the next run (boot pass or scheduled workflow) succeeds -// (design.md D3, Risks). -func EnsureWebhookEventPartitions(ctx context.Context, database *sql.DB, now time.Time, monthsAhead int) error { - monthStart := time.Date(now.UTC().Year(), now.UTC().Month(), 1, 0, 0, 0, 0, time.UTC) - - for i := 0; i <= monthsAhead; i++ { - start := monthStart.AddDate(0, i, 0) - end := start.AddDate(0, 1, 0) - - // The partition name and the boundary literals are composed entirely - // from computed integers (never user input), so building the DDL - // string with fmt.Sprintf is safe — mirroring the migration's own - // EXECUTE FORMAT('... %s ... %L ... %L', part, m_start, m_end) - // approach exactly, which relies on the same property. Boundaries are - // embedded as literals rather than query parameters deliberately: pgx - // against PG18 rejects placeholders inside this DDL shape ("mismatched - // param and argument count") — CREATE TABLE ... PARTITION OF ... - // FOR VALUES FROM/TO isn't a statement pgx's extended protocol can - // describe placeholders for, the same family of pgx/PG18 DDL - // interaction ConnectPlain's doc comment (database.go) already works - // around for multi-statement migrations. - partition := fmt.Sprintf("core.webhook_events_%04d_%02d", start.Year(), int(start.Month())) - stmt := fmt.Sprintf( - `CREATE TABLE IF NOT EXISTS %s PARTITION OF core.webhook_events FOR VALUES FROM ('%s') TO ('%s')`, - partition, start.Format(time.RFC3339), end.Format(time.RFC3339), - ) - if _, err := database.ExecContext(ctx, stmt); err != nil { - return fmt.Errorf("ensure webhook_events partition %s: %w", partition, err) - } +// The naming (core.webhook_events_YYYY_MM, after the UTC month) and the bounds +// (that month's first instant to the next month's, in UTC) are the same in the +// function and in the baseline's initial-partition DO block +// (migrations/00001_init.sql). That parity is load-bearing — partitions +// created by one must never overlap the ones the other creates — so a change +// to the naming or the bounds in either MUST be mirrored in the other. +// +// Idempotent via CREATE TABLE IF NOT EXISTS inside the function: running it +// twice, or racing the migration's own initial-partition creation, is safe. A +// concurrent-create race between two runners surfaces as an error to whichever +// loses; the caller logs it and the next run (boot pass or scheduled workflow) +// succeeds (design.md D3, Risks). +func EnsureWebhookEventPartitions(ctx context.Context, ex interface { + ExecContext(ctx context.Context, query string, args ...any) (sql.Result, error) +}, now time.Time, monthsAhead int) error { + if _, err := ex.ExecContext(ctx, `SELECT core.ensure_webhook_event_partitions($1, $2)`, now, monthsAhead); err != nil { + return fmt.Errorf("ensure webhook_events partitions: %w", err) } - return nil } diff --git a/internal/db/partitions_test.go b/internal/db/partitions_test.go index de32612a..30d7ea42 100644 --- a/internal/db/partitions_test.go +++ b/internal/db/partitions_test.go @@ -3,16 +3,19 @@ package db_test -// DB-backed idempotency test for EnsureWebhookEventPartitions -// (webhook-partition-maintenance, design.md D3/D4). Runs against -// TEST_DATABASE_URL under the same member_console role the running app and -// its boot pass use (D4: per-schema roles are in force, and this proves the -// role can CREATE in schema core rather than assuming it). +// Tests of the webhook partition function and of the ownership of +// core.webhook_events (openspec change runtime-database-user, design D3 and +// D4, tasks 3.4 to 3.6). Who runs what: the gap, the refusal and the +// direct-DDL tests run as console_runtime, through SET LOCAL ROLE inside a +// transaction they roll back, on the superuser TEST_DATABASE_URL login; the +// ownership test reads the catalog as that login; the round-trip and the +// baseline tests migrate scratch databases as that login. import ( "context" "database/sql" "fmt" + "slices" "strings" "testing" "time" @@ -21,142 +24,293 @@ import ( "git.coopcloud.tech/wiki-cafe/member-console/internal/testkit/dbtest" ) -// partitionTableName mirrors EnsureWebhookEventPartitions' and the baseline -// migration's naming exactly (core.webhook_events_YYYY_MM). +const ( + partitionFunction = "core.ensure_webhook_event_partitions" + // partitionTimeZone is a zone whose month starts 14 hours before UTC's, so + // a function that reads the session's zone names other months than UTC's. + partitionTimeZone = "Pacific/Kiritimati" + + insertWebhookEventSQL = `INSERT INTO core.webhook_events (provider, provider_event_id, event_type, received_at) + VALUES ('test-provider', $1, 'test.event', $2)` + listPartitionsSQL = `SELECT n.nspname || '.' || c.relname + FROM pg_inherits i + JOIN pg_class c ON c.oid = i.inhrelid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE i.inhparent = 'core.webhook_events'::regclass + ORDER BY 1` +) + +// partitionTableName mirrors the function's and the baseline migration's +// naming (core.webhook_events_YYYY_MM). func partitionTableName(monthStart time.Time) string { return fmt.Sprintf("core.webhook_events_%04d_%02d", monthStart.Year(), int(monthStart.Month())) } -// insertWebhookEvent inserts a minimal row timestamped at receivedAt, -// returning whatever error the partitioned table's routing produces (nil on -// success). -func insertWebhookEvent(ctx context.Context, database *sql.DB, providerEventID string, receivedAt time.Time) error { - _, err := database.ExecContext(ctx, ` - INSERT INTO core.webhook_events (provider, provider_event_id, event_type, received_at) - VALUES ('test-provider', $1, 'test.event', $2)`, - providerEventID, receivedAt) - return err +// partitionNames lists the partitions of core.webhook_events. +func partitionNames(q *dbtest.Q) []string { return q.Strings(listPartitionsSQL) } + +// relationOwner returns the owner of a schema-qualified relation. +func relationOwner(q *dbtest.Q, relation string) string { + return q.String(`SELECT pg_get_userbyid(relowner) FROM pg_class WHERE oid = $1::regclass`, relation) } -// tableExists reports whether the given core. table exists. -func tableExists(t *testing.T, ctx context.Context, database *sql.DB, name string) bool { +// runtimeTx begins a transaction the test rolls back, as console_runtime, in a +// session whose zone is not UTC. +func runtimeTx(t *testing.T) *dbtest.Q { t.Helper() - var exists bool - err := database.QueryRowContext(ctx, ` - SELECT EXISTS ( - SELECT 1 FROM information_schema.tables - WHERE table_schema = 'core' AND table_name = $1 - )`, name).Scan(&exists) - if err != nil { - t.Fatalf("check table existence for %s: %v", name, err) - } - return exists + q := dbtest.Tx(t, dbtest.Open(t)) + q.Exec("SET LOCAL ROLE " + db.RuntimeGroupRole) + q.Exec("SET LOCAL TimeZone = '" + partitionTimeZone + "'") + return q } -// TestEnsureWebhookEventPartitions_ClosesTheGapAndIsIdempotent proves the -// bug (an insert into an unpartitioned-for month fails), that Ensure closes -// it, and that a second Ensure run is a no-op — the three scenarios in -// openspec/changes/purchase-path-blockers/specs/webhook-partition-maintenance/spec.md. +// requirePartitionExists fails for a partition missing from the catalog. +func requirePartitionExists(q *dbtest.Q, t *testing.T, name string) { + t.Helper() + if q.String(`SELECT coalesce(to_regclass($1)::text, '')`, name) == "" { + t.Errorf("expected partition %s to exist after the ensure, it does not", name) + } +} + +// requireRowInPartition inserts a row at receivedAt and requires it to land in +// the partition. +func requireRowInPartition(q *dbtest.Q, t *testing.T, partition, eventID string, receivedAt time.Time) { + t.Helper() + q.Exec(insertWebhookEventSQL, eventID, receivedAt) + if n := q.Int(`SELECT count(*) FROM core.webhook_events WHERE provider_event_id = $1 AND tableoid = $2::regclass`, eventID, partition); n != 1 { + t.Errorf("the row at %s landed %d times in %s, want 1", receivedAt.UTC().Format(time.RFC3339), n, partition) + } +} + +// TestEnsureWebhookEventPartitions_ClosesTheGapAndIsIdempotent proves the gap +// (an insert into a month with no partition fails), that the ensure closes it +// as the runtime role in a session whose zone is not UTC, that the partitions +// belong to core_owner and that a second ensure creates nothing. The month is +// one no real run reaches, and the transaction rolls back, so nothing is left +// to drop. func TestEnsureWebhookEventPartitions_ClosesTheGapAndIsIdempotent(t *testing.T) { - database := dbtest.Open(t) + q := runtimeTx(t) ctx := context.Background() + tx := q.Tx() - // A synthetic "now" far outside any real calendar month a concurrently - // running test suite, or a prior un-reset run of this same test, could - // have already ensured partitions for — the shared TEST_DATABASE_URL - // database is reset between `make test` runs but not between bare - // `go test` reruns, and this test must be able to prove "fails before, - // succeeds after" regardless of that history. - now := time.Date(2119, time.June, 15, 12, 0, 0, 0, time.UTC) + // The last day of December, midday in UTC, is already January of the next + // year in the session's zone, and the window crosses the year. + now := time.Date(2119, time.December, 31, 12, 0, 0, 0, time.UTC) monthStart := time.Date(now.Year(), now.Month(), 1, 0, 0, 0, 0, time.UTC) - - monthsAhead := db.DefaultWebhookPartitionMonthsAhead // 3 + monthsAhead := db.DefaultWebhookPartitionMonthsAhead targetMonth := monthStart.AddDate(0, monthsAhead, 0) targetPartition := partitionTableName(targetMonth) - // Postgres table name only, without the "core." schema qualifier, for - // information_schema lookups. - targetPartitionUnqualified := strings.TrimPrefix(targetPartition, "core.") - // Guarantee a clean slate for the "before" assertion: drop the target - // partition (and, defensively, the whole synthetic month range) if a - // previous unreset run left it behind. - dropSyntheticPartitions := func() { - for i := 0; i <= monthsAhead; i++ { - name := partitionTableName(monthStart.AddDate(0, i, 0)) - if _, err := database.ExecContext(ctx, "DROP TABLE IF EXISTS "+name); err != nil { - t.Fatalf("drop synthetic partition %s: %v", name, err) - } - } - } - dropSyntheticPartitions() - t.Cleanup(dropSyntheticPartitions) + q.Refuse(dbtest.SQLStateCheck, "", insertWebhookEventSQL, "before-ensure", targetMonth.Add(time.Hour)) - // --- "Missing future partitions" scenario, negative half: before Ensure - // runs, the target month (now + monthsAhead) has no partition, so the - // insert fails with Postgres' partition-routing error. This is the bug - // this change fixes: nothing but the baseline migration's initial - // current+2 loop had ever created partitions. - if err := insertWebhookEvent(ctx, database, "before-ensure", targetMonth.Add(time.Hour)); err == nil { - t.Fatalf("insert into %s succeeded before Ensure ran; the partition gap this change fixes was not reproduced", targetPartition) - } else if !strings.Contains(strings.ToLower(err.Error()), "partition") { - t.Fatalf("insert into %s failed with an unexpected error (want a partition-routing error): %v", targetPartition, err) - } - if tableExists(t, ctx, database, targetPartitionUnqualified) { - t.Fatalf("partition %s already exists before Ensure ran", targetPartition) - } - - // --- Run Ensure: creates the current synthetic month through +monthsAhead. - if err := db.EnsureWebhookEventPartitions(ctx, database, now, monthsAhead); err != nil { + if err := db.EnsureWebhookEventPartitions(ctx, tx, now, monthsAhead); err != nil { t.Fatalf("EnsureWebhookEventPartitions (first run): %v", err) } - for i := 0; i <= monthsAhead; i++ { name := partitionTableName(monthStart.AddDate(0, i, 0)) - unqualified := strings.TrimPrefix(name, "core.") - if !tableExists(t, ctx, database, unqualified) { - t.Errorf("expected partition %s to exist after Ensure, it does not", name) + requirePartitionExists(q, t, name) + if owner := relationOwner(q, name); owner != "core_owner" { + t.Errorf("partition %s is owned by %s, want core_owner", name, owner) } } - // --- "Missing future partitions" scenario, positive half: the insert - // that failed above now succeeds, in the correct partition. - if err := insertWebhookEvent(ctx, database, "after-ensure", targetMonth.Add(time.Hour)); err != nil { - t.Fatalf("insert into %s failed after Ensure ran: %v", targetPartition, err) - } - var rowCount int - if err := database.QueryRowContext(ctx, "SELECT count(*) FROM "+targetPartition+" WHERE provider_event_id = 'after-ensure'"). - Scan(&rowCount); err != nil { - t.Fatalf("count rows landed in %s: %v", targetPartition, err) - } - if rowCount != 1 { - t.Errorf("row landed in %s = %d, want 1 (the row must be routed to the correct monthly partition)", targetPartition, rowCount) - } + requireRowInPartition(q, t, targetPartition, "first-hour", targetMonth.Add(time.Hour)) + requireRowInPartition(q, t, targetPartition, "last-hour", targetMonth.AddDate(0, 1, 0).Add(-time.Hour)) - // --- "Ensure is idempotent" scenario: a second run against the same - // `now` succeeds and creates nothing new. - var tableCountBefore int - if err := database.QueryRowContext(ctx, ` - SELECT count(*) FROM information_schema.tables - WHERE table_schema = 'core' AND table_name LIKE 'webhook_events_%'`).Scan(&tableCountBefore); err != nil { - t.Fatalf("count webhook_events partitions before second Ensure: %v", err) - } - - if err := db.EnsureWebhookEventPartitions(ctx, database, now, monthsAhead); err != nil { + before := partitionNames(q) + if err := db.EnsureWebhookEventPartitions(ctx, tx, now, monthsAhead); err != nil { t.Fatalf("EnsureWebhookEventPartitions (second run): %v", err) } + if after := partitionNames(q); !slices.Equal(before, after) { + t.Errorf("the second ensure changed the partitions: %v -> %v, want none", before, after) + } +} + +// functionFacts are the catalog facts the function's tests read. +type functionFacts struct { + secdef bool + owner string + config string + public bool +} + +func readFunctionFacts(q *dbtest.Q, qualified string) functionFacts { + var f functionFacts + q.Scan([]any{&f.secdef, &f.owner, &f.config, &f.public}, `SELECT p.prosecdef, pg_get_userbyid(p.proowner), + coalesce(array_to_string(p.proconfig, ','), ''), has_function_privilege('public', p.oid, 'EXECUTE') + FROM pg_proc p WHERE p.oid = $1::regproc`, qualified) + return f +} + +// TestWebhookPartitionsBelongToCoreOwner asserts the owner of the parent and of +// every partition, and the definer function's owner, security, search path and +// refusal of PUBLIC. +func TestWebhookPartitionsBelongToCoreOwner(t *testing.T) { + q := dbtest.On(t, dbtest.Open(t)) + + if owner := relationOwner(q, "core.webhook_events"); owner != "core_owner" { + t.Errorf("core.webhook_events is owned by %s, want core_owner", owner) + } + partitions := partitionNames(q) + if len(partitions) == 0 { + t.Fatal("core.webhook_events has no partition") + } + for _, name := range partitions { + if owner := relationOwner(q, name); owner != "core_owner" { + t.Errorf("partition %s is owned by %s, want core_owner", name, owner) + } + } + + f := readFunctionFacts(q, partitionFunction) + if !f.secdef || f.owner != "core_owner" || !strings.Contains(f.config, "search_path=core, pg_temp") || f.public { + t.Errorf("%s: security definer %v, owner %s, config %q, public may execute %v; want a definer owned by core_owner with search_path core, pg_temp and no PUBLIC execute", + partitionFunction, f.secdef, f.owner, f.config, f.public) + } +} + +// TestRuntimeRoleCannotCreatePartitionsDirectly runs the partition DDL the Go +// code ran before the function, as the runtime role, which owns nothing. +func TestRuntimeRoleCannotCreatePartitionsDirectly(t *testing.T) { + q := dbtest.Tx(t, dbtest.Open(t)) + q.Exec("SET LOCAL ROLE " + db.RuntimeGroupRole) + q.Refuse(dbtest.SQLStatePrivilege, "", `CREATE TABLE core.webhook_events_2119_06 PARTITION OF core.webhook_events + FOR VALUES FROM ('2119-06-01 00:00:00+00') TO ('2119-07-01 00:00:00+00')`) +} + +// refuseWindow calls the function with a window it refuses and requires +// SQLSTATE 22023 and no new partition. +func refuseWindow(q *dbtest.Q, t *testing.T, now any, monthsAhead any) { + t.Helper() + before := partitionNames(q) + q.Refuse(dbtest.SQLStateInvalidParameter, "", + `SELECT core.ensure_webhook_event_partitions($1::timestamptz, $2::integer)`, now, monthsAhead) + if after := partitionNames(q); !slices.Equal(before, after) { + t.Errorf("a refused window (%v, %v) changed the partitions: %v -> %v", now, monthsAhead, before, after) + } +} + +func TestEnsureWebhookEventPartitionsRefusesAWindow(t *testing.T) { + now := time.Date(2119, time.June, 15, 12, 0, 0, 0, time.UTC) + rows := []struct { + name string + now any + monthsAhead any + }{ + {"13 months", now, 13}, + {"-1 months", now, -1}, + {"a null time", nil, 3}, + } + for _, row := range rows { + t.Run(row.name, func(t *testing.T) { + refuseWindow(runtimeTx(t), t, row.now, row.monthsAhead) + }) + } +} - var tableCountAfter int - if err := database.QueryRowContext(ctx, ` - SELECT count(*) FROM information_schema.tables - WHERE table_schema = 'core' AND table_name LIKE 'webhook_events_%'`).Scan(&tableCountAfter); err != nil { - t.Fatalf("count webhook_events partitions after second Ensure: %v", err) +// TestWebhookOwnershipMoveRoundTrips runs 00031 up, down and up again on a +// scratch database. The Down gives the parent and every partition back to the +// user that ran it and re-grants core_owner what the baseline gave it, so the +// snapshot (owners and ACLs included) after the Down equals the one before. +func TestWebhookOwnershipMoveRoundTrips(t *testing.T) { + scratch := dbtest.Connect(t, dbtest.Scratch(t)) + ctx := t.Context() + core := db.BaseSources()[0] + + dbtest.UpTo(t, scratch, core, 30) + before := schemaSnapshot(t, ctx, scratch) + + dbtest.ApplyVersion(t, scratch, core, 31, true) + afterUp := schemaSnapshot(t, ctx, scratch) + if slices.Equal(before, afterUp) { + t.Fatal("applying 31 changed nothing in the snapshot") + } + + dbtest.ApplyVersion(t, scratch, core, 31, false) + if afterDown := schemaSnapshot(t, ctx, scratch); !slices.Equal(before, afterDown) { + t.Errorf("the schema after Down differs from the schema before Up:\n%s", snapshotDiff(before, afterDown)) + } + + dbtest.ApplyVersion(t, scratch, core, 31, true) + if again := schemaSnapshot(t, ctx, scratch); !slices.Equal(afterUp, again) { + t.Errorf("the schema after a second Up differs from the first:\n%s", snapshotDiff(afterUp, again)) + } +} + +// utcMonthStarts returns the first instant of the UTC months from the current +// one on, as UTC times. +func utcMonthStarts(now time.Time, n int) []time.Time { + first := time.Date(now.UTC().Year(), now.UTC().Month(), 1, 0, 0, 0, 0, time.UTC) + out := make([]time.Time, n) + for i := range out { + out[i] = first.AddDate(0, i, 0) } - if tableCountAfter != tableCountBefore { - t.Errorf("second Ensure run changed the partition count: %d -> %d, want no-op", tableCountBefore, tableCountAfter) + return out +} + +// partitionBounds reads each partition's bounds as timestamptz, by name. +func partitionBounds(t *testing.T, database *sql.DB) map[string][2]time.Time { + t.Helper() + rows, err := database.QueryContext(t.Context(), ` + SELECT c.relname, + (regexp_match(pg_get_expr(c.relpartbound, c.oid), 'FROM \(''([^'']+)''\) TO \(''([^'']+)''\)'))[1]::timestamptz, + (regexp_match(pg_get_expr(c.relpartbound, c.oid), 'FROM \(''([^'']+)''\) TO \(''([^'']+)''\)'))[2]::timestamptz + FROM pg_inherits i JOIN pg_class c ON c.oid = i.inhrelid + WHERE i.inhparent = 'core.webhook_events'::regclass`) + if err != nil { + t.Fatalf("read the partition bounds: %v", err) + } + defer rows.Close() + out := map[string][2]time.Time{} + for rows.Next() { + var name string + var from, to time.Time + if err := rows.Scan(&name, &from, &to); err != nil { + t.Fatalf("scan a partition: %v", err) + } + out[name] = [2]time.Time{from.UTC(), to.UTC()} + } + if err := rows.Err(); err != nil { + t.Fatalf("read the partitions: %v", err) + } + return out +} + +// TestBaselinePartitionBoundsAreUTC applies core version 1 on a connection +// whose zone is not UTC and requires the three initial partitions to be the +// UTC months, bounded by their UTC first instants. +func TestBaselinePartitionBoundsAreUTC(t *testing.T) { + dsn := dbtest.Scratch(t) + sep := "?" + if strings.Contains(dsn, "?") { + sep = "&" + } + database := dbtest.Connect(t, dsn+sep+"TimeZone="+partitionTimeZone) + if zone := dbtest.On(t, database).String(`SHOW TimeZone`); zone != partitionTimeZone { + t.Fatalf("the connection's TimeZone is %q, want %q", zone, partitionTimeZone) } + before := time.Now() + dbtest.UpTo(t, database, db.BaseSources()[0], 1) + after := time.Now() + // The migration reads NOW() inside its own run; a run that crosses a UTC + // month start between the two readings has no month to compare. + if !utcMonthStarts(before, 1)[0].Equal(utcMonthStarts(after, 1)[0]) { + t.Skipf("the run crossed a UTC month start (%v to %v)", before.UTC(), after.UTC()) + } - // The insert-succeeds behavior still holds after the no-op second run. - if err := insertWebhookEvent(ctx, database, "after-second-ensure", targetMonth.Add(2*time.Hour)); err != nil { - t.Fatalf("insert into %s failed after second Ensure run: %v", targetPartition, err) + got := partitionBounds(t, database) + months := utcMonthStarts(after, 3) + if len(got) != len(months) { + t.Fatalf("the baseline created %d partitions, want %d: %v", len(got), len(months), got) + } + for _, start := range months { + name := strings.TrimPrefix(partitionTableName(start), "core.") + bounds, ok := got[name] + if !ok { + t.Errorf("no partition %s; have %v", name, got) + continue + } + if end := start.AddDate(0, 1, 0); !bounds[0].Equal(start) || !bounds[1].Equal(end) { + t.Errorf("partition %s is bounded %v to %v, want %v to %v", name, bounds[0], bounds[1], start, end) + } } } diff --git a/internal/db/plan_act_obligations_test.go b/internal/db/plan_act_obligations_test.go index 7835cdd7..a0067bf3 100644 --- a/internal/db/plan_act_obligations_test.go +++ b/internal/db/plan_act_obligations_test.go @@ -34,6 +34,7 @@ const ( settlePlanActObligation = `SELECT core.settle_plan_act_obligation($1::uuid, $2::uuid, 'initiated')` recordObligationFailure = `SELECT core.record_plan_act_obligation_failure($1::uuid, $2::uuid, 'begin', 'boom')` requeuePlanActObligates = `SELECT core.requeue_plan_act_obligations($1::uuid)` + lockPlanActObligation = `SELECT count(*) FROM core.lock_plan_act_obligation($1::uuid, $2::uuid)` ) // obligationFixture is the rows the obligation tests share: a person to act, a @@ -283,7 +284,7 @@ func TestPlanActWritersAreEnclosedInTheCatalog(t *testing.T) { f.obligation(act, pool, orgType, org) writers := []string{"record_plan_act", "write_plan_act_obligations", "settle_plan_act_obligation", - "record_plan_act_obligation_failure", "requeue_plan_act_obligations"} + "record_plan_act_obligation_failure", "requeue_plan_act_obligations", "lock_plan_act_obligation"} for _, name := range writers { var secdef bool var owner, config string @@ -295,6 +296,12 @@ func TestPlanActWritersAreEnclosedInTheCatalog(t *testing.T) { t.Errorf("%s: security definer %v, owner %s, config %q, public may execute %v; want a definer owned by core_owner with search_path core, pg_temp and no PUBLIC execute", name, secdef, owner, config, public) } + for _, role := range []string{"core_writer", db.RuntimeGroupRole} { + if q.String(`SELECT has_function_privilege($1, p.oid, 'EXECUTE')::text FROM pg_proc p + WHERE p.pronamespace = 'core'::regnamespace AND p.proname = $2`, role, name) != "true" { + t.Errorf("%s cannot execute %s", role, name) + } + } } bare := dbtest.Name("obligations_bare_") @@ -307,6 +314,7 @@ func TestPlanActWritersAreEnclosedInTheCatalog(t *testing.T) { q.Refuse(dbtest.SQLStatePrivilege, "", recordObligationFailure, act, pool) q.Refuse(dbtest.SQLStatePrivilege, "", requeuePlanActObligates, act) q.Refuse(dbtest.SQLStatePrivilege, "", recordTierRemoved, f.person, f.ladder, f.product) + q.Refuse(dbtest.SQLStatePrivilege, "", lockPlanActObligation, act, pool) }) } // The ownership rule runs with the caller's rights: a reader reads it. diff --git a/internal/db/runtime_role_privileges_test.go b/internal/db/runtime_role_privileges_test.go index 60a24297..118449b9 100644 --- a/internal/db/runtime_role_privileges_test.go +++ b/internal/db/runtime_role_privileges_test.go @@ -215,6 +215,23 @@ func checkNoLoginReachesAWriter(q *dbtest.Q, t *testing.T) { } } +// runtimeFunctions are the definer functions whose EXECUTE the group role holds +// by a grant of its own or through core_writer. +var runtimeFunctions = []string{ + "core.ensure_webhook_event_partitions(timestamptz, integer)", + "core.lock_plan_act_obligation(uuid, uuid)", +} + +func checkFunctionsExecutable(q *dbtest.Q, t *testing.T) { + t.Helper() + for _, fn := range runtimeFunctions { + got := q.String(`SELECT has_function_privilege($1, $2::regprocedure, 'EXECUTE')::text`, db.RuntimeGroupRole, fn) + if got != "true" { + t.Errorf("%s cannot EXECUTE %s", db.RuntimeGroupRole, fn) + } + } +} + func TestRuntimeRolePrivileges(t *testing.T) { q := dbtest.On(t, dbtest.Open(t)) rels := schemaRelations(q) @@ -227,5 +244,6 @@ func TestRuntimeRolePrivileges(t *testing.T) { checkOutboxColumns(q, t) checkNoExtraPrivileges(q, t, rels) checkLedgersReadable(q, t) + checkFunctionsExecutable(q, t) checkNoLoginReachesAWriter(q, t) } diff --git a/internal/db/runtime_user.go b/internal/db/runtime_user.go index 7162a8fd..b9ce66be 100644 --- a/internal/db/runtime_user.go +++ b/internal/db/runtime_user.go @@ -5,7 +5,8 @@ package db // RuntimeGroupRole is the NOLOGIN group role the console's runtime user // holds. Core migration 00029 creates it and grants it core_writer and SELECT -// on the core goose ledger; the group-role migration of each other stream +// on the core goose ledger, and core 00031 grants it EXECUTE on +// core.ensure_webhook_event_partitions; the group-role migration of each other stream // (domains 00003, FedWiki 00005, Stripe 00006, Discourse 00004) grants it that // stream's writer and ledger. No migration grants it to a login: the console // does that at start. diff --git a/internal/entitlements/plan_act_queries_db_test.go b/internal/entitlements/plan_act_queries_db_test.go index d12779b2..3528c24b 100644 --- a/internal/entitlements/plan_act_queries_db_test.go +++ b/internal/entitlements/plan_act_queries_db_test.go @@ -22,6 +22,7 @@ import ( "github.com/google/uuid" + "git.coopcloud.tech/wiki-cafe/member-console/internal/db" "git.coopcloud.tech/wiki-cafe/member-console/internal/entitlements" "git.coopcloud.tech/wiki-cafe/member-console/internal/organization" "git.coopcloud.tech/wiki-cafe/member-console/internal/testkit/dbtest" @@ -295,6 +296,46 @@ func TestLockPlanActObligationReadsTheMove(t *testing.T) { } } +// The runtime role holds no UPDATE on the obligations, and a locking read needs +// it, so the move locks the obligation through core.lock_plan_act_obligation +// (openspec change runtime-database-user, design D19). The acts and their +// obligations are written as console_runtime too, through the definer +// functions, so the whole move is the one the served console runs. The bare +// locking statement fails with 42501, which is the reason for the function. +func TestLockPlanActObligationRunsAsTheRuntimeRole(t *testing.T) { + r := newPlanActRows(t) + ctx := t.Context() + orgType := r.orgType() + org, pool := r.org(orgType, "Alpha", true) + r.q.Exec("SET LOCAL ROLE " + db.RuntimeGroupRole) + + act := r.defaultChange(orgType) + r.write(act, 10) + lock, err := r.e.LockPlanActObligation(ctx, entitlements.LockPlanActObligationParams{ActID: act, PoolID: pool}) + if err != nil { + t.Fatalf("lock the obligation as %s: %v", db.RuntimeGroupRole, err) + } + if !lock.Owned || lock.Status != "pending" || lock.OrgID != org || lock.OrgType != orgType || lock.MotionActID != act { + t.Errorf("the locked obligation reads %+v", lock) + } + + r.fail(act, pool) + retry := r.retry(act) + if n, err := r.e.RequeuePlanActObligations(ctx, retry); err != nil || n != 1 { + t.Fatalf("requeue: %d, %v", n, err) + } + lock, err = r.e.LockPlanActObligation(ctx, entitlements.LockPlanActObligationParams{ActID: act, PoolID: pool}) + if err != nil || lock.MotionActID != retry || lock.Status != "pending" { + t.Errorf("the requeued obligation reads %+v (%v), want pending and moved by %s", lock, err, retry) + } + + if _, err := r.e.LockPlanActObligation(ctx, entitlements.LockPlanActObligationParams{ActID: act, PoolID: uuid.NewString()}); !errors.Is(err, sql.ErrNoRows) { + t.Errorf("locking a pool with no obligation returned %v, want sql.ErrNoRows", err) + } + + r.q.Refuse(dbtest.SQLStatePrivilege, "", `SELECT 1 FROM core.plan_act_obligations WHERE false FOR UPDATE`) +} + // A move that waits for the obligation's lock while a Retry requeues the row // gets the row once the requeue commits: the statement reads only the // obligation, so PostgreSQL's recheck of the updated row has no join to drop @@ -502,8 +543,10 @@ func TestPlanActOwnerRuleIsStatedOnce(t *testing.T) { texts := queryTexts(t) // The queries that read obligations, the reached types or the records of // organizations without a pool through ownership. + // LockPlanActObligation reads through core.lock_plan_act_obligation, which + // states the rule itself (checked with the functions below). for _, name := range []string{ - "LockPlanActObligation", "ListPendingPlanActObligations", "ListPlanActsOwing", + "ListPendingPlanActObligations", "ListPlanActsOwing", "ListPlanActReachedOrgTypes", "CountPlanActOrgsWithoutPool", "ListOpenPlanActsByOrgTypes", "ListOpenPlanActsByLadder", "GetPlanActOwed", "ListUnmovedOrganizations", } { @@ -525,9 +568,10 @@ func TestPlanActOwnerRuleIsStatedOnce(t *testing.T) { } // The functions that supersede and requeue read obligations the same way, - // and so does the frontier the listing and its readers share. + // and so does the frontier the listing and its readers share, and the lock a + // move takes. q := dbtest.On(t, dbtest.Open(t)) - for _, name := range []string{"write_plan_act_obligations", "requeue_plan_act_obligations", "plan_act_listing_frontier"} { + for _, name := range []string{"write_plan_act_obligations", "requeue_plan_act_obligations", "plan_act_listing_frontier", "lock_plan_act_obligation"} { def := q.String(`SELECT pg_get_functiondef(p.oid) FROM pg_proc p WHERE p.pronamespace = 'core'::regnamespace AND p.proname = $1`, name) if !strings.Contains(def, ownerRule) { t.Errorf("core.%s does not call %s", name, ownerRule) diff --git a/internal/entitlements/plan_acts.sql.go b/internal/entitlements/plan_acts.sql.go index 947c48e1..be8e4263 100644 --- a/internal/entitlements/plan_acts.sql.go +++ b/internal/entitlements/plan_acts.sql.go @@ -537,12 +537,11 @@ func (q *Queries) ListUnmovedOrganizations(ctx context.Context, arg ListUnmovedO } const lockPlanActObligation = `-- name: LockPlanActObligation :one -SELECT ob.act_id, ob.pool_id, ob.org_type, ob.org_id, ob.status, - core.plan_act_owns_org_type(ob.act_id, ob.org_type)::boolean AS owned, - COALESCE(ob.requeued_by_act_id, ob.act_id)::uuid AS motion_act_id -FROM core.plan_act_obligations ob -WHERE ob.act_id = $1::uuid AND ob.pool_id = $2::uuid -FOR UPDATE OF ob +SELECT l.act_id::uuid AS act_id, l.pool_id::uuid AS pool_id, l.org_type::varchar AS org_type, + l.org_id::uuid AS org_id, l.status::varchar AS status, l.owned::boolean AS owned, + l.motion_act_id::uuid AS motion_act_id +FROM core.lock_plan_act_obligation($1::uuid, $2::uuid) + AS l(act_id, pool_id, org_type, org_id, status, owned, motion_act_id) ` type LockPlanActObligationParams struct { @@ -565,7 +564,10 @@ type LockPlanActObligationRow struct { // act, or the Retry that requeued it). Only the obligation row is read, so a // Retry that requeues the row while this statement waits for its lock cannot // make PostgreSQL's recheck drop it; GetPlanActMotion reads the act in a -// second statement. The calling transaction holds the pool's row first. +// second statement. The calling transaction holds the pool's row first. The +// function takes the lock for the runtime role, which holds no UPDATE on the +// obligations. sqlc cannot see a function's output columns, so the alias list +// names them and the casts give them their non-null types. func (q *Queries) LockPlanActObligation(ctx context.Context, arg LockPlanActObligationParams) (LockPlanActObligationRow, error) { row := q.db.QueryRowContext(ctx, lockPlanActObligation, arg.ActID, arg.PoolID) var i LockPlanActObligationRow diff --git a/internal/entitlements/querier.go b/internal/entitlements/querier.go index f2be260a..2fc500ec 100644 --- a/internal/entitlements/querier.go +++ b/internal/entitlements/querier.go @@ -611,7 +611,10 @@ type Querier interface { // act, or the Retry that requeued it). Only the obligation row is read, so a // Retry that requeues the row while this statement waits for its lock cannot // make PostgreSQL's recheck drop it; GetPlanActMotion reads the act in a - // second statement. The calling transaction holds the pool's row first. + // second statement. The calling transaction holds the pool's row first. The + // function takes the lock for the runtime role, which holds no UPDATE on the + // obligations. sqlc cannot see a function's output columns, so the alias list + // names them and the casts give them their non-null types. LockPlanActObligation(ctx context.Context, arg LockPlanActObligationParams) (LockPlanActObligationRow, error) // The pool row taken FOR UPDATE: the lock a rule commit below the cap and // each drained pool above it hold while they materialize, taken in ascending diff --git a/internal/entitlements/queries/plan_acts.sql b/internal/entitlements/queries/plan_acts.sql index e02f2065..c767cb5e 100644 --- a/internal/entitlements/queries/plan_acts.sql +++ b/internal/entitlements/queries/plan_acts.sql @@ -57,13 +57,15 @@ SELECT COALESCE(NOT EXISTS ( -- act, or the Retry that requeued it). Only the obligation row is read, so a -- Retry that requeues the row while this statement waits for its lock cannot -- make PostgreSQL's recheck drop it; GetPlanActMotion reads the act in a --- second statement. The calling transaction holds the pool's row first. -SELECT ob.act_id, ob.pool_id, ob.org_type, ob.org_id, ob.status, - core.plan_act_owns_org_type(ob.act_id, ob.org_type)::boolean AS owned, - COALESCE(ob.requeued_by_act_id, ob.act_id)::uuid AS motion_act_id -FROM core.plan_act_obligations ob -WHERE ob.act_id = sqlc.arg(act_id)::uuid AND ob.pool_id = sqlc.arg(pool_id)::uuid -FOR UPDATE OF ob; +-- second statement. The calling transaction holds the pool's row first. The +-- function takes the lock for the runtime role, which holds no UPDATE on the +-- obligations. sqlc cannot see a function's output columns, so the alias list +-- names them and the casts give them their non-null types. +SELECT l.act_id::uuid AS act_id, l.pool_id::uuid AS pool_id, l.org_type::varchar AS org_type, + l.org_id::uuid AS org_id, l.status::varchar AS status, l.owned::boolean AS owned, + l.motion_act_id::uuid AS motion_act_id +FROM core.lock_plan_act_obligation(sqlc.arg(act_id)::uuid, sqlc.arg(pool_id)::uuid) + AS l(act_id, pool_id, org_type, org_id, status, owned, motion_act_id); -- name: GetPlanActMotion :one -- Who put an obligation in motion and what its act did, for the move's reason diff --git a/openspec/changes/runtime-database-user/tasks.md b/openspec/changes/runtime-database-user/tasks.md index f72c9511..ca4655fe 100644 --- a/openspec/changes/runtime-database-user/tasks.md +++ b/openspec/changes/runtime-database-user/tasks.md @@ -56,14 +56,14 @@ Sections 1 to 6 are one commit each, and their items are that commit's parts; se The plan-act obligation lock (items 3.10 to 3.14) lives in this section, not in a section of its own, because it is the same kind of change as the partition function (a core migration that gives the runtime user a path through the enclosure that its role alone does not have), it needs only `console_runtime` from section 1, it passes `make test` alone, and it lands before section 4, which is the first section that serves anything as the runtime user. A section of its own would renumber sections 4 to 7 and every reference to them. -- [ ] 3.1 Core `migrations/00031_webhook_partition_function.sql` (design D3, D4): Up moves `core.webhook_events` to `core_owner`, then each of its partitions through a `DO` loop over `pg_inherits`; creates `core.ensure_webhook_event_partitions` as the design writes it; sets its owner, revokes `EXECUTE` from `PUBLIC` and grants it to `console_runtime`. Down drops the function, gives the parent and each partition to `CURRENT_USER`, and then, in the same loop, runs `GRANT ALL ON core.webhook_events TO core_owner` and the same for each partition, so the database is back where the baseline's `GRANT ALL ON ALL TABLES IN SCHEMA core TO core_owner` left it (the Up's `OWNER TO core_owner` merges that explicit entry into the owner's, and without the re-grant the Down leaves `core_owner` none). The header comment says why `core_owner` owns the table, why the Down re-grants, and that the bounds must match the baseline's `DO` block and the Go names. -- [ ] 3.2 Core `internal/db/migrations/00001_init.sql`, the initial-partition `DO` block (`:1048-1064`; design D4): `m_start` becomes a `TIMESTAMP` computed as `DATE_TRUNC('month', NOW() AT TIME ZONE 'UTC') + make_interval(months => i)`, the upper bound is `m_start + INTERVAL '1 month'`, and both bounds are rendered with `AT TIME ZONE 'UTC'`, so the block names and bounds the partitions as the function does in any session time zone. The block's comment says so. Nothing else in the baseline changes in this section. -- [ ] 3.3 `internal/db/partitions.go`: `EnsureWebhookEventPartitions(ctx, ex, now, monthsAhead)` takes an executor (`ExecContext`) and runs `SELECT core.ensure_webhook_event_partitions($1, $2)`. The doc comment says that the function, owned by `core_owner`, creates the partitions, and that the naming and bounds parity is between the function and the baseline's `DO` block; the pgx placeholder paragraph goes. `cmd/bootstate.go` and `internal/workflows/maintenance/activities.go` compile unchanged. -- [ ] 3.4 `internal/db/partitions_test.go`, rewritten. `TestEnsureWebhookEventPartitions_ClosesTheGapAndIsIdempotent` runs in one transaction it rolls back, under `SET LOCAL ROLE console_runtime` and `SET LOCAL TimeZone = 'Pacific/Kiritimati'`, with the synthetic month of today's test: an insert into the target month fails with `23514` in a savepoint; the ensure creates the four partitions; inserts at the target month's first hour and last hour (UTC) each land in the target month's partition; a second ensure creates nothing; every created partition is owned by `core_owner`. Nothing is dropped, since the transaction rolls back. `TestWebhookPartitionsBelongToCoreOwner` asserts the owner of `core.webhook_events` and of every partition, and that the function is `SECURITY DEFINER`, owned by `core_owner`, configured `search_path=core, pg_temp`, and not executable by `PUBLIC`. `TestRuntimeRoleCannotCreatePartitionsDirectly`, as `console_runtime`, runs the `CREATE TABLE ... PARTITION OF` the Go code ran before and expects `42501`. `TestEnsureWebhookEventPartitionsRefusesAWindow`, one row each for 13 months, -1 months and a null time, expects `22023` and no new partition. The file's header comment says what runs as whom. -- [ ] 3.5 `schemaSnapshot` (`internal/db/integration_delivery_migration_test.go:152-153`) renders every ACL order-insensitively, for `relacl`, `attacl`, `nspacl` and `proacl`: for example `(SELECT string_agg(x::text, ',' ORDER BY x::text) FROM unnest(c.relacl) x)` in place of `c.relacl::text`. Sorting both sides keeps equal sets equal and unequal sets unequal, so the three existing up/down/up tests keep their results, and a Down that re-grants an entry appended at the end of the ACL compares equal. `TestWebhookOwnershipMoveRoundTrips` (`internal/db`), on a scratch database migrated with `dbtest.UpTo` to core `00030`: record `schemaSnapshot` (which records `relowner` and the ACLs); apply `00031` up and record again; apply it down and assert that the snapshot equals the first, so the parent and every partition are back with the user that ran the Down; apply it up again and assert that the snapshot equals the second. -- [ ] 3.6 `TestBaselinePartitionBoundsAreUTC` (`internal/db`), on a scratch database: apply core version 1 on a connection whose `TimeZone` is `Pacific/Kiritimati` (a runtime parameter of the connection string), then assert that each of the three initial partitions' bounds, read as `timestamptz`, is midnight UTC on the first of the current UTC month and of the two months after it, and that the names carry those months. -- [ ] 3.7 `TestRuntimeRolePrivileges` asserts `EXECUTE` on `core.ensure_webhook_event_partitions` and on `core.lock_plan_act_obligation` for `console_runtime`. -- [ ] 3.8 Breaks (the migrations' with the reset): +- [x] 3.1 Core `migrations/00031_webhook_partition_function.sql` (design D3, D4): Up moves `core.webhook_events` to `core_owner`, then each of its partitions through a `DO` loop over `pg_inherits`; creates `core.ensure_webhook_event_partitions` as the design writes it; sets its owner, revokes `EXECUTE` from `PUBLIC` and grants it to `console_runtime`. Down drops the function, gives the parent and each partition to `CURRENT_USER`, and then, in the same loop, runs `GRANT ALL ON core.webhook_events TO core_owner` and the same for each partition, so the database is back where the baseline's `GRANT ALL ON ALL TABLES IN SCHEMA core TO core_owner` left it (the Up's `OWNER TO core_owner` merges that explicit entry into the owner's, and without the re-grant the Down leaves `core_owner` none). The header comment says why `core_owner` owns the table, why the Down re-grants, and that the bounds must match the baseline's `DO` block and the Go names. +- [x] 3.2 Core `internal/db/migrations/00001_init.sql`, the initial-partition `DO` block (`:1048-1064`; design D4): `m_start` becomes a `TIMESTAMP` computed as `DATE_TRUNC('month', NOW() AT TIME ZONE 'UTC') + make_interval(months => i)`, the upper bound is `m_start + INTERVAL '1 month'`, and both bounds are rendered with `AT TIME ZONE 'UTC'`, so the block names and bounds the partitions as the function does in any session time zone. The block's comment says so. Nothing else in the baseline changes in this section. +- [x] 3.3 `internal/db/partitions.go`: `EnsureWebhookEventPartitions(ctx, ex, now, monthsAhead)` takes an executor (`ExecContext`) and runs `SELECT core.ensure_webhook_event_partitions($1, $2)`. The doc comment says that the function, owned by `core_owner`, creates the partitions, and that the naming and bounds parity is between the function and the baseline's `DO` block; the pgx placeholder paragraph goes. `cmd/bootstate.go` and `internal/workflows/maintenance/activities.go` compile unchanged. +- [x] 3.4 `internal/db/partitions_test.go`, rewritten. `TestEnsureWebhookEventPartitions_ClosesTheGapAndIsIdempotent` runs in one transaction it rolls back, under `SET LOCAL ROLE console_runtime` and `SET LOCAL TimeZone = 'Pacific/Kiritimati'`, with the synthetic month of today's test: an insert into the target month fails with `23514` in a savepoint; the ensure creates the four partitions; inserts at the target month's first hour and last hour (UTC) each land in the target month's partition; a second ensure creates nothing; every created partition is owned by `core_owner`. Nothing is dropped, since the transaction rolls back. `TestWebhookPartitionsBelongToCoreOwner` asserts the owner of `core.webhook_events` and of every partition, and that the function is `SECURITY DEFINER`, owned by `core_owner`, configured `search_path=core, pg_temp`, and not executable by `PUBLIC`. `TestRuntimeRoleCannotCreatePartitionsDirectly`, as `console_runtime`, runs the `CREATE TABLE ... PARTITION OF` the Go code ran before and expects `42501`. `TestEnsureWebhookEventPartitionsRefusesAWindow`, one row each for 13 months, -1 months and a null time, expects `22023` and no new partition. The file's header comment says what runs as whom. +- [x] 3.5 `schemaSnapshot` (`internal/db/integration_delivery_migration_test.go:152-153`) renders every ACL order-insensitively, for `relacl`, `attacl`, `nspacl` and `proacl`: for example `(SELECT string_agg(x::text, ',' ORDER BY x::text) FROM unnest(c.relacl) x)` in place of `c.relacl::text`. Sorting both sides keeps equal sets equal and unequal sets unequal, so the three existing up/down/up tests keep their results, and a Down that re-grants an entry appended at the end of the ACL compares equal. `TestWebhookOwnershipMoveRoundTrips` (`internal/db`), on a scratch database migrated with `dbtest.UpTo` to core `00030`: record `schemaSnapshot` (which records `relowner` and the ACLs); apply `00031` up and record again; apply it down and assert that the snapshot equals the first, so the parent and every partition are back with the user that ran the Down; apply it up again and assert that the snapshot equals the second. +- [x] 3.6 `TestBaselinePartitionBoundsAreUTC` (`internal/db`), on a scratch database: apply core version 1 on a connection whose `TimeZone` is `Pacific/Kiritimati` (a runtime parameter of the connection string), then assert that each of the three initial partitions' bounds, read as `timestamptz`, is midnight UTC on the first of the current UTC month and of the two months after it, and that the names carry those months. +- [x] 3.7 `TestRuntimeRolePrivileges` asserts `EXECUTE` on `core.ensure_webhook_event_partitions` and on `core.lock_plan_act_obligation` for `console_runtime`. +- [x] 3.8 Breaks (the migrations' with the reset): - the function computes the month in the session's time zone (`date_trunc('month', p_now)` without `AT TIME ZONE 'UTC'`): caught by the last-hour insert; - no `GRANT EXECUTE ... TO console_runtime`: caught by the gap test and `TestRuntimeRolePrivileges`; - `SECURITY INVOKER` in place of `SECURITY DEFINER`: caught by the gap test (`42501` on the create) and the ownership test; @@ -78,9 +78,9 @@ The plan-act obligation lock (items 3.10 to 3.14) lives in this section, not in - `00031`'s Down without the `core_owner` re-grant: caught by `TestWebhookOwnershipMoveRoundTrips`; - `schemaSnapshot` with the ACLs in stored order, as the test code reads it (a test-side record): caught by `TestWebhookOwnershipMoveRoundTrips` on a correct Down, whose re-grant appends the entry at the end; - `TestRuntimeRoleCannotCreatePartitionsDirectly` without its `SET LOCAL ROLE` (a test-side record): caught, because the statement succeeds as the superuser; -- [ ] 3.9 `docs/database-management.md`, "Roles": a paragraph on ownership (design D3): objects belong to the migration user, `core_owner` owns the definer functions and `core.webhook_events` with its partitions, and the runtime user owns nothing, neither in the schemas nor the database. -- [ ] 3.10 Core `migrations/00032_lock_plan_act_obligation.sql` (design D19): Up creates `core.lock_plan_act_obligation(p_act_id UUID, p_pool_id UUID)` as the design writes it, sets its owner to `core_owner`, revokes `EXECUTE` from `PUBLIC` and grants it to `core_writer`, in the order and shape of `00028`'s other definer functions; Down drops the function. The header comment says that a locking read needs `UPDATE`, which the enclosure withholds from the writer, why the function reads only the obligation row, and that the lock, its mode and its order are those of the query it replaces. -- [ ] 3.11 `internal/entitlements/queries/plan_acts.sql`: `LockPlanActObligation` becomes the query below (design D19); its comment keeps the account of why only the obligation row is read, adds that the function takes the lock for the runtime role, and says that sqlc cannot see a function's output columns, so the alias list names them and the casts give them their non-null types. Regenerate with `make sqlc-generate`, which restores the SPDX header that sqlc drops, so `internal/entitlements/plan_acts.sql.go` is regenerated; the row type, `LockPlanActObligationRow`, and `plan_act_move.go` do not change. +- [x] 3.9 `docs/database-management.md`, "Roles": a paragraph on ownership (design D3): objects belong to the migration user, `core_owner` owns the definer functions and `core.webhook_events` with its partitions, and the runtime user owns nothing, neither in the schemas nor the database. +- [x] 3.10 Core `migrations/00032_lock_plan_act_obligation.sql` (design D19): Up creates `core.lock_plan_act_obligation(p_act_id UUID, p_pool_id UUID)` as the design writes it, sets its owner to `core_owner`, revokes `EXECUTE` from `PUBLIC` and grants it to `core_writer`, in the order and shape of `00028`'s other definer functions; Down drops the function. The header comment says that a locking read needs `UPDATE`, which the enclosure withholds from the writer, why the function reads only the obligation row, and that the lock, its mode and its order are those of the query it replaces. +- [x] 3.11 `internal/entitlements/queries/plan_acts.sql`: `LockPlanActObligation` becomes the query below (design D19); its comment keeps the account of why only the obligation row is read, adds that the function takes the lock for the runtime role, and says that sqlc cannot see a function's output columns, so the alias list names them and the casts give them their non-null types. Regenerate with `make sqlc-generate`, which restores the SPDX header that sqlc drops, so `internal/entitlements/plan_acts.sql.go` is regenerated; the row type, `LockPlanActObligationRow`, and `plan_act_move.go` do not change. ```sql SELECT l.act_id::uuid AS act_id, l.pool_id::uuid AS pool_id, l.org_type::varchar AS org_type, @@ -89,9 +89,9 @@ The plan-act obligation lock (items 3.10 to 3.14) lives in this section, not in FROM core.lock_plan_act_obligation(sqlc.arg(act_id)::uuid, sqlc.arg(pool_id)::uuid) AS l(act_id, pool_id, org_type, org_id, status, owned, motion_act_id); ``` -- [ ] 3.12 `TestLockPlanActObligationRunsAsTheRuntimeRole` (`internal/entitlements/plan_act_queries_db_test.go`, with the helpers of `TestLockPlanActObligationReadsTheMove`), in one transaction it rolls back: write the act and its obligation as the test login, `SET LOCAL ROLE console_runtime`, call `LockPlanActObligation` and assert the row of `TestLockPlanActObligationReadsTheMove`; requeue it and assert the Retry as the motion; assert `sql.ErrNoRows` for a pool with no obligation; and in a savepoint run `SELECT ... FROM core.plan_act_obligations WHERE false FOR UPDATE` and assert SQLSTATE `42501`, which names the reason for the function. `TestPlanActWritersAreEnclosedInTheCatalog` (`internal/db/plan_act_obligations_test.go`) gains `lock_plan_act_obligation` in its list of functions and in its `PUBLIC` and `core_reader` refusal loop, which asserts that the function is `SECURITY DEFINER`, owned by `core_owner`, configured `search_path=core, pg_temp`, executable by `core_writer` and `console_runtime`, and not by `PUBLIC` or `core_reader`; no new test. `TestLockPlanActObligationSurvivesARequeueItWaitedOn` and the plan-act drain tests run unchanged. -- [ ] 3.13 `docs/database-locks.md`: in the plan act obligation row's "Taken by" cell and in the move's paragraph, the `LockPlanActObligation` query is described as calling `core.lock_plan_act_obligation`, which takes the same `FOR UPDATE` after the pool row; the lock, its mode and the order are unchanged. Nothing else in the file changes. -- [ ] 3.14 Breaks for 3.10 to 3.13 (the migration's with the reset): +- [x] 3.12 `TestLockPlanActObligationRunsAsTheRuntimeRole` (`internal/entitlements/plan_act_queries_db_test.go`, with the helpers of `TestLockPlanActObligationReadsTheMove`), in one transaction it rolls back: write the act and its obligation as the test login, `SET LOCAL ROLE console_runtime`, call `LockPlanActObligation` and assert the row of `TestLockPlanActObligationReadsTheMove`; requeue it and assert the Retry as the motion; assert `sql.ErrNoRows` for a pool with no obligation; and in a savepoint run `SELECT ... FROM core.plan_act_obligations WHERE false FOR UPDATE` and assert SQLSTATE `42501`, which names the reason for the function. `TestPlanActWritersAreEnclosedInTheCatalog` (`internal/db/plan_act_obligations_test.go`) gains `lock_plan_act_obligation` in its list of functions and in its `PUBLIC` and `core_reader` refusal loop, which asserts that the function is `SECURITY DEFINER`, owned by `core_owner`, configured `search_path=core, pg_temp`, executable by `core_writer` and `console_runtime`, and not by `PUBLIC` or `core_reader`; no new test. `TestLockPlanActObligationSurvivesARequeueItWaitedOn` and the plan-act drain tests run unchanged. +- [x] 3.13 `docs/database-locks.md`: in the plan act obligation row's "Taken by" cell and in the move's paragraph, the `LockPlanActObligation` query is described as calling `core.lock_plan_act_obligation`, which takes the same `FOR UPDATE` after the pool row; the lock, its mode and the order are unchanged. Nothing else in the file changes. +- [x] 3.14 Breaks for 3.10 to 3.13 (the migration's with the reset): - the sqlc query returned to the bare `SELECT ... FROM core.plan_act_obligations ... FOR UPDATE OF ob` (the generated text edited the same way): caught by `TestLockPlanActObligationRunsAsTheRuntimeRole` (`42501` at the lock); - the function without `FOR UPDATE`: caught by `TestLockPlanActObligationSurvivesARequeueItWaitedOn` (the move never waits); - `SECURITY INVOKER` in place of `SECURITY DEFINER`: caught by `TestLockPlanActObligationRunsAsTheRuntimeRole` and `TestPlanActWritersAreEnclosedInTheCatalog`; @@ -100,6 +100,7 @@ The plan-act obligation lock (items 3.10 to 3.14) lives in this section, not in - no `GRANT EXECUTE ... TO core_writer`: caught by `TestLockPlanActObligationRunsAsTheRuntimeRole` and `TestRuntimeRolePrivileges`; - the function written to return a single composite row in place of `TABLE` (a missing pair would return a row of nulls): caught by the `sql.ErrNoRows` assertion of `TestLockPlanActObligationRunsAsTheRuntimeRole`; - `UPDATE` granted on `core.plan_act_obligations` to `core_writer` in place of the function (with the reset): caught by `TestRuntimeRolePrivileges` (the locked list holds `SELECT` only). +- [x] 3.15 `TestPlanActOwnerRuleIsStatedOnce` (`internal/entitlements/plan_act_queries_db_test.go`) changes: `LockPlanActObligation` leaves its list of queries that must call `core.plan_act_owns_org_type`, because the query now calls `core.lock_plan_act_obligation` and the function states the rule, and `lock_plan_act_obligation` joins the list of functions whose definitions must call it. The test would otherwise fail on the query, and the rule is still stated once per reader. Break (with the reset): the function's `core.plan_act_owns_org_type(ob.act_id, ob.org_type)::boolean` replaced by `true`: caught by `TestPlanActOwnerRuleIsStatedOnce` (`core.lock_plan_act_obligation does not call`). ## 4. Two connection strings, the boot grant, and the test stack on two users