chore(config/db): low correctness findings from the 2026-09-02 audit #316

Summary

Four low-severity correctness defects in the Postgres migrations and the config store, each confirmed against the audited commit: the 000059 down migration deletes the Standard Users group, 000027 dedupes a partitioned table by ctid without tableoid, 000040 rewrites a purchase_plans key delimiter the API never writes and the provider parser fails open on the other shape, and MarkPurchaseRevoked leaves revocation_in_flight set against its documented contract.

Each item is independently actionable; they are grouped so one sitting can clear them.

Findings

• Rollback of 000059 deletes the Standard Users group and strips every member (A04-010)

  • Location: internal/database/postgres/migrations/000059_seed_purchaser_group.down.sql:5-11; same unguarded array_remove in 000024_*.down.sql:5-14 and 000096_*.down.sql:11-13 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
  • Failure scenario: At head, UUID ...0005 is Standard Users (000057) and Purchaser was relocated to ...0007 by 000064, which records that 000059's INSERT was a silent no-op. Rolling back through 000059 therefore removes ...0005 from every user's group_ids and deletes Standard Users. A user whose only group was Standard Users ends with an empty array and trips users_min_one_group (still in force at that point), aborting the rollback mid-way; if no such user exists the rollback silently destroys the group. 000096.down and 000024.down have the same empty-array hazard for RI Exchanger and Administrators.
  • Evidence:

    UPDATE users
    SET group_ids = array_remove(group_ids, '00000000-0000-5000-8000-000000000005'::UUID)
    WHERE '00000000-0000-5000-8000-000000000005'::UUID = ANY(group_ids);
    DELETE FROM groups
    WHERE id = '00000000-0000-5000-8000-000000000005';

  • Suggested fix: Make 000059.down a documented no-op (000064 owns Purchaser's lifecycle) and guard the other seed downs with WHERE cardinality(group_ids) > 1, failing loud otherwise.

• Migration 000027 dedupes a partitioned table by ctid, which is not unique across partitions (A04-011)

  • Location: internal/database/postgres/migrations/000027_savings_snapshots_pk.up.sql:52-62; table declared PARTITION BY RANGE (timestamp) at 000001_initial_schema.up.sql:226-230 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
  • Failure scenario: ctid is per-heap, so two rows in different savings_snapshots_YYYY_MM partitions can share (0,1). The DELETE joins s.ctid = d.ctid without tableoid, so a genuine duplicate at ctid (0,1) in one partition also matches the unrelated first row of every other partition and deletes it. Harmless only because production had zero duplicates when it ran; the migration's own header anticipates a forced re-run (CUDLY_FORCE_MIGRATION_VERSION=26), which on a database that has since accumulated a duplicate destroys unrelated snapshot rows.
  • Evidence:

    WITH duplicates AS (
        SELECT id, timestamp, ctid,
               ROW_NUMBER() OVER (PARTITION BY id, timestamp ORDER BY created_at DESC, ctid DESC) AS row_num
        FROM savings_snapshots
    )
    DELETE FROM savings_snapshots s
    USING duplicates d
    WHERE s.ctid = d.ctid AND d.row_num > 1;

  • Suggested fix: Select tableoid in the CTE and join on (tableoid, ctid).

• Migration 000040's purchase_plans JSONB rewrite targets a key delimiter the API never writes, and the parser fails open on the other (A04-012)

  • Location: internal/database/postgres/migrations/000040_split_savingsplans.up.sql:118-149 (keys at :125,148); key writer internal/api/types.go:861; parser internal/config/purchase_plan_providers.go:17-20; empty-set guards at internal/config/store_postgres.go:3629-3632 and internal/api/handler_accounts.go:1234-1238 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
  • Failure scenario: Every plan row is written by buildServiceConfig with keys provider + "/" + service. 000040 rewrites keys named aws:savings-plans / aws:savingsplans, so an API-written plan keeps aws/savings-plans and was never split; only the aliases in internal/purchase/execution.go:1171-1179 keep it working. Conversely DerivePlanProviders cuts on / and silently skips unparseable keys, so any plan whose keys carry the colon form yields an empty provider set, and both guards treat an empty set as "no restriction", accepting an account of any provider on that plan. (The audit's claim that 000001's schema comment assumes the colon form did not hold up; the 000040 and 000060 fixture shapes do.)
  • Evidence:

    WHERE k NOT IN ('aws:savings-plans', 'aws:savingsplans')
    ...
    WHERE services ?| ARRAY['aws:savings-plans', 'aws:savingsplans'];
    -- parser:
    -- provider, _, ok := strings.Cut(k, "/")
    -- if !ok || provider == "" { continue }

  • Suggested fix: Pick one delimiter, add a data migration that normalizes existing keys to it, and make DerivePlanProviders return an error rather than an empty set on an unparseable key so the provider check fails closed. Related identifier cleanup: Normalize 'savings-plans' vs 'savingsplans' identifier across frontend + backend #7.

• MarkPurchaseRevoked does not clear revocation_in_flight, contradicting the documented contract (A04-013)

  • Location: internal/config/store_postgres.go:2433-2442; contract at migrations/000072_purchase_history_revocation_in_flight.up.sql:13-15 and internal/config/types.go:906-913; only caller of ClearRevocationInFlight is the Return-error path at internal/api/handler_purchases_revoke.go:746-747 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd
  • Failure scenario: After a successful Azure return, FlipPurchaseRevocationInFlight sets the flag and MarkPurchaseRevoked stamps revoked_at but leaves revocation_in_flight = true. idx_purchase_history_revocation_in_flight never shrinks and every revoked Azure row reads back RevocationInFlight: true; only the revoke handler checking RevokedAt != nil first keeps a revoked purchase from being reported as reconcile-pending. Any new reader that trusts the documented meaning misclassifies every completed revocation.
  • Evidence:

    UPDATE purchase_history
       SET revoked_at            = $2,
           revoked_via           = $3,
           support_case_id       = $4,
           calc_refund_amount    = $5,
           calc_refund_currency  = $6
     WHERE purchase_id = $1
       AND revoked_at IS NULL

  • Suggested fix: Add revocation_in_flight = false to the SET list, or change the two doc comments to say the flag is never cleared.

Found by the 2026-09-02 codebase audit, findings A04-010, A04-011, A04-012, A04-013, reported by one reviewer and independently confirmed by a second. Full report: docs/audits/codebase-audit-2026-09-02.md.