fix(addon): fully wipe add-on storage on logout (#1054, #1023) - #1048
Merged
Conversation
menuLogout() previously called browser.storage.local.clear(), wiping unrelated in-flight data in the same namespace -- specifically PENDING_ADDON_TOKEN (an in-progress AccountHub login) and SEND_MESSAGE_TO_BRIDGE (a passphrase staged for the bridge handoff). Replace the blanket clear with a scoped remove() targeting only the auth session, leaving the in-flight data intact. Refs #1023 Closes #1023
menuLogout() now scoped-removes STORAGE_KEY_AUTH instead of a blanket storage.local.clear() (this PR's fix), but two pre-existing tests still asserted clear() was called and failed CI (addon-changes / Shared package tests and lint): - menu.test.ts 'opens the logout page and clears storage after closing tabs' - happy-clean-logout-no-inflight-work.test.ts 'closes only Send tabs...' Both now assert storage.local.remove(STORAGE_KEY_AUTH) and that clear() is not called. Added the STORAGE_KEY_AUTH import to the integration spec.
4 tasks
Revert the scoped single-key remove() approach and restore a full browser.storage.local.clear() on genuine logout, returning the add-on to a clean, logged-out state (auth token, staged passphrase, pending OIDC token set, folder-lock records, cloud-file configs all cleared). storage.local is per-extension isolated (keyed to the add-on's gecko id), NOT shared with Thunderbird core or other add-ons, so a blanket clear() only ever touches TB-Send's own data. A scoped remove() is fail-open: it leaks the passphrase and a live refresh token past logout. A concurrent in-flight login (PENDING_ADDON_TOKEN) is intentionally cancelled by logout. The blanket clear() is only reachable from a genuine logout (menuLogout via LOGOUT action / SIGN_OUT); the read-only getLoginState() probe never wipes (see #948/#949). Tests updated to assert the full-wipe behavior. Closes #1054
aaspinwall
changed the title
radishmouse
approved these changes
Aug 10, 2026
radishmouse
left a comment
radishmouse
left a comment
Collaborator
There was a problem hiding this comment.
I see that the main code change (adding browser.storage.local.clear()) already exists in menu.ts - maybe added by another PR?
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
2 participants
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 changed?
Restores a full
browser.storage.local.clear()on genuine logout, reverting the earlier scoped single-keyremove(STORAGE_KEY_AUTH)approach. A genuine logout now returns the add-on to a clean, logged-out state: the auth token, the staged passphrase (SEND_MESSAGE_TO_BRIDGE), the pending OIDC token set (PENDING_ADDON_TOKEN), folder-lock records, and cloud-file account configs are all cleared, so the next launch requires a fresh login.Files:
packages/addon/src/menu.ts, plus the A5 integration test, the happy-path integration test, and themenu.test.tsunit test (all updated to assert the full-wipe behavior).Why the reversal?
The original PR scoped the clear to
STORAGE_KEY_AUTHonly, on the premise thatmenuLogout()shares itsbrowser.storage.localnamespace with unrelated in-flight data it shouldn't wipe.That premise doesn't hold:
browser.storage.localis namespaced PER-EXTENSION (keyed to this add-on's gecko id,tbpro-addon-stage@thunderbird.net). It is not shared with Thunderbird core or any other add-on — confirmed against the manifest (storagepermission, nomanaged/native storage) and the WebExtensions storage model (MDN). Every key in the store is TB-Send's own, so a blanketclear()only ever touches TB-Send data.SIGN_OUT) — must fully wipe to a clean, login-required state. The scopedremove()was fail-open: it leaked the passphrase and a live refresh token past logout, and any newly-added key would silently leak too.PENDING_ADDON_TOKEN) is intentionally cancelled by logout — logout wins over a half-finished login by design.Guardrail
The blanket
clear()is only reachable from a genuine logout (menuLogout(), via theLOGOUTmenu action /SIGN_OUTmessage). The read-onlygetLoginState()probe (60s timer + Send route guard) never wipes — it only callsstorage.local.get(). See #948/#949.Tests
undefinedafter logout, via a singleclear().clear()(not scopedremove()).Limitations and Notes
PENDING_ADDON_TOKENis a bearer refresh token that lingers in storage during the normal login window if a login is abandoned without a logout. That's a separate concern (a TTL/cleanup on the pending token) and out of scope here.--no-verify) due to sandbox limitations; CI will run them on the PR.Applicable Issues
Closes #1054
Closes #1023
Ref: ADDON-BUG-REPORTS-2026-07-22.md #A5, ADDON-SYNC-VERIFIED-FINDINGS-2026-07-21.md §A5.
Screenshots
N/A -- no UI changes.
🤖 AI-assisted: code, tests, and this description were written by an AI agent (Munky) under the repo owner's direction and review.