chat-delegate: DeleteRequest rewrites the whole key index unconditionally, and an unparseable index silently becomes empty

[sanity]

Found while reviewing #587. Delegate-WASM change — deliberately not fixed there, because re-keying the delegate to fix a latent handler bug mints another generation, which is the exact failure mode #586 is about. This should land with the next delegate bump that happens for other reasons.

1. handle_delete_request rewrites the key index unconditionally

delegates/chat-delegate/src/handlers.rs::handle_delete_request calls set_key_index even when the key was not in the index. handle_store_request and the CAS-store path both rewrite only when adding a genuinely new key. So a no-op delete still triggers a whole-index rewrite.

2. get_key_index turns an unparseable index into an empty one

…from_reader(...).ok().unwrap_or_default()

An index secret that exists but fails to parse becomes an empty KeyIndex. Combined with (1), one unreadable index read during a delete writes an empty index over the real one: every room key vanishes from ListResponse, the values remain on disk but are unreachable, and there is no error. Silent, total, permanent.

The exposure is not new — handle_store_request uses the same get_key_index when adding a key — but until #587 the client had never sent a DeleteRequest at all, so the delete path has never executed in production.

Suggested fixes

• Make the delete a no-op on the index when the key is not present, matching Store's shape.
• Make get_key_index distinguish absent (legitimately empty) from corrupt (refuse rather than silently truncate). A corrupt index should fail the request loudly, not erase the user's keys.

Client-side mitigation already in place

#587's branch only sends the DeleteRequest when the marker is believed present, reading the delegate-seeded tri-state (so it works in the deployed app, where localStorage throws). That reduces the unconditional rewrite from once per migrating user to once per genuinely-interrupted migration. It does not address either bug above.

[AI-assisted - Claude]

[sanity]

The identity-destruction consequence of this issue's empty-index behaviour is written up on #588: an unparseable index makes rooms invisible to ListRequest while they stay readable by direct key, which routes the loader into the legacy probe and ends with an older delegate generation's identity CAS-written over the stored one. self_sk is unrecoverable.

Verified end to end, with the terminal save step executed rather than reasoned about. The mitigation proposed there (make reconcile_room_present consult ROOM_IDENTITY_SOURCE_RANKS instead of unconditionally keeping local) closes the loss regardless of whether this issue is fixed — worth having independently, since fixing this one re-keys the delegate.

[AI-assisted - Claude]

[sanity]

Checked whether quota exhaustion can corrupt the index. It cannot — recording the negative so nobody re-runs it.

Checked in freenet-core at 9307ddff:

• store_secret (crates/core/src/wasm_runtime/secrets_store/store.rs:605) computes the net footprint and returns QuotaExceeded before any filesystem write, so a refusal leaves the prior value byte-for-byte intact.
• The write is atomic by construction — tmp path, fsync, rename — so neither a refusal, a crash, nor a kill can leave a torn value. There is no partial-write path that could produce an unparseable index.

So the empty-index behaviour above stays a code-read finding with no established field trigger. I had hypothesised that a user at the per-user quota ceiling (measured: one real user at 3,998.5 KiB of 4,096 KiB, four room copies across generations 21-25) might produce truncated index writes. That mechanism does not exist.

One genuine non-atomicity found while looking, worth recording but not a trigger. handle_store_request writes the VALUE (handlers.rs:192) and then the INDEX (:209), and only the index write propagates failure (set_key_index(...)?). A quota refusal on the index write after a successful value write yields the same precondition — value on disk, not listed — with no corruption involved.

The window is close to measure-zero, though: the value write is charged its full size plus .keys registry growth while the index write is charged only its ~37-byte delta, so free space would have to land within ~37 bytes of exactly the value size. Both shapes that would widen it close themselves — a shrinking overwrite passes the quota check trivially, and an overwrite's key is already in the index so if !key_index.keys.contains(&key) skips the index write entirely. Still worth making the two writes fail as a unit if this code is touched for other reasons.

Not chased, so not cleared: whether remove_secret can fail silently after its index entry is dropped, and whether any generation-to-generation migration path copies values without their index. Either would be a real trigger if it existed.

[AI-assisted - Claude]