fix(addon): fully wipe add-on storage on logout (#1054, #1023)

packages/addon/src/menu.ts

@@ -115,7 +115,25 @@ export async function menuLogout() {
   console.log('🧹 Clearing menu items and storage');
   await browser.TBProMenu.clear('root');
 
-  // Clear all extension storage
+  // Full wipe of the add-on's storage to a clean, logged-out state.
+  //
+  // browser.storage.local is namespaced PER-EXTENSION (keyed to this add-on's
+  // gecko id) -- it is NOT shared with Thunderbird core or any other add-on.
+  // So every key in here is TB-Send's own: STORAGE_KEY_AUTH, the staged
+  // passphrase (SEND_MESSAGE_TO_BRIDGE), the pending OIDC token set
+  // (PENDING_ADDON_TOKEN), folder-lock records, cloud-file account configs, etc.
+  //
+  // On a genuine logout the product requirement is a clean wipe: auth token,
+  // passphrase, and ALL other add-on data must be gone so the next launch
+  // requires a fresh login. A scoped single-key remove() leaks the passphrase
+  // and a live refresh token past logout (see #1023 / #1054). A concurrent
+  // in-flight login (PENDING_ADDON_TOKEN) is intentionally cancelled here --
+  // logout wins over a half-finished login by design.
+  //
+  // This blanket clear() is ONLY reached from a genuine logout (menuLogout(),
+  // driven by the LOGOUT menu action / the SIGN_OUT message). The read-only
+  // getLoginState() probe (60s timer + route guard) MUST NOT reach this and
+  // does not -- it only ever calls storage.local.get(). See #948/#949.
   await browser.storage.local.clear();
 
   // Clear localStorage (if running in a context that has access to it)

packages/addon/src/test/integration/a5-logout-clears-unrelated-storage.test.ts

@@ -1,28 +1,34 @@
 /**
- * A5 — `menuLogout()`'s blanket `storage.local.clear()` collateral damage.
+ * A5 — `menuLogout()` must fully wipe the add-on's storage on logout.
  *
- * See ADDON-BUG-REPORTS-2026-07-22.md #A5 and
- * ADDON-SYNC-VERIFIED-FINDINGS-2026-07-21.md §A5.
+ * See ADDON-BUG-REPORTS-2026-07-22.md #A5,
+ * ADDON-SYNC-VERIFIED-FINDINGS-2026-07-21.md §A5, and #1054.
+ *
+ * Product requirement: a genuine logout (from the menu OR the web app inside
+ * Thunderbird) returns the add-on to a clean, logged-out state -- the auth
+ * token, the staged passphrase, and ALL other add-on data are gone, so the
+ * next launch requires a fresh login.
+ *
+ * browser.storage.local is namespaced PER-EXTENSION (keyed to this add-on's
+ * gecko id); it is NOT shared with Thunderbird core or any other add-on. Every
+ * key in it is TB-Send's own, so a blanket clear() only ever touches TB-Send
+ * data -- which is exactly what "clean wipe" wants. A scoped single-key
+ * remove() would leak the passphrase (SEND_MESSAGE_TO_BRIDGE) and a live
+ * refresh token (PENDING_ADDON_TOKEN) past logout.
+ *
+ * A concurrent in-flight login (PENDING_ADDON_TOKEN) is intentionally
+ * cancelled by logout -- logout wins over a half-finished login by design.
  *
  * Mechanism under test (menu.ts, menuLogout()):
  *   await browser.storage.local.clear();
  *
- * This unconditional, blanket clear lives in the same browser.storage.local
- * namespace as unrelated in-flight staged data:
- *   - PENDING_ADDON_TOKEN (background.ts's triggerAddonLogin() staging key
- *     for an AccountHub-driven login in progress)
- *   - SEND_MESSAGE_TO_BRIDGE (a passphrase staged for the bridge handoff)
- *
- * This test drives the real `menuLogout()` against the fake host's shared
- * storage and proves that BOTH of those unrelated keys are destroyed by a
- * logout that has nothing to do with them, with no error surfaced to
- * whichever flow was relying on them.
+ * This spec asserts that a logout leaves NOTHING behind in storage.
  */
 import { afterEach, beforeEach, describe, expect, it } from 'vitest';
 import { type FakeHost } from './fakeThunderbirdHost';
 import { setupHost, stubContext, teardownHost } from './testHelpers';
 
-describe('A5: menuLogout() blanket storage.local.clear() wipes unrelated in-flight data', () => {
+describe('A5: menuLogout() must fully wipe add-on storage to a clean logged-out state', () => {
   let host: FakeHost;
 
   beforeEach(() => {
@@ -33,56 +39,54 @@ describe('A5: menuLogout() blanket storage.local.clear() wipes unrelated in-flig
     teardownHost();
   });
 
-  it('CONFIRMED BUG: a concurrent AccountHub login and bridged passphrase are silently destroyed by an unrelated logout', async () => {
+  it('FULL WIPE: auth token, staged passphrase, pending login token, and account config are ALL cleared on logout', async () => {
     const ctx = stubContext(host);
     const { menuLogout } = await import('../../menu');
 
-    // Simulate two unrelated flows mid-flight, both staging data in the
-    // same browser.storage.local namespace menuLogout() will nuke:
-    //   1. An AccountHub-driven login has staged a pending token set.
-    //   2. A passphrase bridge handoff is staged, waiting to be consumed by
-    //      restoreKeysUsingLocalStorage() in another context (popup/web).
+    // Seed the storage with a representative spread of the add-on's own keys,
+    // matching what background.ts / auth-store.ts / extension-store.ts write:
+    //   - the auth session
+    //   - a staged passphrase (bridge handoff) -- security-sensitive
+    //   - a pending OIDC token set (in-flight add-on login) -- a refresh token
+    //   - a per-account cloud-file server config
     await ctx.browser.storage.local.set({
-      'tbpro-pending-addon-token': {
-        refresh_token: 'staged-refresh-token',
-      },
+      'send-auth': { some: 'auth-session' },
       SEND_MESSAGE_TO_BRIDGE: 'staged-passphrase-words',
-      // Also seed an unrelated per-account cloud-file server config key,
-      // matching extension-store.ts's browser.storage.local.set({[id]: ...}).
+      'tbpro-pending-addon-token': { refresh_token: 'staged-refresh-token' },
       'account-123': { server: 'https://send.tb.pro' },
     });
 
-    // Sanity: all three unrelated keys are present before logout.
+    // Sanity: everything is present before logout.
     const before = await ctx.browser.storage.local.get([
-      'tbpro-pending-addon-token',
+      'send-auth',
       'SEND_MESSAGE_TO_BRIDGE',
+      'tbpro-pending-addon-token',
       'account-123',
     ]);
-    expect(before['tbpro-pending-addon-token']).toBeDefined();
+    expect(before['send-auth']).toBeDefined();
     expect(before['SEND_MESSAGE_TO_BRIDGE']).toBeDefined();
+    expect(before['tbpro-pending-addon-token']).toBeDefined();
     expect(before['account-123']).toBeDefined();
 
-    // Now an unrelated logout happens (e.g. the user clicked Logout in the
-    // hamburger menu, or a SIGN_OUT message arrived) while the above flows
-    // are still mid-flight.
+    // A genuine logout (LOGOUT menu action / SIGN_OUT message).
     await menuLogout();
 
-    // THE BUG: menuLogout() calls browser.storage.local.clear() -- a
-    // blanket wipe, not scoped to STORAGE_KEY_AUTH -- so all three unrelated
-    // keys are silently destroyed with no error surfaced to either the
-    // AccountHub login flow or the passphrase bridge handoff.
+    // FULL WIPE: nothing the add-on owns survives the logout. The passphrase
+    // and the refresh token in particular must NOT leak past sign-out.
     const after = await ctx.browser.storage.local.get([
-      'tbpro-pending-addon-token',
+      'send-auth',
       'SEND_MESSAGE_TO_BRIDGE',
+      'tbpro-pending-addon-token',
       'account-123',
     ]);
-    expect(after['tbpro-pending-addon-token']).toBeUndefined();
+    expect(after['send-auth']).toBeUndefined();
     expect(after['SEND_MESSAGE_TO_BRIDGE']).toBeUndefined();
+    expect(after['tbpro-pending-addon-token']).toBeUndefined();
     expect(after['account-123']).toBeUndefined();
 
-    // Confirm this was via the blanket clear() call specifically (not a
-    // scoped set of individual remove() calls for an allow-list), which is
-    // exactly the fix target identified in the bug report.
+    // Confirm the wipe was via the blanket clear() -- the correct mechanism
+    // for a "return to clean state" logout, since storage.local is
+    // per-extension isolated (see #1054).
     expect(ctx.browser.storage.local.clear).toHaveBeenCalledTimes(1);
   });
 });

packages/addon/src/test/integration/happy-clean-logout-no-inflight-work.test.ts

@@ -36,8 +36,10 @@ describe('Happy path: clean logout with no in-flight work', () => {
     expect(ctx.browser.tabs.remove).toHaveBeenCalledTimes(1);
     expect(ctx.browser.tabs.remove).toHaveBeenCalledWith(1);
 
-    // Storage was cleared (there was nothing unrelated in it to lose, so
-    // this is the intended, harmless case).
+    // Storage was fully wiped via the blanket clear() -- a genuine logout
+    // returns the add-on to a clean, logged-out state. storage.local is
+    // per-extension isolated, so this only touches TB-Send's own data.
+    // See #1054 / A5.
     expect(ctx.browser.storage.local.clear).toHaveBeenCalledTimes(1);
 
     // Menu was reset to the logged-out state.

packages/addon/src/test/menu.test.ts

@@ -146,7 +146,11 @@ describe('menuLogout', () => {
 
     await menuLogout();
 
-    expect(browser.storage.local.clear).toHaveBeenCalled();
+    // menuLogout() fully wipes the add-on's storage via a blanket
+    // storage.local.clear() -- a genuine logout returns the add-on to a clean,
+    // logged-out state. storage.local is per-extension isolated, so this only
+    // touches TB-Send's own data (see #1054 / A5).
+    expect(browser.storage.local.clear).toHaveBeenCalledTimes(1);
     expect(browser.tabs.create).toHaveBeenCalledWith({
       url: `${BASE_URL}/logout`,
     });