feat(identity): vault + account delete — real teardown, 24h undo, convergence sweep (cloud#226) - #241
Open
unforcedagi wants to merge 6 commits into
Open
Conversation
…cloud#226) Replaces the honest 501 stub with the door the issue asks for: the hub twin's retype-the-name confirm + userOwnsVault gate, then the vault worker's POST /api/internal/destroy (PR-1, ParachuteComputer#229 — DO storage + the whole vault-<name>/ R2 prefix), then the identity-side D1 cascade. Order is load-bearing. The vault worker is the source of truth for whether tenant data actually disappeared, so destroy runs BEFORE any D1 write and its response is inspected, not assumed — a transport error, a non-2xx, an unreadable body, or a malformed success shape all leave every identity row intact for a retry (502 vault_destroy_failed). Only once storage is gone does the D1 batch run, and if THAT fails the response says so distinctly (500 d1_cleanup_failed) because the storage side is already erased. vaults.ts deleteVaultD1Rows ports the hub's cascade onto D1: revoke the tokens naming this vault, REWRITE (not drop) the grants that name it — a grants row is keyed (user, client) and spans every vault that user approved for that client, so dropping it over one vault would silently revoke consent on the others — then drop the vault, usage, and snapshot rows, all in one D1 batch. Matching is EXACT scope-segment comparison, never LIKE. The LIKE pattern is a candidate PRE-FILTER only (cloud's registry is multi-tenant and must not be read whole, as the hub reads its own): every true match contains the `vault:<name>:` substring, so the pattern is a superset that can over-fetch but never miss, and the JS check discards the extras. `xvault:foo:read` and `account:<id>:vaults:foo:read` are exactly the strings that must survive it, and tests pin both. No undo window here, deliberately, and not to be confused with its sibling: the only verb that erases vault content is irreversible by construction (deleteAll + an R2 prefix purge), so the retype confirm IS the guard — exactly as the self-hosted twin has it. The 24-hour window belongs to ACCOUNT deletion (the A-train), which lands in the following commits and reuses this same teardown pair per owned vault rather than growing a second one. Idempotency: after a successful delete the ownership row is gone, so a second request gets the same neutral 403 not_owner an unknown or unowned vault gets — no existence oracle, and a re-delete never 500s. Also: the door descriptor now advertises vault_delete: true (an unadvertised working route ships dark), and scripts/staging-sweep.ts's header is corrected — a real teardown verb exists now, but that script still deletes only the D1 row, and its comment said so as if nothing else could. Recovered verbatim from a session that died at the usage limit before its first commit; reviewed, rebased onto main, and gated here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…A-3/A-4)
Wires A-1 (migration 0023's tombstone + read-time refusals) and A-2
(billing-teardown.ts), both of which have been deliberately inert, into the
lifecycle Aaron ratified: delete severs auth immediately, erases nothing for
24 hours, then converges.
New workers/identity/src/account-delete.ts:
- handleAccountDelete — admin-gated, retype-the-email confirm. deferBilling
FIRST (the reversible hold, so "no new charge posts after a delete is
requested" does not depend on the D1 write), then the tombstone
(conditional on deleted_at IS NULL, so concurrent requests cannot both
open a window), COMPENSATING the hold with resumeBilling if that write
fails. Then sessions dropped + live tokens revoked — belt, not braces:
A-1's chokepoints already refuse a tombstoned account everywhere, so the
account is dead the instant the tombstone commits. Then the notice email
carrying the undo link; delete_notice_sent_at is stamped only on a real
send. Undo token returned in the body too, so an API client with no inbox
can still offer undo.
- handleAccountDeleteUndo — authenticated by the mailed token ALONE (a
bearer/cookie is structurally impossible: A-1 refuses both for a
tombstoned account). GET (the emailed link) and POST (API). Resumes
billing BEFORE clearing the tombstone; reports resumeBilling's
already_canceled case plainly rather than claiming a restore it did not
get. Every unusable token gets one neutral answer — no oracle.
- runAccountDeleteSweep — past the window: teardownBilling, then per owned
vault callVaultDestroy + deleteVaultD1Rows (the SAME pair the single-vault
door drives, so there is one teardown path, not two), then the account
rows. A vault that fails to destroy defers the WHOLE account — the user
row must not be purged while a vault it owns still exists, since that row
is the only thing that still says whose it was. Per-account failure
isolation; deferral is never exposure, because the account stays
tombstoned and A-1 keeps refusing it.
Routes land on /account/delete, NOT a bare DELETE /account: run_worker_first
matches on path not method, and the bare /account is the SPA's own
Account-manager screen (route-manifest.ts SUBTREE_ONLY_PREFIXES).
Sweep rides the existing hourly drip tick in its own try — the window is a day
long, so hourly resolution is ample and a new cron pattern would have to be
added to two wrangler.toml [triggers] blocks for nothing.
Tests to follow in the next commit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
One describe per leg of the ratified lifecycle — request, window, undo,
convergence — so a regression names which promise broke.
Notable pins:
- the stored undo secret is a HASH, asserted against sha256Hex of the
returned token and asserted NOT to equal it;
- the request path calls deferBilling and ONLY deferBilling (goes red if
teardownBilling is ever pulled forward to request time), while the sweep
is the only place cancel + delete-customer happen;
- the severed bearer is dead on the very next call, with the exact
"account not found" body a missing row gets — A-1's chokepoint, now
reachable for the first time;
- a sweep one second before expiry finds nothing due and leaves every
artifact (the window is a promise, not a delay);
- every unusable undo token produces ONE body, asserted by collapsing five
of them into a Set of size 1;
- the sweep's destroy calls are checked for the per-vault admin mint and
audience, not merely that a fetch happened;
- a wedged vault defers the WHOLE account and a later pass converges without
re-destroying the vault that already went;
- a live account is invisible to the sweep (negative control), and one
wedged account does not block the queue;
- the sweep is driven through handleScheduled(DRIP_CRON, …), so the suite
cannot pass against a function no deployment calls.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ick doc Tried to advertise the whole-account delete door beside vault_delete and found the reason it can't be done here: `AccountCapabilities` is the SHARED door contract, a `file:` dep on parachute-hub's packages/door-contract that CI pins to a reviewed commit. Widening it is an upstream PR plus a pin bump, not something a cloud-side wiring branch gets to do — CLAUDE.md's shared-wire- contract rule. So the absence is documented at the descriptor and PINNED in conformance (account_delete toBeUndefined), which is what will fail, loudly and in the right place, the day the contract widens and nobody lights the cloud door up. The route works regardless; what it lacks is the advertisement. Also records the account-delete sweep on ops.ts's hourly-tick doc, and the mutation check behind it: replacing the runAccountDeleteSweep call in handleScheduled's drip branch with a no-op turns 'the sweep is actually ON the hourly cron' red (1 failed | 22 skipped), green again on restore. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, alerting, undo index
Adversarial review of the cloud#226 wiring. Seven code findings:
1. BLOCKER — the SWEEP believed a bare 200 from the vault DO and then deleted
the D1 rows, while the single-vault door validated the body hard. Those
rows are the ONLY record of whose bytes a vault held, so a 200 that is not
the DO's real reply (renamed route, service-binding quirk, a future handler
answering {ok:true}) would have orphaned the storage permanently: still
billed, no longer attributable, and beyond any retry because nothing left
would know to try. Extracted the door's judgement into vault-call.ts
readDestroyOutcome — literal destroyed===true plus a safe non-negative
integer r2_objects_deleted — and routed BOTH callers through it. A bad body
now defers the account exactly like a failed destroy. The asymmetry is the
point: a false negative costs one idempotent retry, a false positive is
unrecoverable.
3. ops.ts ran unguarded while the comment claimed three
independent try blocks. A permanently-throwing drip starved BOTH sweeps
forever — no downgrades applied, no deleted account ever purged. Wrapped;
the comment is now true.
4. users.delete_undo_hash had no index, and the undo lookup is unauthenticated
BY CONSTRUCTION (a tombstoned account cannot present a bearer or cookie),
so it was a full table scan any caller could drive at will. Migration 0024
adds a UNIQUE index — unique, not plain, because the lookup uses .first()
and two rows sharing a hash would restore an ARBITRARY account. Same
pattern as idx_users_drip_unsub_token (0008). 0024 was reserved for the
vault-delete train's PR-2a, which turned out to need no columns.
5. Hold placed + tombstone failed + resumeBilling ALSO failed left
cancel_at_period_end stuck true on a NON-deleted account with no tombstone,
so no sweep would ever revisit it — the user's subscription silently lapses
at the period boundary. Now raises an operator alert.
6. Permanent deferral was unbounded and invisible: billing converges on pass
one, so a wedged vault settles into an account paying nothing, absent from
the product, still holding data promised deleted. Added
delete_purge_attempts (0024) and an operator page past 48 passes (two days
of hourly ticks) — long enough for a transient fault to drain unattended,
short enough that the unmet promise cannot quietly become a month.
8. The module docstring claimed one neutral answer for invalid/unknown/expired
while expired deliberately gets 410. Documented the exception and why it
leaks nothing: reaching it requires having held the real token.
Also extracts ops.ts's alert helpers into ops-alerts.ts so surfaces other than
the health check can page without importing the cron router that imports them
back. ALERT_DEDUPE_MS re-exported; behavior unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… (cloud#240)
Coverage for the folds, each verified load-bearing by mutation rather than
assumed:
- the 200-that-isn't (finding 1/7): eight cases on the door and six on the
sweep — destroyed:false, absent, a string; r2_objects_deleted missing,
negative, fractional, stringly-typed; and an unparseable body. Mutating
readDestroyOutcome to trust any 200 turns 15 tests red across the two
files. These are the cases that would have orphaned R2 storage while
deleting the only rows that said whose it was.
- the guarded drip (finding 3): a live welcome-window arrival plus a
throwing sendDrip, asserting the deleted account still gets purged on the
same tick.
- the stuck hold (finding 5): deferBilling succeeds, the tombstone write is
blocked by a trigger, the release throws — asserts the operator page names
the account, the subscription, and that the account is NOT deleted.
- the deferral counter (finding 6): the two passes straddling the threshold
(quiet below, page above), the alert's content, hourly dedupe, and a
negative control that a converging account never accrues attempts.
TWO OF THESE TESTS WERE VACUOUS WHEN FIRST WRITTEN, and the mutation is what
caught it:
- the drip test seeded the DELETED account as the drip-eligible arrival, but
A-1 excludes a tombstoned row from every drip eligibility query, so the
sender was never called and removing the guard passed. Now seeds a
separate live user and asserts eligibleFor actually returns it first — a
control on the control.
- the stuck-hold test dropped the _mbsetupuser uni table, which requireAccount's own
read hits first; it never reached the code under test. Now blocks exactly
the tombstone UPDATE with a trigger.
Also retitles account-api's reclaim test (finding 2). It was called "the same
name can be created again" and certified that against a STUBBED vaultFetch —
no DO involved. Production does not have that property: idFromName maps the
reused name back to the same DO, whose in-memory destroyed latch 410s every
request while the instance stays resident. Filed as cloud#240 with probe
evidence; the test now claims only the D1 bookkeeping it actually proves.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #226 (the hosted vault-delete door) and completes A-3/A-4 of the
account-delete train coordinated on that issue's thread (A-1 landed as #232,
A-2's billing-teardown module was already inert-on-main). Also folds in a
full adversarial review pass and pins the resulting hardening with tests.
What
Single-vault delete (
DELETE /account/vaults/<name>) replaces the honest501 stub: retype-the-name confirm +
userOwnsVaultgate, then the vaultworker's real
POST /api/internal/destroy(DO storage + R2 prefix), then theidentity-side D1 cascade (tokens revoked, grants rewritten — not dropped, a
grants row spans every vault a client was approved for — vault/usage/snapshot
rows dropped). Order is load-bearing: storage teardown runs and is inspected
before any D1 write, so a transport error, non-2xx, or malformed body leaves
every identity row intact for retry. No undo window here, deliberately — the
verb is irreversible by construction, so the retype confirm is the guard.
Whole-account delete (A-3/A-4) wires A-1's tombstone + read-time refusals
and A-2's billing-teardown module into the ratified lifecycle: delete severs
auth immediately, erases nothing for 24 hours, then converges.
handleAccountDelete— admin-gated, retype-the-email confirm. Defersbilling first (so "no new charge posts" doesn't depend on the D1 write),
then the tombstone (conditional on
deleted_at IS NULL, so concurrentrequests can't both open a window), compensating with
resumeBillingifthe tombstone write fails. Sessions dropped + live tokens revoked as
belt-not-braces (A-1 already refuses a tombstoned account everywhere).
Undo token returned in the body as well as mailed, so an API client with no
inbox can still offer undo.
handleAccountDeleteUndo— authenticated by the mailed token alone (abearer/cookie is structurally impossible on a tombstoned account). Resumes
billing before clearing the tombstone; every unusable token gets one
neutral answer, no oracle.
runAccountDeleteSweep— past the window: teardown billing, then per ownedvault the same destroy+D1-cleanup pair the single-vault door uses (one
teardown path, not two), then the account rows. A vault that fails to
destroy defers the whole account — the user row must not be purged while a
vault it owns still exists. Rides the existing hourly drip cron.
Routes land on
/account/delete, not a bareDELETE /account— that path isthe SPA's own Account-manager screen. The door descriptor deliberately does
not advertise
account_delete: true:AccountCapabilitiesis the shareddoor contract pinned from parachute-hub's
door-contract, and widening it isan upstream PR, not something this branch gets to do. Pinned in conformance
(
account_deleteassertedtoBeUndefined) so the gap fails loudly, in theright place, the day the contract widens.
Review-fold hardening
An adversarial pass over the wiring above surfaced these, all fixed here:
200from the vault DO and deletedthe D1 rows on the strength of it, while the single-vault door validated
the body hard; those rows are the only record of whose bytes a vault held.
Extracted the judgement into
vault-call.ts'sreadDestroyOutcome(
destroyed === trueplus a safe non-negativer2_objects_deleted) androuted both callers through it. A bad body now defers the account exactly
like a failed destroy — a false negative costs one retry, a false
positive would have been unrecoverable.
try blocks; a permanently-throwing drip starved both sweeps forever. Now
wrapped, and the comment is true.
users.delete_undo_hashhad no index, and the undo lookup isunauthenticated by construction — a full table scan any caller could
drive at will. Migration
0024adds a unique index (the lookup uses.first(); two rows sharing a hash would restore an arbitrary account).cancel_at_period_endstucktrueon a non-deleted, non-tombstoned account no sweep would revisit —silent subscription lapse. Now raises an operator alert.
delete_purge_attempts(migration0024) and an operator page past 48passes (two days of hourly ticks).
expired tokens; expired deliberately 410s. Documented the exception.
Each fold is pinned by a mutation-verified test — two of them (the drip guard
and the stuck-hold alert) were vacuous as first written and only caught by
the mutation check; both are now controls-on-controls (see the last commit's
message for the mechanics).
Also extracts
ops.ts's alert helpers intoops-alerts.tsso non-cronsurfaces can page without importing the cron router back (behavior
unchanged,
ALERT_DEDUPE_MSre-exported).Test evidence
Local at 4ce7819: identity 1160 pass (40 files, vitest under workerd) ·
vault 423 pass + 1 todo (27 files) · control plane 177 pass · both
worker typechecks clean. (Local pool ran in single-worker mode — this box
storms loopback booting 40 workerd runtimes at once; CI runs the standard
invocation.)
Known gaps referenced, not fixed here
name 410-bricks until the DO evicts, because the
destroyedlatch isin-memory and
idFromNamemaps the reused name back to the same instance.Pre-existing since feat(vault): POST /api/internal/destroy — vault erasure (cloud#226 PR-1) #229 (PR-1); this branch widens who can reach it (every
tenant, not just operators) and calls it out explicitly. The account-api
reclaim test is retitled to claim only the D1 bookkeeping it actually
proves, pointing at Delete → recreate same vault name is a 410 brick until the DO evicts (destroyed latch + idFromName reuse) #240 for the DO-level gap.
handleCheckoutSessionCompleted,handleSubscriptionUpdated) don't checkdeletedAtand could wake atombstoned owner's vault DO during the 24h window.
billing-lifecycle.tsis untouched by this PR; A-3: Stripe webhooks bypass the deleted_at gate and can wake a tombstoned owner's vault DO #234 remains open and unaddressed.