From c77ab346d39d0c071ec3bfc3c87de6966fb5f3b3 Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Sat, 18 Jul 2026 16:18:27 -0400 Subject: [PATCH] fix(translation): address review findings Follow-up hardening from two review passes on the on-device translation feature: - Privacy (HIGH): the translation cache is decrypted message plaintext, but logout did not clear it (unlike the search index), leaving up to 300 cleartext bodies in localStorage on shared devices. Add clearTranslationCache() and call it from both logout paths (logoutClient and the server-forced SessionLoggedOut handler). - Edited messages (MEDIUM): the cache key was eventId:target with no content dependence, so an edit reused the pre-edit translation. Fold a content fingerprint into the key, and re-arm the auto-translate one-shot when the text changes. - Settings (LOW): coerce a persisted translateTargetLang to a supported curated code so the hook never targets a language the engine can't produce (previously only the UI clamped it). - Chinese (LOW): restore canonical BCP-47 case (zh-Hant / zh-Hans) at the Translator API boundary, since normalizeLang lowercases the script subtag for internal keys. Co-Authored-By: Claude Opus 4.8 --- src/app/hooks/useMessageTranslation.ts | 14 ++++++---- src/app/pages/client/ClientRoot.tsx | 4 +++ src/app/state/settings.ts | 9 ++++--- src/app/state/translation.test.ts | 13 +++++++--- src/app/state/translation.ts | 30 ++++++++++++++++++++-- src/app/utils/translation/chromeEngine.ts | Bin 3537 -> 3981 bytes src/client/initMatrix.ts | 3 +++ 7 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/app/hooks/useMessageTranslation.ts b/src/app/hooks/useMessageTranslation.ts index b67c1752a..872ced76f 100644 --- a/src/app/hooks/useMessageTranslation.ts +++ b/src/app/hooks/useMessageTranslation.ts @@ -49,7 +49,9 @@ export const useMessageTranslation = (eventId: string, text: string): MsgTransla const [active, setActive] = useAtom(msgTranslationActiveAtomFamily(eventId)); const [cache, setCache] = useAtom(translationCacheAtom); - const key = makeCacheKey(eventId, targetLang); + // The key includes a fingerprint of `text`, so an edit (same event id, new + // body) misses the old entry and re-translates instead of showing stale text. + const key = makeCacheKey(eventId, targetLang, text); const cached = findTranslation(cache, key); const [status, setStatus] = useState('idle'); @@ -72,11 +74,13 @@ export const useMessageTranslation = (eventId: string, text: string): MsgTransla // Auto-translate: when enabled, flip `active` on for foreign messages whose // model is already downloaded (no gesture/download needed). Messages needing // a first-time download keep the manual chip (which provides the gesture). - const autoTried = useRef(false); + // Store the text we last auto-tried so an edit (new text) re-triggers auto + // detection instead of being suppressed by a one-shot flag. + const autoTriedText = useRef(undefined); useEffect(() => { - if (!supported || !autoTranslate || active || cached || autoTried.current || !eventId) return; - if (!text.trim()) return; - autoTried.current = true; + if (!supported || !autoTranslate || active || cached || !eventId) return; + if (!text.trim() || autoTriedText.current === text) return; + autoTriedText.current = text; let alive = true; (async () => { const from = await engine.detect(text); diff --git a/src/app/pages/client/ClientRoot.tsx b/src/app/pages/client/ClientRoot.tsx index 14c17e608..e4925bfcb 100644 --- a/src/app/pages/client/ClientRoot.tsx +++ b/src/app/pages/client/ClientRoot.tsx @@ -32,6 +32,7 @@ import { startClient, } from '../../../client/initMatrix'; import { deleteSearchCacheDatabase } from '../../utils/searchCache'; +import { clearTranslationCache } from '../../state/translation'; import { SplashScreen } from '../../components/splash-screen'; import { ServerConfigsLoader } from '../../components/ServerConfigsLoader'; import { CapabilitiesProvider } from '../../hooks/useCapabilities'; @@ -162,6 +163,9 @@ const useLogoutListener = (mx?: MatrixClient) => { // change) — the manual logout path already does, but this path didn't, so // the plaintext survived on disk (and persist() makes it non-evictable). await deleteSearchCacheDatabase(); + // The message-translation cache also holds decrypted plaintext — wipe it + // on server-forced logout too. + clearTranslationCache(); // Remove only the session credential keys — NOT settings, drafts, and // other preferences (N98). The SDK's IndexedDB stores are cleared above; // window.localStorage.clear() is reserved for the explicit reset path. diff --git a/src/app/state/settings.ts b/src/app/state/settings.ts index 6d8b94d83..1845e1956 100644 --- a/src/app/state/settings.ts +++ b/src/app/state/settings.ts @@ -1,4 +1,5 @@ import { atom } from 'jotai'; +import { isSupportedTargetLang } from '../utils/translation/langUtils'; const STORAGE_KEY = 'settings'; export type DateFormat = @@ -409,10 +410,12 @@ export const getSettings = (): Settings => { saved.ringtoneId === 'none' ? saved.ringtoneId : defaultSettings.ringtoneId, - // Coerce persisted target language to a non-empty string; anything else - // (missing/wrong type) falls back to the default. + // Coerce persisted target language to a curated, supported code; anything + // else (missing/wrong type/unknown) falls back to the default so the hook + // never targets a language the engine can't produce. translateTargetLang: - typeof saved.translateTargetLang === 'string' && saved.translateTargetLang.trim() + typeof saved.translateTargetLang === 'string' && + isSupportedTargetLang(saved.translateTargetLang) ? saved.translateTargetLang : defaultSettings.translateTargetLang, autoTranslate: diff --git a/src/app/state/translation.test.ts b/src/app/state/translation.test.ts index ad0402547..85ca3d46f 100644 --- a/src/app/state/translation.test.ts +++ b/src/app/state/translation.test.ts @@ -18,9 +18,16 @@ const entry = (key: string, translated = `t-${key}`, fromLang = 'de') => ({ fromLang, }); -test('makeCacheKey: eventId + normalized target', () => { - assert.equal(makeCacheKey('$abc', 'en-US'), '$abc:en'); - assert.equal(makeCacheKey('$abc', 'EN'), '$abc:en'); +test('makeCacheKey: eventId + normalized target + content fingerprint', () => { + // Same event + target + text is stable and prefixed by event:normalizedTarget. + const k1 = makeCacheKey('$abc', 'en-US', 'hola'); + const k2 = makeCacheKey('$abc', 'EN', 'hola'); + assert.equal(k1, k2); + assert.ok(k1.startsWith('$abc:en:')); + // Different body (an edit) => different key, so a stale translation misses. + assert.notEqual(makeCacheKey('$abc', 'en', 'hola'), makeCacheKey('$abc', 'en', 'adios')); + // Missing text arg still yields a stable key. + assert.ok(makeCacheKey('$abc', 'en').startsWith('$abc:en:')); }); test('addTranslation: prepends, newest first', () => { diff --git a/src/app/state/translation.ts b/src/app/state/translation.ts index 0a2bffc3a..b5858f80f 100644 --- a/src/app/state/translation.ts +++ b/src/app/state/translation.ts @@ -36,8 +36,21 @@ export const translationCacheAtom = atom( }, ); -export const makeCacheKey = (eventId: string, targetLang: string): string => - `${eventId}:${normalizeLang(targetLang)}`; +// Small, fast, non-cryptographic string hash — just enough to detect that a +// message's text changed (an edit reuses the same event id), so the key below +// invalidates a stale translation of pre-edit text. Kept bitwise-free (and thus +// within safe-integer range via the modulus) to satisfy the no-bitwise lint. +const HASH_MOD = 2147483647; // 2^31 - 1 +const fingerprint = (text: string): string => { + let h = 0; + for (let i = 0; i < text.length; i += 1) { + h = (h * 31 + text.charCodeAt(i)) % HASH_MOD; + } + return h.toString(36); +}; + +export const makeCacheKey = (eventId: string, targetLang: string, text = ''): string => + `${eventId}:${normalizeLang(targetLang)}:${fingerprint(text)}`; /** Look up a cached translation by key (returns undefined if absent). */ export const findTranslation = ( @@ -64,3 +77,16 @@ export const addTranslation = ( // (which flips it) and the inline body render (which reacts to it). Keyed by // event id so each message has its own independent state. export const msgTranslationActiveAtomFamily = atomFamily((_eventId: string) => atom(false)); + +/** + * Wipe the persisted translation cache. Called on logout — cached entries are + * decrypted message plaintext, so they must not survive a session on a shared + * device (mirrors the search-index wipe). + */ +export const clearTranslationCache = (): void => { + try { + localStorage.removeItem(STORAGE_KEY); + } catch { + /* localStorage unavailable — nothing to clear */ + } +}; diff --git a/src/app/utils/translation/chromeEngine.ts b/src/app/utils/translation/chromeEngine.ts index e903993ab7d3cf95cd9896c1fcdf3c9188c293bb..1e3627f27b110e5781baf2769577a522f6c313b4 100644 GIT binary patch delta 475 zcmZ`!%}T>S5Jss7V?gjGJvg^yu|F3eYu3 z`Xs)Qo3s|8IH!-9?{B`Vi?gqwGH*7aN7{^Ks;2lTY1D&MzoLmHBZEy4I*B4=1dffG zSm3O0?^NTgHVvFvm$f6;c!VBhHgMMb6ozY_q7@mzJo~P1k z1Nb(<*fPYDG^yALZadxj#bs&dnv`+(+c<|j7Q?Tcc<`5*y?$H!$-6)Nsv!D2gE1ycq~EXfs3Qrf3qh{^9Zt zmrL$OUxc^3_m<~L&S`aCxjtAP`=|c4VaQ@B{OZYX6rlEk9))DnbHi9+q<98S5( Xl3c=@Q@Jyl7>g(0;+5XS*UJb1*l8IM diff --git a/src/client/initMatrix.ts b/src/client/initMatrix.ts index 6db8358bd..65c681c5f 100644 --- a/src/client/initMatrix.ts +++ b/src/client/initMatrix.ts @@ -7,6 +7,7 @@ import { LotusOidcTokenRefresher } from './oidcTokenRefresher'; import { revokeOidcTokens } from './oidcLogout'; import { pushSessionToSW } from '../sw-session'; import { deleteSearchCacheDatabase } from '../app/utils/searchCache'; +import { clearTranslationCache } from '../app/state/translation'; // Thrown when the local IndexedDB has a higher schema version than this SDK expects. // This happens after a downgrade (e.g. matrix-js-sdk was briefly upgraded and then reverted). @@ -123,6 +124,8 @@ export const logoutClient = async (mx: MatrixClient) => { // The opt-in local search index stores decrypted plaintext — always wipe it // on logout. (clearLoginData below nukes all IDB databases, covering it too.) await deleteSearchCacheDatabase(); + // The message-translation cache also holds decrypted plaintext — wipe it too. + clearTranslationCache(); // Remove only the session credential keys, preserving user preferences and // unsent drafts (N98). The factory-reset path is clearLoginData() below. removeFallbackSession();