From ddba8b22fd2f2ee55a113cc868daa4067010c148 Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Tue, 5 May 2026 00:37:32 +0000 Subject: [PATCH] fix(messaging): revert sharedSecretCache reuse in getMessageHistory (regression hotfix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Batch 8 (commit 52becd7, PR #74) added a module-level shared-secret cache read-path in getMessageHistory to avoid re-deriving ECDH on every poll tick. Post-merge CI showed a real Firefox/WebKit messaging E2E regression: - Run 25306048649 (main, dd83ac7, pre-batch-8) at 07:14 UTC: all green - Run 25332113434 (main, 0e20fb3, post-batch-8) at 17:04 UTC: firefox-msg failed with messages-not-visible timeouts on real-time-delivery, encrypted-messaging, and message-delete-placeholder specs - Subsequent PR runs reproduced the same pattern, with webkit-msg also failing on the rebased PR #70 branch The cache reuse is symmetric in design (same pattern as sendMessage's cache, which has been there for months) but interacts badly with Firefox/WebKit reload + tab semantics. The cached CryptoKey reference appears to behave differently across these browsers when getCurrentKeys returns a freshly-deserialized key from IndexedDB after a page reload — the invalidation guard `cachedSenderPrivateKey !== currentKeys.privateKey` was likely thrashing or holding a stale reference. Hotfix: getMessageHistory now derives a fresh shared secret per call, matching pre-batch-8 behavior. The cache infrastructure remains in place (sendMessage still uses it as before) — this revert is read-path only. Cost: ~6 ECDH derivations/min under polling — measurable but not user-perceivable. A browser-safe instance-aware cache key can return as a separate change once the Firefox CryptoKey-reference behavior is properly understood. Verification: - pnpm run type-check: clean - pnpm run lint: clean - pnpm test: 3237/3237 pass Refs: #74 (introduced regression), runs 25306048649 (last green), 25332113434 (first red post-batch-8) Co-Authored-By: Claude Opus 4.7 (1M context) --- src/services/messaging/message-service.ts | 55 +++++++++++------------ 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/src/services/messaging/message-service.ts b/src/services/messaging/message-service.ts index 65e17a0..c271a01 100644 --- a/src/services/messaging/message-service.ts +++ b/src/services/messaging/message-service.ts @@ -771,37 +771,32 @@ export class MessageService { }; } - // Reuse the module-level sharedSecretCache so a polling loop or a - // refresh of the same conversation doesn't re-run ECDH derivation - // every call. ECDH is computationally cheap individually but compounds - // at ~6/min under polling. Invalidate the cache when the sender's - // private key identity changes (e.g. after key rotation / re-derive). - if (cachedSenderPrivateKey !== currentKeys.privateKey) { - sharedSecretCache.clear(); - cachedSenderPrivateKey = currentKeys.privateKey; - } - let sharedSecret = sharedSecretCache.get(otherParticipantId) ?? null; - if (!sharedSecret) { - logger.debug('Importing other user public key via crypto.subtle'); - const otherPublicKeyCrypto = await crypto.subtle.importKey( - 'jwk', - otherPublicKey, - { - name: 'ECDH', - namedCurve: 'P-256', - }, - false, - [] - ); - logger.debug('Other user public key imported successfully'); + // Derive a fresh shared secret per call. An earlier batch reused the + // module-level sharedSecretCache here (matching sendMessage's pattern) + // but it caused a real Firefox/WebKit messaging E2E regression — the + // cached CryptoKey reference behaves differently across reload + tab + // contexts on those browsers, and polling could see stale or mismatched + // shared secrets. Reverted in hotfix until a browser-safe instance-aware + // cache key is designed. Fresh derivation is ~6 ECDH derivations/min + // under polling — measurable but not user-perceivable. + logger.debug('Importing other user public key via crypto.subtle'); + const otherPublicKeyCrypto = await crypto.subtle.importKey( + 'jwk', + otherPublicKey, + { + name: 'ECDH', + namedCurve: 'P-256', + }, + false, + [] + ); + logger.debug('Other user public key imported successfully'); - // Derive shared secret using our derived private key - sharedSecret = await encryptionService.deriveSharedSecret( - currentKeys.privateKey, - otherPublicKeyCrypto - ); - sharedSecretCache.set(otherParticipantId, sharedSecret); - } + // Derive shared secret using our derived private key + const sharedSecret = await encryptionService.deriveSharedSecret( + currentKeys.privateKey, + otherPublicKeyCrypto + ); // Log key fingerprints for E2E diagnostics (debug-suppressed in prod) const otherKeyFingerprint = otherPublicKey?.x?.slice(0, 8) ?? 'null';