Skip to content

Re-key the open conversation when the DEK moves, instead of stranding it - #20

Merged
pablo-tech merged 1 commit into
devfrom
fix/chat-rekey-on-dek-change
Oct 24, 2026
Merged

pablo-tech merged 1 commit into
devfrom
fix/chat-rekey-on-dek-change

Conversation

@pablo-tech

Copy link
Copy Markdown
Contributor

What is wrong today

An in-app vault key rotation re-keys the vault blob and nothing else. It is reached twice — revoking an active support principal, and App's boot-time resume of a deferred rotation — and in both cases the selected client never changes and nothing in that path touches chat-{id}.enc.

So the conversation stays encrypted under the key just discarded, and the tab holding it is the last place its plaintext exists. hydrate()'s early return is what threw that copy away:

if (!id || !key || chatLoadedForId === id) return;

The effect re-ran with a new key and did nothing with it. Every later reader — the next session, a second tab — then gets ciphertext it cannot open, chat-store latches the client unreadable, and from then on every save of that conversation is refused behind a banner. That is the fault that stranded seven histories in the 2026-10-06 and 2026-10-07 rotations.

W71 shipped the recovery for a blob already stranded, and that recovery is untouched here: setAsideUnreadableHistory and the x-replace-unreadable route are still the only answer once the key is provably gone. This is the prevention, and the two do not overlap.

The fix

chatOpenedWithKey records the DEK the conversation now in memory was decrypted out of storage under. On a hydrate() for the same client with a different key, the conversation is written back under the new key rather than returned from. It deliberately does not re-read: the stored blob is under the discarded key, so a load would only prove it unopenable and latch the store's refusal to write over it.

Only a conversation that came out of storage may be re-keyed. The marker is null until a load actually restores one, so a placeholder thread can never overwrite a blob this tab has not read — the one way a prevention could cost more than the fault it prevents. It is not $state: nothing renders from it, and a reactive read inside the hydrate effect would make the effect depend on the value it writes.

Tests, at the two layers that answer different questions

tests/unit/chat-rekey-on-dek-change.test.ts asks whether the conversation survives, and asks it of the store rather than of the in-memory thread list: real WebCrypto, a fake /api/chat-history/{id} with etag and 412 semantics, and a second module instance standing in for the next reader, which holds only the new key. Its second case pins the other direction — a tab that never opened a stored conversation writes nothing at all.

tests/unit/chat-thread-session.test.ts asks whether the decision is right, in the three directions it can be got wrong: a changed key writes, an unrestored conversation does not, and an unchanged key does not. Cheap against the module's existing fake store, and it is where a future early return would be reintroduced. :67 — loads once per client id — stays green, the same key object both times.

Verification

  • 346 test files / 4273 tests pass on both backends (Miniflare 131s, TEST_BACKEND=node 44s)
  • oxlint --deny-warnings clean; tsc --noEmit exit 0
  • Proved to detect the defect, not merely to pass beside it: with the source change reverse-applied, the round-trip case fails at chat-rekey-on-dek-change.test.ts:106 — the next reader's restored titles read empty — and passes with it. A rotation test that never saw the bug proves nothing.
  • chatOpenedWithKey appears nowhere on origin/dev, in src or in tests, so the premise was re-measured against the live branch rather than taken from the plan.

Replaces #4

#4 carries this change and two things that should not land: a base predating chatUnreadable, so its patch no longer applies, and tests/unit/zz-scratch-probe.test.ts — a console.log probe asserting expect(true).toBe(true). Its round-trip test is taken verbatim; the probe is not here, and git ls-files apps/lexitar/tests/unit | grep -c zz-scratch-probe reads 0.

🤖 Generated with Claude Code

An in-app vault key rotation re-keys the vault blob and nothing else. It is reached
twice -- revoking an active support principal, and App's boot-time resume of a
deferred rotation -- and in both cases the selected client never changes and nothing
in that path touches `chat-{id}.enc`.

