Skip to content

Delete and expire library-owned experience data (story 4.5) - #17

Merged
fabbrik merged 1 commit into
mainfrom
feat/4-5-delete-and-expire
Sep 23, 2026
Merged

fabbrik merged 1 commit into
mainfrom
feat/4-5-delete-and-expire

Conversation

@fabbrik

@fabbrik fabbrik commented Sep 23, 2026

Copy link
Copy Markdown
Owner

Revocation stops reads. This adds the erasure: one authorized, atomic, scope-safe operation that removes every payload-bearing trace of an experience across seven tables and leaves a payload-free tombstone preventing the ID from being written again — plus a bounded retention sweep running the same operation.

Erasure is an exception to the append-only guards, not a suspension of them

0010 replaces the trigger functions so they recognise one transaction-scoped marker set only inside the purge function. 0006 is journaled and untouched, every ENABLE ALWAYS binding survives, and the guard stays armed for every other statement in every other session throughout. This replaces the previously documented runbook, which disabled a trigger table-wide — leaving the audit guard off for every connection in the pool, and off permanently if the transaction died in between.

It is an auditability mechanism, not a privilege boundary, and the docs say so in those words. A custom GUC is settable by any session, and the guards already do not bind a role that can ALTER TABLE — which is the application role. What it buys is one code path, one transaction, and a guard that is never switched off.

What three reviewers found

One reviewer stood up a real database and attacked the purge with SQL rather than reasoning about it. That found what code reading did not:

  • Privilege escalation. The SECURITY DEFINER purge functions had no REVOKE EXECUTE … FROM PUBLIC, so a SELECT-only reporting role — denied DELETE and UPDATE on every table — could permanently erase a record in another tenant. Fixed, with a test using a real non-owner role.
  • Concurrent writers survived a purge that reported Deleted. An embedding whose vector is a searchable derivative of the erased text; a live 90-day grant over a spent ID; a reviewer identity and free-text rationale about the erased record, in an append-only table. Every writer that gates on deleted_at now takes a key-share lock and serializes against the purge.
  • DELETE FROM experience_records was completely unguarded — orphaning the audit trail and freeing the ID, so a reinsert inherited the old grants. Now refused outright: the purge updates that row into a tombstone and never deletes it.
  • Two concurrent purges sharing a feedback submission orphaned it permanently, each seeing the other's uncommitted exposure delete. Submissions are now locked before their exposures are removed.
  • The full erased payload survives in dead heap tuples until VACUUM — and the one sentence about residue reassured the opposite. Corrected; it is the sentence that matters most to anyone with a real erasure obligation.

The mutation reviewer explained why the worst defects had to be found by hand: there was no concurrency test anywhere in this story, though the suite has the pattern three times elsewhere. There are now six.

Also in this change

Closes the cross-scope squat by mixing Scope into the derived experience ID — a breaking change to record identity, taken now because it is free before release. Injects TimeProvider into the stores that lacked one. The destructive grant purge uses LEAST(host clock, clock_timestamp()), so a skewed host clock cannot destroy a grant the database still considers live.

Documentation states what erasure does not reach, that a sweep matches one scope exactly (the one operation whose failure mode is a missed retention obligation reported as success), which tombstone protections are schema-enforced and which are adapter-enforced, and a CREATE INDEX CONCURRENTLY runbook for the three new indexes, which build under a lock on the largest table this library writes.

Tests: 1295 → 1333, zero warnings under TreatWarningsAsErrors.

🤖 Generated with Claude Code

Revocation stops reads; this adds the erasure. One authorized, atomic,
scope-safe operation removes every payload-bearing trace of an experience
across seven tables and leaves a payload-free tombstone that prevents the ID
being written again. A retention sweep runs the same operation in bounded
batches; default retention is indefinite and scheduling belongs to the host.

Erasure is an authorized exception to the append-only guards, not a suspension
of them. A new 0010 replaces the trigger functions so they recognise one
transaction-scoped marker set only inside the purge function; 0006 is journaled
and untouched, every ENABLE ALWAYS binding survives, and the guard stays armed
for every other statement in every other session throughout. This replaces the
documented runbook that disabled a trigger table-wide.

It is an auditability mechanism, not a privilege boundary, and the
documentation says so: a custom GUC is settable by any session and the guards
already do not bind a role that can ALTER TABLE. What it buys is one code path,
inside one transaction, with the guard never switched off and never left off
across a failure.

The purge functions are SECURITY DEFINER, so EXECUTE is revoked from PUBLIC and
granted explicitly. Without that revoke a SELECT-only role could erase any
record in any tenant.

Writers that gate on deleted_at now take a key-share lock on the record row, so
an in-flight embedding, grant or feedback write serializes against a purge
instead of against a snapshot taken before it committed. experience_records
gains DELETE and TRUNCATE guards: the purge updates that row into a tombstone
and never deletes it, so nothing legitimate removes it, and a removal would
orphan the audit trail and free the ID for reuse under another scope's grants.

What erasure does not reach is stated rather than implied: backups, replicas,
WAL, exported telemetry, external artifacts, dead index entries until VACUUM,
and the previous row version in the heap, which still carries the erased text
until the table is vacuumed.

Also closes the cross-scope squat by mixing Scope into the derived experience
ID -- a breaking change to record identity, taken now because it is free before
release -- and injects TimeProvider into the stores that lacked one.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@fabbrik
fabbrik merged commit 718318f into main Sep 23, 2026
1 check passed
@fabbrik
fabbrik deleted the feat/4-5-delete-and-expire branch September 23, 2026 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant