Audit retention drops partitions by name in the session's time zone, without locking, and can destroy events newer than the cutoff #1039

Audit retention drops partitions chosen by name in the calling session's time zone, and locks nothing before dropping them. Both lose audit rows that are newer than the retention cutoff.

This is pre-existing, in 1_baseline.up.sql. What makes it worth filing now is that #1000 documents both defects precisely, builds a hardened replacement, and routes only the one-time history copy through it — leaving the recurring, unattended path on the unhardened function.

Where it is

PostgresStore.DropAuditPartitionsBefore (module/services/accounts/code/pkg/infra/postgres_audit_registry.go:138) calls audit_events_drop_partitions_before, and its own comment says "This is how audit retention runs."

That function is defined at module/services/store/migrations/1_baseline.up.sql:212-242.

Defect 1 — the upper bound is derived from the partition's name, in the session's zone

upper_bound := date_trunc(
    'month',
    to_date(substring(part.relname FROM 'audit_events_([0-9]{4}_[0-9]{2})'), 'YYYY_MM')
) + INTERVAL '1 month';

date_trunc('month', <date>) yields timestamp without time zone; + INTERVAL '1 month' keeps it so; assigning it into the declared upper_bound TIMESTAMPTZ is an implicit cast in the calling session's zone. The partition's real bounds were set by whatever zone created it.

So a partition created from a New York session ends at 2026-09-01 00:00-04 — 04:00 UTC. A retention run in UTC computes 2026-09-01 00:00+00. With cutoff = 2026-09-01 00:00 UTC, upper_bound <= cutoff is true, and audit_events_2026_08 is dropped — destroying four hours of events whose created_at is after the cutoff.

It never reads relpartbound, so it cannot be correct in general.

Defect 2 — nothing is locked and nothing is counted

The function iterates and DROP TABLEs. A row committed into a partition concurrently is destroyed unseen: DROP takes ACCESS EXCLUSIVE implicitly, waits for the inserting transaction, then takes its row with the table. There is also no lock_timeout, so retention can block indefinitely on a long writer.

This is exactly what #1000 wrote down

32_audit_verified_partition_drop.up.sql's header names both, as the reasons a verified retirement "cannot rely on" that function:

• It chooses partitions by the month in their NAME, read in the calling session's time zone. … a drop session in UTC asked to retire everything before 1 September also dropped audit_events_2026_08, whose last four hours (when created from New York) are September's events, which no one had verified.
• It drops what it finds. A row committed into a partition between the copy's last recount and the DROP was destroyed unseen: nothing locked the partition, and a DROP waits for an inserting transaction and then takes its row with the table.

The new audit_events_drop_verified_partitions fixes both — reads bounds from pg_get_expr(c.relpartbound, c.oid), locks the parent then each partition ACCESS EXCLUSIVE in a fixed order, recounts inside the drop's own transaction, sets lock_timeout to 10s. I verified each of those claims against the code and they hold.

But retention is the path that runs on a schedule, with no human present, and it is the one left exposed. The hardened path runs once per deployment.

The fix

Retention cannot reuse audit_events_drop_verified_partitions: that one requires an explicit partition list with verified row counts, which retention does not have.

The narrow fix is a new migration redefining audit_events_drop_partitions_before to:

1. read each partition's upper bound from pg_get_expr(c.relpartbound, c.oid) and parse it back, rather than deriving it from the name — the exact pattern auditPartitionsSQL (postgres_audit_history.go:26-35) and migration 32 already use, and which is zone-safe because audit_events.created_at is timestamp with time zone (1_baseline.up.sql:1996) so the rendered bound carries its offset;
2. LOCK TABLE public.audit_events IN ACCESS EXCLUSIVE MODE, then each partition it is about to drop, before dropping any;
3. set lock_timeout so a stuck writer is a refusal rather than a hang;
4. refuse a partition whose bounds do not parse — a DEFAULT or MAXVALUE partition should never be dropped by a cutoff rule.

Roughly fifteen lines of plpgsql, with migration 32 alongside to copy from.

Worth a regression that would have caught it: create a partition from a session in a non-UTC zone, run retention from UTC with a cutoff at that month's UTC boundary, and assert the partition survives.

Not verified

I read this against the tree at #1000's head (db384ee77) and did not run it: no database, no retention run, and I did not reproduce the zone drop against a live PostgreSQL. The reasoning is from the function body, the declared types and audit_events' partition key; the scenario is the one migration 32's own header describes.