fix(db): purchase_history.account_id VARCHAR(20) drops the audit row after every Azure/GCP purchase

[cristim]

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.

purchase_history.account_id is still VARCHAR(20) and was never widened. Every Azure purchase, and every GCP purchase whose project ID exceeds 20 characters, spends real money and then fails its audit write. This was verified independently by two reviewers on this pass.

Where

• internal/database/postgres/migrations/000001_initial_schema.up.sql:103 - account_id VARCHAR(20) NOT NULL
• Writer: internal/config/store_postgres.go:1686 (SavePurchaseHistory)
• Value source: internal/purchase/execution.go:255 (accountID := account.ExternalID) flowing to execution.go:811 (AccountID: accountID)
• Error propagation: internal/purchase/execution.go:842 (savePurchaseHistory returns the error deliberately, per bug(purchases): approved AWS purchase vanishes from History — 'approved'/'running' execution statuses hidden by the History list (P0) #621, so the caller can record an audit gap)
• Contrast: cloud_accounts.external_id is VARCHAR(255) (000011_...up.sql:16), so the value reaching SavePurchaseHistory is unconstrained at its source.

What

An Azure subscription ID is a 36-character GUID. A GCP project ID is 6 to 30 characters. Both are written into a column that holds 20.

The decisive evidence that this is an oversight and not a deliberate constraint: the identical widening was already applied to the sibling column and skipped here. Migration 000067_analytics_snapshot_correctness widens savings_snapshots.account_id to VARCHAR(255), and 000074_repair_partial_migration_058_067 repairs that same widening on partially-migrated databases. Migration 000067's own header states the reason verbatim:

VARCHAR(20) is too small for Azure subscription IDs (36) and GCP project IDs (<=30)

It fixed exactly one of the two tables carrying that column. Grepping every ALTER TABLE purchase_history in the tree (000011, 000034, 000063, 000070, 000071, 000072, 000073, 000074, 000087) and every ALTER COLUMN ... TYPE across all 86 migration pairs confirms it: no migration anywhere alters purchase_history.account_id.

Failure scenario

An Azure reservation is purchased for a cloud account with external_id = "3f2504e0-4f89-11d3-9a0c-0305e82c3301" (36 chars, a subscription GUID). processPurchaseRecommendations passes it as accountID. The Azure SDK call succeeds and the customer is billed for the commitment. SavePurchaseHistory then issues its INSERT and Postgres rejects it with:

22001: value too long for type character varying(20)

The result is a real, billed Azure commitment with no purchase_history row at all. Downstream consequences, in order of damage:

1. It is invisible in the History view.
2. It is absent from GetActivePurchaseHistory, so the analytics collector never snapshots it and the dashboard KPIs undercount committed spend.
3. The grace-period and suppression logic never sees it.
4. The next recommendation cycle believes the capacity was never bought and re-proposes it. So the failure does not merely lose an audit record: it silently repeats the purchase.

GCP project IDs over 20 characters fail identically. This hits any Azure deployment, and any GCP deployment with a project ID longer than 20 characters, on every single purchase.

Fix direction

Add a migration widening purchase_history.account_id to VARCHAR(255), matching cloud_accounts.external_id and savings_snapshots.account_id. Guard it with the same information_schema.columns character-length probe migration 000067 uses, so it is idempotent on partially-migrated databases (the same reason 000074 exists).

Because purchases have already been silently dropped on any Azure deployment that ran a purchase, the fix should be paired with an operator-facing note: rows lost to the 22001 are not recoverable from the database and have to be reconciled from the provider side or from the CLI audit log.

Related

• chore(db): backfill purchase_history.cloud_account_id from (provider, external_id) cloud-commitments-platform#22 (backfill purchase_history.cloud_account_id from (provider, external_id)) touches the same column pair; a backfill cannot repair rows that were never inserted.
• bug(purchases): approved AWS purchase vanishes from History — 'approved'/'running' execution statuses hidden by the History list (P0) #621 is the reason the error is propagated rather than swallowed, which is correct behaviour and not part of this defect.

[cristim]

Closed by PR #1677 — verification record

Auto-closed by the Closes keyword; recording what was actually verified, since the merge decision rested on it.

Independent adversarial review (reviewer distinct from the implementer) verified the fix against a real Postgres testcontainer, not by reading the diff:

• The regression test genuinely bites. Against the pre-migration schema, both realistic values were rejected live:

  Migrated to version 92
  pre-fix rejection for 3f2504e0-4f89-11d3-9a0c-0305e82c3301:
    ERROR: value too long for type character varying(20) (SQLSTATE 22001)
  pre-fix rejection for cudly-production-analytics-001:
    ERROR: value too long for type character varying(20) (SQLSTATE 22001)
  

  36-char Azure subscription GUID and 30-char GCP project ID, inserted through the same NOT NULL column set SavePurchaseHistory uses.

• Falsified, not just run. Replacing the up migration with SELECT 1; makes the post-migration assertions fail — so the test could not have stayed green with the bug present.

• Migration numbering: 000095 checked against origin/main's highest (000093) and every open PR (only feat(admin): per-API-key usage stats + summary card (closes #380 scope) #1523 carries 000094). No collision in either merge order.

• A latent CI trap was found and fixed. TestMigrations_AutoHealDirty asserted rollback lands on headVersion - 1 — a contiguity assumption this repo violates (it already skips 61-62, 68-69, 82, 84-85). The replacement derives the target from .up.sql filenames and was exercised against every gap in the repo, not only the one this PR creates.

• Down migration fails loudly rather than truncating: with an Azure GUID row present, rollback errors with refusing to narrow purchase_history.account_id and the row is re-read intact.

• Scanner/projection parity: account_id present in all five SELECT projections and the shared scanner as a plain string, NOT NULL since 000001, never altered by a later migration. No Go change required.

• Catalog verified post-migration: all 9 indexes indisvalid/indisready, 6 constraints validated, 0 dependent views or matviews. Widening a varchar is binary-coercible so Postgres skipped the rewrite.

Two non-blocking notes carried forward

1. TestMigration095_DownNarrowsWhenSafe is vacuous as a regression guard — it passes under falsification, since asserting width==20 after rollback is trivially true if the up never widened. It earns its keep only as coverage of the down's ALTER branch.
2. The down migration guards on typmod IS DISTINCT FROM 259, so it cannot distinguish "the up widened 20→255" from "an operator hand-widened it beforehand and the up no-opped". Data loss is still prevented by the over_long > 0 guard; worst case is a schema regression, not truncation.

Sibling issues filed from this work

• #1676 — ri_exchange_history.account_id has the same VARCHAR(20) plus an AWS-shaped ^\d{12}$ CHECK. Latent (no Azure/GCP writer today), and widening alone would not fix it because the CHECK rejects a GUID at any width.
• #1679 — interactive AWS/Azure exchange endpoints persist no ri_exchange_history row at all, so that spend bypasses the daily-cap accounting.