Repository navigation
Re-key the open conversation when the DEK moves, instead of stranding it - #20
Merged
Merged
Conversation
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>
Closed
Deploying health-dash-main with
|
| 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 |
Deploying health-dash-dev with
|
| 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 |
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.
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 toucheschat-{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: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-storelatches 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:
setAsideUnreadableHistoryand thex-replace-unreadableroute are still the only answer once the key is provably gone. This is the prevention, and the two do not overlap.The fix
chatOpenedWithKeyrecords the DEK the conversation now in memory was decrypted out of storage under. On ahydrate()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.tsasks 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.tsasks 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
TEST_BACKEND=node44s)oxlint --deny-warningsclean;tsc --noEmitexit 0chat-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.chatOpenedWithKeyappears nowhere onorigin/dev, insrcor intests, 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, andtests/unit/zz-scratch-probe.test.ts— aconsole.logprobe assertingexpect(true).toBe(true). Its round-trip test is taken verbatim; the probe is not here, andgit ls-files apps/lexitar/tests/unit | grep -c zz-scratch-probereads0.🤖 Generated with Claude Code