diff options
| -rw-r--r-- | src/features/messages.js | 47 | ||||
| -rw-r--r-- | tools/account-switch-leak-test.js | 123 | ||||
| -rw-r--r-- | tools/notif-faces-test.js | 2 | ||||
| -rw-r--r-- | tools/notif-note-cache-test.js | 2 |
4 files changed, 164 insertions, 10 deletions
diff --git a/src/features/messages.js b/src/features/messages.js index ac25b54..2097a5a 100644 --- a/src/features/messages.js +++ b/src/features/messages.js @@ -565,6 +565,12 @@ export function messagesFeature(ctx) { // ---- shared runtime ----------------------------------------------------- + // Bumped on every stop(): an account switch. Async work started under the + // previous identity (a relay query, a wrap mid-decrypt) checks it after + // each await and drops what it got — it used to land in the NEXT account's + // state and be saved there, so a throwaway account showed adam's DMs and + // notifications for good. + let session = 0; const rooms = new Map(); // cid -> room runtime const threads = new Map(); // peerPk -> Map(rumorId -> { rumor, mine }) // DM reactions, keyed by the reacted message's id — no thread needed, so a @@ -2911,9 +2917,11 @@ export function messagesFeature(ctx) { // true: opened. false: not ours. 'later': a remote signer never answered, // so nothing is known yet — the wrap must be tried again, uncounted. async function openInboxWrap(wrap, heal = false) { + const g = session; for (const d of dmDecryptors()) { let got; - try { got = await unwrapDMAny(wrap, d); } catch { return 'later'; } + try { got = await unwrapDMAny(wrap, d); } catch { return g === session ? 'later' : 'stale'; } + if (g !== session) return 'stale'; // opened with the previous account's key if (!got) continue; if (heal && ![7, 14, 15].includes(got.rumor.kind)) return true; if (got.rumor.kind === 14 || got.rumor.kind === 15) { @@ -2922,6 +2930,9 @@ export function messagesFeature(ctx) { // sent-copy authored by any of them must thread under the RECIPIENT, // or every welcome DM the registrar sends lands in a self-thread. const mine = isMe(got.author); + // a message that neither comes from nor names this identity isn't + // this account's conversation, however it reached us + if (!mine && !(got.rumor.tags || []).some((x) => x[0] === 'p' && isMe(x[1]))) return true; const to = got.rumor.tags?.find((x) => x[0] === 'p')?.[1]; const peer = mine ? (to || got.peer || got.author) : got.author; noteEmoji(got.rumor); @@ -2964,6 +2975,7 @@ export function messagesFeature(ctx) { } seenWraps.add(wrap.id); const r = await openInboxWrap(wrap, heal); + if (r === 'stale') return; if (r === true) { rememberWrap(wrap.id); return; } if (pendingWraps.size >= PENDING_MAX) pendingWraps.delete(pendingWraps.keys().next().value); pendingWraps.set(wrap.id, { wrap, heal, tries: r !== 'later' && decryptorsComplete() ? 1 : 0 }); @@ -3003,6 +3015,7 @@ export function messagesFeature(ctx) { // tried. Stop here — the slow retry picks the rest up. if (complete && !decryptorsComplete()) break; const r = await openInboxWrap(p.wrap, p.heal); + if (r === 'stale') break; // the account changed under this pass if (r === true) { pendingWraps.delete(id); rememberWrap(id); } else if (r === 'later') break; // the signer went quiet — nothing learned, try the rest later else if (complete && ++p.tries >= PENDING_TRIES) { pendingWraps.delete(id); rememberWrap(id); } @@ -3049,6 +3062,14 @@ export function messagesFeature(ctx) { swept = true; continue; } + // a row from someone who is neither us nor the peer: another + // identity's conversation, saved here by an account switch mid-decrypt + // (fixed — see session). The whole thread goes; ours refetches. + if (list.some((m) => !isMe(m.from) && m.from !== peer)) { + delete s.dms[peer]; + swept = true; + continue; + } for (const m of list) threadOf(peer).set(m.id, { rumor: rumorFromRow(m), mine: isMe(m.from) }); } bumpMsgRev(); @@ -12931,6 +12952,7 @@ export function messagesFeature(ctx) { const NOTIF_KINDS = [1, COMMENT, 6, 7, ...ZAP_KINDS]; const NOTIF_KEEP = 150; let notif = null; // { status, items } + const NOTIF_V = 2; let notifAt = 0, notifUnsub = null; const notifNoteAsking = new Set(); const notifSeen = () => st().notifSeen || 0; @@ -12941,7 +12963,9 @@ export function messagesFeature(ctx) { function notifNow() { if (!notif) { const s = st(); - const stored = (!s.notifsPk || s.notifsPk === identityPk()) ? (s.notifs || []) : []; + // v2: lists saved before the account-switch fix may hold another + // identity's rows under this one's key — dropped once, refetched + const stored = (s.notifsV === NOTIF_V && s.notifsPk === identityPk()) ? (s.notifs || []) : []; notif = { status: stored.length ? 'ready' : 'loading', items: stored }; // Reply events and the notes reactions/zaps point at ride beside their // notification rows. Seed the in-memory note caches synchronously, so @@ -12988,7 +13012,7 @@ export function messagesFeature(ctx) { const notes = {}; for (const id of wanted) if (notifNotes.has(id)) notes[id] = savedNotifNote(notifNotes.get(id)); const s2 = st(); - s2.notifs = notif.items; s2.notifsPk = identityPk(); + s2.notifs = notif.items; s2.notifsPk = identityPk(); s2.notifsV = NOTIF_V; s2.notifNoteCache = notes; s2.notifNotesPk = identityPk(); save(s2); } @@ -13055,10 +13079,12 @@ export function messagesFeature(ctx) { return null; } function mergeNotifs(evs) { + // only what names this identity + evs = (evs || []).filter((ev) => ev && (ev.tags || []).some((x) => x[0] === 'p' && isMe(x[1]))); const c = notifNow(); const known = new Set(c.items.map((x) => x.id)); - for (const ev of evs || []) noteForSpam(ev); - const add = (evs || []).map((ev) => ((ev.kind === 1 || ev.kind === COMMENT) && hidden(ev) ? null : notifItem(ev))) + for (const ev of evs) noteForSpam(ev); + const add = evs.map((ev) => ((ev.kind === 1 || ev.kind === COMMENT) && hidden(ev) ? null : notifItem(ev))) .filter((x) => x && !hiddenPk(x.actor) && !known.has(x.id) && known.add(x.id)); if (!add.length) return false; warmNotifFaces(add); @@ -13087,14 +13113,18 @@ export function messagesFeature(ctx) { if (!my.length) { if (notif) notif.status = 'ready'; return; } if (!force && Date.now() - notifAt < 30_000) return; notifAt = Date.now(); + const g = session; try { const evs = await queryOn(zapRelays(), { kinds: NOTIF_KINDS, '#p': my, limit: 120 }, 5000); + if (g !== session) return; // asked for the previous account if (mergeNotifs(evs)) scheduleRepaint(); } catch {} finally { - if (notif) notif.status = 'ready'; - scheduleRepaint(); + if (g === session) { + if (notif) notif.status = 'ready'; + scheduleRepaint(); + } } - watchNotifs(); + if (g === session) watchNotifs(); } function watchNotifs() { stopNotifWatch(); @@ -13610,6 +13640,7 @@ export function messagesFeature(ctx) { // what happened to THEIR posts stays with them — the next identity // starts its list empty and asks the relays under its own key stopNotifWatch(); notif = null; notifAt = 0; notifNotes.clear(); + session++; zapWho.clear(); whoOpenIds.clear(); clearTimeout(zapSaveT); zapSaveT = null; zapSeed = null; zapTotals.clear(); zapAsked.clear(); zapPending.clear(); // 'mine' is per identity — refetch under the next diff --git a/tools/account-switch-leak-test.js b/tools/account-switch-leak-test.js new file mode 100644 index 0000000..ea3b4ee --- /dev/null +++ b/tools/account-switch-leak-test.js @@ -0,0 +1,123 @@ +// Switching accounts while the previous one's relay query is still out: the +// answer used to land in the NEXT account's notifications and be saved there +// under its key, so a throwaway account showed adam's zaps and reactions for +// good. Also: a DM cache already polluted that way is swept on load. +// Run: bun tools/account-switch-leak-test.js +import puppeteer from './puppeteer.js'; +import { cacheKeyFor } from '../src/wallet.js'; +import { generateSecretKey, getPublicKey, finalizeEvent } from 'nostr-tools/pure'; +import { privateKeyFromSeedWords } from 'nostr-tools/nip06'; +import { buildHtml } from '../build.js'; +import { makeDM } from '../src/dm.js'; +const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); +let ok = true; +const check = (n, c, d = '') => { console.log(` ${c ? '✓' : '✗'} ${n}${d ? ' — ' + d : ''}`); if (!c) ok = false; }; +const ACTOR = (() => { const sk = generateSecretKey(); return { sk, pk: getPublicKey(sk) }; })(); +const STRANGER = getPublicKey(generateSecretKey()); +const now = Math.floor(Date.now() / 1000); + +const html = await buildHtml({ minify: true, pwa: false }); +const server = Bun.serve({ port: 5302, fetch: () => new Response(html, { headers: { 'content-type': 'text/html' } }) }); +const browser = await puppeteer.launch({ executablePath: '/usr/bin/google-chrome', headless: 'new', args: ['--no-sandbox'] }); +const page = await browser.newPage(); +await page.evaluateOnNewDocument(() => { + // notification queries (#p, reactions) for the account under test are + // answered slowly — long enough to switch accounts while one is out + const Real = window.WebSocket; + window.WebSocket = function (...a) { + const ws = new Real(...a); + const send = ws.send.bind(ws); + ws.send = (data) => { + try { + const m = JSON.parse(data); + if (m[0] === 'REQ') { + const f = m[2] || {}; + const victim = window.__victim && (f['#p'] || []).includes(window.__victim.pk) && (f.kinds || []).includes(7); + const dm = window.__dm && (f['#p'] || []).includes(window.__dm.pk) && (f.kinds || []).includes(1059) ? [window.__dm.wrap] : []; + if (victim) (window.__asked = (window.__asked || 0) + 1); + setTimeout(() => { + if (victim) ws.dispatchEvent(new MessageEvent('message', { data: JSON.stringify(['EVENT', m[1], window.__victim.react]) })); + for (const w of dm) ws.dispatchEvent(new MessageEvent('message', { data: JSON.stringify(['EVENT', m[1], w]) })); + ws.dispatchEvent(new MessageEvent('message', { data: JSON.stringify(['EOSE', m[1]]) })); + }, victim ? 4000 : 0); + return; + } + if (m[0] === 'EVENT') return; + } catch {} + return send(data); + }; + return ws; + }; + window.WebSocket.prototype = Real.prototype; Object.assign(window.WebSocket, Real); + try { const v = sessionStorage.getItem('__victim'); if (v) window.__victim = JSON.parse(v); } catch {} + try { const v = sessionStorage.getItem('__dm'); if (v) window.__dm = JSON.parse(v); } catch {} +}); +const errs = []; +page.on('pageerror', (e) => errs.push(String(e))); +const click = (t) => page.evaluate((x) => { const e = [...document.querySelectorAll('button')].find((n) => n.textContent.trim().toLowerCase().includes(x)); if (e) { e.click(); return true; } return false; }, t); +const text = () => page.evaluate(() => document.body.innerText); +const waitText = async (x, ms = 25000) => { for (let i = 0; i < ms / 250; i++) { if ((await text()).toLowerCase().includes(x)) return true; await sleep(250); } return false; }; +const nav = (label) => page.evaluate((l) => document.querySelector(`.app-nav-button[aria-label="${l}"]`)?.click(), label); +async function newWallet() { + await click('create a new wallet'); await sleep(600); + if (!(await page.$('.words .w .t'))) { await click('generate'); await sleep(600); } + await page.waitForSelector('.words .w .t'); + const mn = (await page.$$eval('.words .w .t', (l) => l.map((e) => e.textContent.trim()))).join(' '); + await click('skip verification'); + await waitText('receive', 20000); + return mn; +} + +try { + await page.setViewport({ width: 390, height: 844 }); + await page.goto('http://localhost:5302/', { waitUntil: 'domcontentloaded' }); + await page.evaluate(() => localStorage.setItem('btc-wallet-network', 'regtest')); + await page.reload({ waitUntil: 'domcontentloaded' }); + await sleep(400); + const mnA = await newWallet(); + const A = getPublicKey(privateKeyFromSeedWords(mnA)); + const react = finalizeEvent({ kind: 7, created_at: now - 30, tags: [['e', 'f'.repeat(64)], ['p', A]], content: '🔥' }, ACTOR.sk); + await page.evaluate((v) => { sessionStorage.setItem('__victim', JSON.stringify(v)); }, { pk: A, react }); + await page.reload({ waitUntil: 'domcontentloaded' }); + await waitText('receive', 20000); + + console.log('\n[account A asks for its notifications; the answer is slow]'); + await nav('Notifications'); await sleep(800); + check('A\'s notification query is out', (await page.evaluate(() => window.__asked || 0)) > 0); + + console.log('\n[switch to a new account B before it answers]'); + await page.evaluate(() => document.querySelector('.header-avatar')?.click()); await sleep(1200); + if (process.env.DEBUG) console.log(await page.evaluate(() => [...document.querySelectorAll('button')].map((b) => b.textContent.trim() || b.getAttribute('aria-label')).join(' | '))); + check('the accounts page offers another sign-in', await click('sign into another account')); + await sleep(500); + const mnB = await newWallet(); + const B = getPublicKey(privateKeyFromSeedWords(mnB)); + check('B is a different identity', B !== A); + await sleep(6000); // A's answer has long since landed + await nav('Notifications'); await sleep(1500); + const baseB = cacheKeyFor(mnB + '\n'); + const stB = await page.evaluate((k) => JSON.parse(localStorage.getItem(k + ':messages') || '{}'), baseB); + check('B\'s saved notifications hold nothing of A\'s', !(stB.notifs || []).some((x) => x.actor === ACTOR.pk), JSON.stringify((stB.notifs || []).map((x) => x.actor.slice(0, 8)))); + check('...and its page doesn\'t show A\'s reaction', !(await page.evaluate(() => document.body.innerText.includes('🔥')))); + + console.log('\n[a DM cache polluted before the fix is swept]'); + await page.evaluate(([k, a, s]) => { + const st = JSON.parse(localStorage.getItem(k + ':messages') || '{}'); + st.dms = { [s]: [{ id: 'd'.repeat(64), from: a, text: 'adam wrote this to someone', t: Math.floor(Date.now() / 1000) - 60 }, + { id: 'e'.repeat(64), from: s, text: 'their answer to adam', t: Math.floor(Date.now() / 1000) - 30 }] }; + localStorage.setItem(k + ':messages', JSON.stringify(st)); + }, [baseB, A, STRANGER]); + // and a real DM to B, so the sweep is seen not to cost B its own messages + const { toPeer } = await makeDM(ACTOR.sk, B, 'hello B, a real DM'); + await page.evaluate((v) => sessionStorage.setItem('__dm', JSON.stringify(v)), { pk: B, wrap: toPeer }); + await page.reload({ waitUntil: 'domcontentloaded' }); + await waitText('receive', 20000); + await nav('Messages'); await sleep(2500); + check('a DM sent to B still arrives', await waitText('hello b, a real dm', 10000)); + check('A\'s conversation isn\'t in B\'s messages', !(await page.evaluate(() => document.body.innerText.includes('their answer to adam')))); + const dmsB = await page.evaluate((k) => JSON.parse(localStorage.getItem(k + ':messages') || '{}').dms || {}, baseB); + check('...and is gone from its cache', !dmsB[STRANGER], Object.keys(dmsB).length + ' thread(s)'); + check('no page errors', errs.length === 0, errs.slice(0, 2).join(' | ')); +} finally { await browser.close(); server.stop(true); } +console.log(ok ? '\n✅ an account switch keeps each identity\'s notifications and DMs apart' : '\n❌ failed'); +process.exit(ok ? 0 : 1); diff --git a/tools/notif-faces-test.js b/tools/notif-faces-test.js index b43a41f..b45b120 100644 --- a/tools/notif-faces-test.js +++ b/tools/notif-faces-test.js @@ -75,7 +75,7 @@ try { const items = ACTORS.map((a, i) => ({ id: 'r'.repeat(63) + i, what: 'react', actor: a.pk, target: 't'.repeat(64), hint: [], emoji: '❤️', ts: now - 60 * (i + 1) })); await page.evaluate(([k, me, items]) => { const st = JSON.parse(localStorage.getItem(k + ':messages') || '{}'); - st.notifs = items; st.notifsPk = me; + st.notifs = items; st.notifsPk = me; st.notifsV = 2; localStorage.setItem(k + ':messages', JSON.stringify(st)); }, [base, ME, items]); await page.reload({ waitUntil: 'domcontentloaded' }); diff --git a/tools/notif-note-cache-test.js b/tools/notif-note-cache-test.js index a3173fa..e24c2e2 100644 --- a/tools/notif-note-cache-test.js +++ b/tools/notif-note-cache-test.js @@ -93,7 +93,7 @@ try { const item = { id: 'a'.repeat(64), what: 'react', actor: actorPk, target: target.id, hint: [], emoji: '❤️', ts: now - 30 }; await page.evaluate(([key, owner, notification]) => { const state = JSON.parse(localStorage.getItem(key + ':messages') || '{}'); - state.notifs = [notification]; state.notifsPk = owner; + state.notifs = [notification]; state.notifsPk = owner; state.notifsV = 2; localStorage.setItem(key + ':messages', JSON.stringify(state)); }, [base, me, item]); await page.reload({ waitUntil: 'domcontentloaded' }); |