So the conversation stays encrypted under the key that was just discarded, and the
tab holding it is the last place its plaintext exists. `hydrate()`'s early return
(`if (!id || !key || chatLoadedForId === id) return;`) is what threw that copy away:
the effect re-ran with a new key and did nothing with it. Every later reader -- the
next session, a second tab -- then gets ciphertext it cannot open, chat-store latches
the client unreadable, and from then on every save of that conversation is refused
behind a banner. That is the fault that stranded seven histories in the 2026-10-06
and 2026-10-07 rotations.

W71 shipped the recovery for a blob already stranded, and that recovery stays exactly
as it is: `setAsideUnreadableHistory` and the `x-replace-unreadable` route are still
the only answer once the key is provably gone. This is the prevention, and the two do
not overlap.

THE FIX. `chatOpenedWithKey` records the DEK the conversation now in memory was
decrypted out of storage under. On a `hydrate()` for the same client with a different
key, the conversation is written back under the new key rather than returned from. It
deliberately does not re-read: the stored blob is under the discarded key, so a load
would only prove it unopenable and latch the store's refusal to write over it.

Only a conversation that CAME OUT OF STORAGE may be re-keyed. The marker is null
until a load actually restores one, so a placeholder thread can never overwrite a
blob this tab has not read -- the one way a prevention could cost more than the fault
it prevents. It is not `$state`: nothing renders from it, and a reactive read inside
the hydrate effect would make the effect depend on the value it writes.

TESTS, AT THE TWO LAYERS THAT ANSWER DIFFERENT QUESTIONS.

`tests/unit/chat-rekey-on-dek-change.test.ts` asks whether the conversation survives,
and asks it of the STORE rather than of the in-memory thread list: real WebCrypto, a
fake `/api/chat-history/{id}` with etag and 412 semantics, and a second module
instance standing in for the next reader, which holds only the new key. Its second
case pins the other direction -- a tab that never opened a stored conversation writes
nothing at all. Taken from #4, whose change this is; its base predates
`chatUnreadable` so the patch no longer applies, and its `zz-scratch-probe.test.ts`
(a `console.log` asserting `expect(true).toBe(true)`) is deliberately not here.

`tests/unit/chat-thread-session.test.ts` asks whether the DECISION is right, in the
three directions it can be got wrong: a changed key writes, an unrestored
conversation does not, and an unchanged key does not. Cheap against the module's
existing fake store, and it is where a future early return would be reintroduced.
`:67` -- load once per client id -- stays green, the same key object both times.

VERIFIED. 346 test files / 4273 tests pass on both backends (Miniflare 131s,
TEST_BACKEND=node 44s); `oxlint --deny-warnings` clean; `tsc --noEmit` exit 0.

Proved to detect the defect rather than only to pass beside it: with the source
change reverse-applied, the round-trip case fails at
`chat-rekey-on-dek-change.test.ts:106` -- the next reader's restored titles read
empty -- and passes with it. A rotation test that never saw the bug proves nothing.
`chatOpenedWithKey` appears nowhere on `origin/dev`, in `src` or in `tests`, so the
premise was re-measured against the live branch rather than taken from the plan.

Replaces #4.

Signed-off-by: Pablo Rodriguez <contact@promontory.studio>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying health-dash-main with  Cloudflare Pages  Cloudflare Pages

Latest commit: 53b76a3
Status: ✅  Deploy successful!
Preview URL: https://2be5c7bb.health-dash-prod.pages.dev
Branch Preview URL: https://fix-chat-rekey-on-dek-change.health-dash-prod.pages.dev

View logs

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying health-dash-dev with  Cloudflare Pages  Cloudflare Pages

Latest commit: 53b76a3
Status: ✅  Deploy successful!
Preview URL: https://ece3fcbd.health-dash-aex.pages.dev
Branch Preview URL: https://fix-chat-rekey-on-dek-change.health-dash-aex.pages.dev

View logs

@pablo-tech
pablo-tech merged commit 59c4c89 into dev Oct 24, 2026
17 checks passed
@pablo-tech
pablo-tech deleted the fix/chat-rekey-on-dek-change branch October 24, 2026 14:32
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