feat(vault): POST /api/internal/destroy — vault erasure (cloud#226 PR-1) - #229
Merged
Conversation
…cloud#226 PR-1) The vault worker's destroy endpoint: close live WS sockets (1001), purge every R2 object under vault-<name>/ (attachments+snapshots+exports in one prefix pass via a new purgePrefix, generalized from purgeAttachments), deleteAlarm() then deleteAll() DO storage, and flag the warm instance so no later request on it can re-persist config or re-arm alarms. Dispatched ahead of ensureState() so the idempotent retry path never touches DO state. Independently useful today: an operator bearer can now tear down a vault via API instead of hand-written SQL. Auth is the pre-existing platform-vs-tenant gate (internalForbidden) — no new scheme. confirm:"<vault name>" is defense-in-depth at the internal seam, not the primary guard. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XLZtmuSs1RirWGMGyCB1QB
…rencyWhile, pagination pin Two fixes from PR #229 review, folded into the same branch/rc (not a new PR): 1. [MEDIUM] `webSocketMessage`/`webSocketClose`/`webSocketError`/`alarm()` now check `this.destroyed` explicitly, instead of relying on it being emergent (ensureStateForWake's warm fast-path never re-arming, wiped storage reading null). Durability hardening on the primitive the account-delete cascade extends, not a currently-observable bug — verified by mutation that the guard is presently unreachable dead code (deleteAll leaves no `config` key, so each method's own pre-existing null check already short-circuits), so no test asserts it; asserting "nothing bad happens" here would pass identically with the guard deleted. Also wrapped handleDestroy's WS-close→purge→deleteAlarm→deleteAll→flag sequence in `ctx.blockConcurrencyWhile` — purgePrefix awaits multiple R2 round-trips, and DO event interleaving could otherwise let a concurrent write land between the purge finishing and deleteAll clearing storage (the same hazard handleSnapshot's CONCURRENCY NOTE already names, for a lower-stakes verb). `this.destroyed = true` stays LAST inside the block — setting it earlier would let a failed purge skip re-purge on retry, an R2 leak worse than what this closes. 2. [LOW] Extracted `purgePrefix` from a private method into a module-level exported function (bucket param explicit) — mirrors `pruneExportTarballs`'s pattern in export.ts, letting the cursor/chunking loop be pinned with a fake bucket instead of 1500 real R2 objects or mutating the DO's shared env binding. New test: 1500 keys across 2 pages, watched red (mutated the cursor assignment to never continue → `listCalls` 1 not 2) before restoring. Cold idempotence (destroy → evict → destroy again) requested on review is SKIPPED and documented why: `__simulateEviction` only drops in-memory subscription state, never touches `destroyed` — so it can't produce a test distinct from the existing warm-instance idempotent-retry test. Gates: typecheck clean; full suite 409 passed | 1 todo (410) across 27 files (was 408/1/409 before this fold — net +1 from the new pagination test). No orphaned workerd before or after. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XLZtmuSs1RirWGMGyCB1QB
unforced
added a commit
that referenced
this pull request
Jul 28, 2026
…e-fix commit Adds the required coverage for the account-delete substrate (8e72fe6), one test (or set) per chokepoint: the account bearer gate (account-api.ts + account-mcp-http.ts), the session JOIN (via the account-token mint path), both password-login paths, magic-link request/consume + the 2FA divert, the signup-collision degrade (verification only — no new code), the drip's three eligibility queries, and the billing/usage/snapshot sweep enumerations. Verified red-then-green: reverted the 12 chokepoint src files (kept migration 0023) to the pre-fix commit, ran all 10 touched test files — 14 of 15 new tests failed for the stated reason (the 15th, signup collision, is a verification test with no new code to break, and passed as expected both before and after). Restored the fix; same 10 files now 463/463 passing. rc.128 -> rc.130 (re-bumped after rebasing onto main: #229 landed rc.128 first, #230 holds rc.129 ahead of this PR in the merge queue).
unforced
added a commit
that referenced
this pull request
Jul 28, 2026
…e-fix commit Adds the required coverage for the account-delete substrate (8e72fe6), one test (or set) per chokepoint: the account bearer gate (account-api.ts + account-mcp-http.ts), the session JOIN (via the account-token mint path), both password-login paths, magic-link request/consume + the 2FA divert, the signup-collision degrade (verification only — no new code), the drip's three eligibility queries, and the billing/usage/snapshot sweep enumerations. Verified red-then-green: reverted the 12 chokepoint src files (kept migration 0023) to the pre-fix commit, ran all 10 touched test files — 14 of 15 new tests failed for the stated reason (the 15th, signup collision, is a verification test with no new code to break, and passed as expected both before and after). Restored the fix; same 10 files now 463/463 passing. rc.128 -> rc.130 (re-bumped after rebasing onto main: #229 landed rc.128 first, #230 holds rc.129 ahead of this PR in the merge queue).
unforced
added a commit
that referenced
this pull request
Jul 28, 2026
…d-time refusal chokepoints (#232) * fix(identity): account-delete substrate (A-1) — migration 0023 + read-time refusal chokepoints Adds the nullable deleted_at/delete_undo_hash/delete_notice_sent_at columns (migration 0023, claiming the slot ahead of the vault-delete train's PR-2a — see cloud#226) and wires the read-time refusal every account-acting surface needs so a tombstoned row can never still act: the account bearer gate (account-api.ts requireAccount, account-mcp-http.ts authenticate), the session JOIN (sessions.ts findActiveSession), both password-login paths (console.ts, oauth-authorize.ts), magic-link request + consume + the 2FA divert (auth-handlers.ts), the onboarding drip's three eligibility queries, and the billing/usage/snapshot sweep enumerations. Deliberately inert: no route sets these columns yet (A-3 owns the delete endpoint, A-4 the undo + convergence sweep). Mirrors migration 0011's suspended_at no-oracle posture rather than inventing a second style, except where deletion is stronger: requireAccount/authenticate answer a deleted owner with the exact "account not found" body a missing row gets, not the distinguishable account_suspended a suspended owner gets. Tests to follow in a separate commit. * test(identity): A-1 chokepoint tests + rc.130 — watched red on the pre-fix commit Adds the required coverage for the account-delete substrate (8e72fe6), one test (or set) per chokepoint: the account bearer gate (account-api.ts + account-mcp-http.ts), the session JOIN (via the account-token mint path), both password-login paths, magic-link request/consume + the 2FA divert, the signup-collision degrade (verification only — no new code), the drip's three eligibility queries, and the billing/usage/snapshot sweep enumerations. Verified red-then-green: reverted the 12 chokepoint src files (kept migration 0023) to the pre-fix commit, ran all 10 touched test files — 14 of 15 new tests failed for the stated reason (the 15th, signup collision, is a verification test with no new code to break, and passed as expected both before and after). Restored the fix; same 10 files now 463/463 passing. rc.128 -> rc.130 (re-bumped after rebasing onto main: #229 landed rc.128 first, #230 holds rc.129 ahead of this PR in the merge queue).
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
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.
Summary
PR-1 of the vault-delete train (#226). Builds the piece that actually erases data:
POST /vault/<name>/api/internal/destroy. Independently useful today — with it, an operator bearer can tear down a vault via API instead of hand-written SQL (yesterday's staging reclamation needed exactly this).Scope:
workers/vaultonly. No identity changes, nothing user-facing.What it does (in order)
vault-<name>/— one prefix pass covers all three families (attachments/,snapshots/,exports/), via a newpurgePrefixgeneralized from the existingpurgeAttachments. The trailing slash is load-bearing (vault-foo/must not matchvault-foobar/) — pinned by a dedicated test.ctx.storage.deleteAlarm()thenctx.storage.deleteAll()— in that order, sincedeleteAll()does not clear a pending alarm, and this DO arms transcription/embedding alarms that must not fire against a destroyed vault.destroyedflag. Every later request on this warm instance answers 410 and writes nothing.{ destroyed: true, r2_objects_deleted: n }.Authorization
Unchanged — the pre-existing
internalForbiddengate (first-partyclient_id+ admin scope, or theVAULT_AUTH_TOKENoperator bearer). No new scheme.confirm:"<vault name>"(exact match, lowercased) in the body is defense-in-depth at the internal seam, not the primary guard.A deliberate dispatch-ordering choice
/api/internal/destroyis dispatched at the very top offetch(), beforeensureState()— not inside the normal/api/internal/*block. This is required, not stylistic:ensureState()'smaybeArmEmbeddingBackfillre-arms the embedding alarm on every wake unless the backfill is already known-done, which would re-fire the very alarm this endpoint just cleared. Running the destroy route (including idempotent retries) ahead ofensureState()is what keeps a warm-but-destroyed instance from resurrecting any state. Every other route on an already-destroyed warm instance also short-circuits to 410 ahead ofensureState(), for the same reason.What residue remains (said explicitly, not implied away)
The DO's
idFromNamemapping is permanent — a later stray request re-materializes an empty-schema DO (~KB, zero tenant data). That's the same property any never-created vault name already has; this PR does not (and cannot, from the vault worker alone) erase the identity-side ownership record or D1 rows. That's later PRs in the train.Tests (workerd,
workers/vault/test/destroy.test.ts, 6 new tests)Seeded a vault with a note, an attachment, a snapshot, and an export so all three R2 prefixes are populated:
vault-<name>/gone, count matches.vault-<name>x/survives — the test most likely to be skipped, included deliberately.runInDurableObjectthat DO storage is empty and no alarm got re-armed.client_id) → the existinginternal_config_forbidden403.Each of the 6 tests was verified red on a stubbed-out version of the fix (documented failure per test, then restored) before being trusted green.
Gates
bun run typecheck(root +workers/vault): clean.cd workers/vault && bun x vitest run: 27 files, 408 passed, 1 todo (pre-existing todo, unrelated).workerdprocesses left running.bun scripts/smoke-staging.tswas not run (per instructions — it creates live staging debris).🤖 Generated with Claude Code
https://claude.ai/code/session_01XLZtmuSs1RirWGMGyCB1QB