From d6df6b5c85121161fe66d53b1ef97a3525a51509 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 1 Apr 2026 13:12:52 -0400 Subject: [PATCH 01/24] feat: harden sidebar mode with Firefox support, security fixes, and reliability improvements Port all sidebar changes from sidebar-mode-audit: add Firefox sidebar_action support, harden trust boundaries (sender validation, port origin checks, isFromExtensionPage guard on OPEN_SIDEBAR), replace broadcast messaging with direct port communication, add safeResolve to prevent double-resolving promises, fix boolean coercion for isOpenSidebarByDefault, reject pending signing requests on sidebar close, remove dead SIDEBAR_REGISTER/UNREGISTER code, and add extension protocol guard to isSidebarMode. Co-Authored-By: Claude Opus 4.6 (1M context) --- @shared/api/types/message-request.ts | 11 -- @shared/constants/services.ts | 4 +- extension/public/static/manifest/v3.json | 7 + extension/src/background/index.ts | 91 +++++++++++- .../__tests__/loadSaveSettings.test.ts | 123 ++++++++++++++++ .../messageListener/__tests__/sidebar.test.ts | 139 ++++++++++++++++++ .../freighterApiMessageListener.ts | 125 ++++++++++++---- .../messageListener/handlers/loadSettings.ts | 3 +- .../messageListener/handlers/saveSettings.ts | 5 +- .../messageListener/popupMessageListener.ts | 21 ++- .../SidebarSigningListener/index.tsx | 20 ++- .../account/AccountHeader/index.tsx | 6 +- extension/src/popup/helpers/isSidebarMode.ts | 3 + extension/src/popup/helpers/navigate.ts | 2 +- .../src/popup/locales/en/translation.json | 3 + .../src/popup/locales/pt/translation.json | 3 + 16 files changed, 489 insertions(+), 77 deletions(-) create mode 100644 extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts create mode 100644 extension/src/background/messageListener/__tests__/sidebar.test.ts diff --git a/@shared/api/types/message-request.ts b/@shared/api/types/message-request.ts index 9eff6052ce..932a65b1c7 100644 --- a/@shared/api/types/message-request.ts +++ b/@shared/api/types/message-request.ts @@ -445,15 +445,6 @@ export interface MarkQueueActiveMessage extends BaseMessage { isActive: boolean; } -export interface SidebarRegisterMessage extends BaseMessage { - type: SERVICE_TYPES.SIDEBAR_REGISTER; - windowId: number; -} - -export interface SidebarUnregisterMessage extends BaseMessage { - type: SERVICE_TYPES.SIDEBAR_UNREGISTER; -} - export interface OpenSidebarMessage extends BaseMessage { type: SERVICE_TYPES.OPEN_SIDEBAR; windowId: number; @@ -524,6 +515,4 @@ export type ServiceMessageRequest = | ChangeCollectibleVisibilityMessage | GetHiddenCollectiblesMessage | MarkQueueActiveMessage - | SidebarRegisterMessage - | SidebarUnregisterMessage | OpenSidebarMessage; diff --git a/@shared/constants/services.ts b/@shared/constants/services.ts index ad9c9445b9..0f5b3bf751 100644 --- a/@shared/constants/services.ts +++ b/@shared/constants/services.ts @@ -63,11 +63,11 @@ export enum SERVICE_TYPES { CHANGE_COLLECTIBLE_VISIBILITY = "CHANGE_COLLECTIBLE_VISIBILITY", GET_HIDDEN_COLLECTIBLES = "GET_HIDDEN_COLLECTIBLES", MARK_QUEUE_ACTIVE = "MARK_QUEUE_ACTIVE", - SIDEBAR_REGISTER = "SIDEBAR_REGISTER", - SIDEBAR_UNREGISTER = "SIDEBAR_UNREGISTER", OPEN_SIDEBAR = "OPEN_SIDEBAR", } +export const SIDEBAR_NAVIGATE = "SIDEBAR_NAVIGATE"; + export enum EXTERNAL_SERVICE_TYPES { REQUEST_ACCESS = "REQUEST_ACCESS", REQUEST_PUBLIC_KEY = "REQUEST_PUBLIC_KEY", diff --git a/extension/public/static/manifest/v3.json b/extension/public/static/manifest/v3.json index 5a8a1124bc..8653aff462 100644 --- a/extension/public/static/manifest/v3.json +++ b/extension/public/static/manifest/v3.json @@ -38,5 +38,12 @@ "side_panel": { "default_path": "index.html?mode=sidebar" }, + "sidebar_action": { + "default_panel": "index.html?mode=sidebar", + "default_icon": { + "16": "images/icon16.png", + "32": "images/icon32.png" + } + }, "manifest_version": 3 } diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index 19eb0a9425..16580c57d5 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -11,9 +11,19 @@ import { popupMessageListener, clearSidebarWindowId, setSidebarWindowId, + responseQueue, + transactionQueue, + blobQueue, + authEntryQueue, + tokenQueue, } from "./messageListener/popupMessageListener"; +import { + setSidebarPort, + clearSidebarPort, +} from "./messageListener/freighterApiMessageListener"; import { freighterApiMessageListener } from "./messageListener/freighterApiMessageListener"; import { SIDEBAR_PORT_NAME } from "popup/components/SidebarSigningListener"; +import { activeQueueUuids } from "./helpers/queueCleanup"; import { SESSION_ALARM_NAME, SessionTimer, @@ -52,19 +62,64 @@ export const initContentScriptMessageListener = () => { }; export const initSidebarConnectionListener = () => { - chrome.runtime.onConnect.addListener((port) => { + browser.runtime.onConnect.addListener((port) => { if (port.name !== SIDEBAR_PORT_NAME) return; + // Reject connections from content scripts; only extension pages are trusted. + // Extension pages have no associated tab and load from the extension origin. + const isExtensionPage = + !port.sender?.tab && + port.sender?.url?.startsWith(browser.runtime.getURL("")); + if (!isExtensionPage) { + port.disconnect(); + return; + } + + // Store port reference so openSigningWindow can send messages directly + setSidebarPort(port); + // Sidebar sends its window ID as first message - port.onMessage.addListener((msg: { windowId: number }) => { - if (msg.windowId !== undefined) { - setSidebarWindowId(msg.windowId); + port.onMessage.addListener((msg: unknown) => { + const { windowId } = msg as { windowId?: number }; + if (typeof windowId === "number") { + setSidebarWindowId(windowId); } }); // When sidebar closes (for any reason), clear the window ID + // and reject any pending signing requests port.onDisconnect.addListener(() => { + clearSidebarPort(); clearSidebarWindowId(); + + // Reject all active signing requests that were open in the sidebar + for (const uuid of activeQueueUuids) { + const responseIndex = responseQueue.findIndex( + (item) => item.uuid === uuid, + ); + if (responseIndex !== -1) { + const responseQueueItem = responseQueue.splice(responseIndex, 1)[0]; + responseQueueItem.response(undefined); + } + + // Clean up the data queues + const txIndex = transactionQueue.findIndex( + (item) => item.uuid === uuid, + ); + if (txIndex !== -1) transactionQueue.splice(txIndex, 1); + + const blobIndex = blobQueue.findIndex((item) => item.uuid === uuid); + if (blobIndex !== -1) blobQueue.splice(blobIndex, 1); + + const authIndex = authEntryQueue.findIndex( + (item) => item.uuid === uuid, + ); + if (authIndex !== -1) authEntryQueue.splice(authIndex, 1); + + const tokenIndex = tokenQueue.findIndex((item) => item.uuid === uuid); + if (tokenIndex !== -1) tokenQueue.splice(tokenIndex, 1); + } + activeQueueUuids.clear(); }); }); }; @@ -92,6 +147,7 @@ export const initExtensionMessageListener = () => { localStore, keyManager, sessionTimer, + sender, ); } if ( @@ -135,17 +191,38 @@ export const initInstalledListener = () => { export const initSidebarBehavior = async () => { const localStore = dataStorageAccess(browserLocalStorage); + // getItem returns a string from localStorage; compare explicitly to avoid + // any truthy string (e.g. "false") being coerced to true. const val = - ((await localStore.getItem(IS_OPEN_SIDEBAR_BY_DEFAULT_ID)) as boolean) ?? - false; + (await localStore.getItem(IS_OPEN_SIDEBAR_BY_DEFAULT_ID)) === "true"; if (chrome.sidePanel?.setPanelBehavior) { + // Chrome: delegate action-click to the side panel when enabled chrome.sidePanel - .setPanelBehavior({ openPanelOnActionClick: !!val }) + .setPanelBehavior({ openPanelOnActionClick: val }) .catch((e) => console.error("Failed to set panel behavior:", e)); + } else if ((browser as any).sidebarAction) { + // Firefox: when "open by default" is on, clear the browser-action popup so + // clicks fire onClicked instead of opening the popup, and open the sidebar. + // When off, restore the popup so the normal popup opens on click. + if ((browser as any).browserAction?.setPopup) { + await (browser as any).browserAction + .setPopup({ popup: val ? "" : "index.html" }) + .catch((e: unknown) => + console.error("Failed to set browser action popup:", e), + ); + } } }; export const initAlarmListener = () => { + // Firefox: when "open by default" is on the browser-action popup is cleared, + // so clicks fire onClicked. Open the sidebar in response. + if ((browser as any).browserAction?.onClicked) { + (browser as any).browserAction.onClicked.addListener(() => { + (browser as any).sidebarAction?.open?.(); + }); + } + browser?.alarms?.onAlarm.addListener(async ({ name }: { name: string }) => { const sessionStore = await buildStore(); const localStore = dataStorageAccess(browserLocalStorage); diff --git a/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts b/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts new file mode 100644 index 0000000000..bfaaaf81d8 --- /dev/null +++ b/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts @@ -0,0 +1,123 @@ +import { loadSettings } from "../handlers/loadSettings"; +import { saveSettings } from "../handlers/saveSettings"; +import { IS_OPEN_SIDEBAR_BY_DEFAULT_ID } from "constants/localStorageTypes"; + +jest.mock("background/helpers/account", () => ({ + getAllowList: jest.fn().mockResolvedValue([]), + getAssetsLists: jest.fn().mockResolvedValue([]), + getIsExperimentalModeEnabled: jest.fn().mockResolvedValue(false), + getIsHashSigningEnabled: jest.fn().mockResolvedValue(false), + getIsHideDustEnabled: jest.fn().mockResolvedValue(true), + getIsMemoValidationEnabled: jest.fn().mockResolvedValue(true), + getIsNonSSLEnabled: jest.fn().mockResolvedValue(false), + getNetworkDetails: jest.fn().mockResolvedValue({ + network: "TESTNET", + networkName: "Test Net", + networkUrl: "https://horizon-testnet.stellar.org", + networkPassphrase: "Test SDF Network ; September 2015", + }), + getNetworksList: jest.fn().mockResolvedValue([]), + verifySorobanRpcUrls: jest.fn().mockResolvedValue(undefined), + getFeatureFlags: jest.fn().mockResolvedValue({ useSorobanPublic: false }), + getOverriddenBlockaidResponse: jest.fn().mockResolvedValue(null), +})); + +jest.mock("../helpers/get-hidden-assets", () => ({ + getHiddenAssets: jest.fn().mockResolvedValue({ hiddenAssets: {} }), +})); + +(global as any).chrome = { + sidePanel: { + setPanelBehavior: jest.fn().mockResolvedValue(undefined), + }, +}; + +describe("loadSettings isOpenSidebarByDefault", () => { + it('returns true when localStorage value is the string "true"', async () => { + const localStore = { + getItem: jest.fn().mockImplementation((key: string) => { + if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) + return Promise.resolve("true"); + return Promise.resolve(null); + }), + setItem: jest.fn(), + } as any; + + const result = await loadSettings({ localStore }); + expect(result.isOpenSidebarByDefault).toBe(true); + }); + + it('returns false when localStorage value is the string "false"', async () => { + const localStore = { + getItem: jest.fn().mockImplementation((key: string) => { + if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) + return Promise.resolve("false"); + return Promise.resolve(null); + }), + setItem: jest.fn(), + } as any; + + const result = await loadSettings({ localStore }); + expect(result.isOpenSidebarByDefault).toBe(false); + }); + + it("returns false when localStorage value is null", async () => { + const localStore = { + getItem: jest.fn().mockResolvedValue(null), + setItem: jest.fn(), + } as any; + + const result = await loadSettings({ localStore }); + expect(result.isOpenSidebarByDefault).toBe(false); + }); +}); + +describe("saveSettings isOpenSidebarByDefault", () => { + it("calls setPanelBehavior with the boolean from the request", async () => { + const localStore = { + getItem: jest.fn().mockImplementation((key: string) => { + if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) + return Promise.resolve("true"); + return Promise.resolve(null); + }), + setItem: jest.fn().mockResolvedValue(undefined), + } as any; + + const request = { + isDataSharingAllowed: true, + isMemoValidationEnabled: true, + isHideDustEnabled: true, + isOpenSidebarByDefault: true, + } as any; + + const result = await saveSettings({ request, localStore }); + + expect(chrome.sidePanel.setPanelBehavior).toHaveBeenCalledWith({ + openPanelOnActionClick: true, + }); + expect(result.isOpenSidebarByDefault).toBe(true); + }); + + it('returns boolean false (not the string "false") after saving', async () => { + const localStore = { + getItem: jest.fn().mockImplementation((key: string) => { + if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) + return Promise.resolve("false"); + return Promise.resolve(null); + }), + setItem: jest.fn().mockResolvedValue(undefined), + } as any; + + const request = { + isDataSharingAllowed: true, + isMemoValidationEnabled: true, + isHideDustEnabled: true, + isOpenSidebarByDefault: false, + } as any; + + const result = await saveSettings({ request, localStore }); + expect(result.isOpenSidebarByDefault).toBe(false); + // Ensure it's actually a boolean, not the string "false" + expect(typeof result.isOpenSidebarByDefault).toBe("boolean"); + }); +}); diff --git a/extension/src/background/messageListener/__tests__/sidebar.test.ts b/extension/src/background/messageListener/__tests__/sidebar.test.ts new file mode 100644 index 0000000000..c27db5b795 --- /dev/null +++ b/extension/src/background/messageListener/__tests__/sidebar.test.ts @@ -0,0 +1,139 @@ +import { SERVICE_TYPES } from "@shared/constants/services"; +import { popupMessageListener } from "background/messageListener/popupMessageListener"; +import { + setSidebarPort, + clearSidebarPort, +} from "background/messageListener/freighterApiMessageListener"; + +// Mock chrome.sidePanel API +const mockSetOptions = jest.fn().mockResolvedValue(undefined); +const mockOpen = jest.fn().mockResolvedValue(undefined); + +(global as any).chrome = { + sidePanel: { + setOptions: mockSetOptions, + open: mockOpen, + }, + runtime: { getURL: (path: string) => `chrome-extension://fake-id${path}` }, +}; + +const mockSessionStore = { + getState: jest.fn().mockReturnValue({ session: { publicKey: "" } }), +} as any; + +const mockLocalStore = { + getItem: jest.fn().mockResolvedValue(null), + setItem: jest.fn(), +} as any; + +const mockKeyManager = {} as any; +const mockSessionTimer = {} as any; + +describe("sidebar message handlers", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe("OPEN_SIDEBAR", () => { + const request = { + type: SERVICE_TYPES.OPEN_SIDEBAR, + windowId: 42, + }; + + it("returns Unauthorized when sender is a content script (has sender.tab)", async () => { + const contentScriptSender = { tab: { id: 1 } }; + const result = await popupMessageListener( + request as any, + mockSessionStore, + mockLocalStore, + mockKeyManager, + mockSessionTimer, + contentScriptSender, + ); + expect(result).toEqual({ error: "Unauthorized" }); + expect(mockSetOptions).not.toHaveBeenCalled(); + expect(mockOpen).not.toHaveBeenCalled(); + }); + + it("opens the sidebar when sender is an extension page (no sender.tab)", async () => { + const extensionPageSender = {}; + const result = await popupMessageListener( + request as any, + mockSessionStore, + mockLocalStore, + mockKeyManager, + mockSessionTimer, + extensionPageSender, + ); + expect(result).toEqual({}); + expect(mockSetOptions).toHaveBeenCalledWith({ + path: "index.html?mode=sidebar", + enabled: true, + }); + expect(mockOpen).toHaveBeenCalledWith({ windowId: 42 }); + }); + + it("opens the sidebar when no sender is provided", async () => { + const result = await popupMessageListener( + request as any, + mockSessionStore, + mockLocalStore, + mockKeyManager, + mockSessionTimer, + ); + expect(result).toEqual({}); + expect(mockSetOptions).toHaveBeenCalled(); + expect(mockOpen).toHaveBeenCalled(); + }); + }); + + describe("sidebarWindowId management", () => { + it("getSidebarWindowId returns null initially", async () => { + const { + getSidebarWindowId, + clearSidebarWindowId, + } = require("background/messageListener/popupMessageListener"); + clearSidebarWindowId(); + expect(getSidebarWindowId()).toBeNull(); + }); + + it("setSidebarWindowId / clearSidebarWindowId work correctly", () => { + const { + getSidebarWindowId, + clearSidebarWindowId, + setSidebarWindowId, + } = require("background/messageListener/popupMessageListener"); + setSidebarWindowId(99); + expect(getSidebarWindowId()).toBe(99); + clearSidebarWindowId(); + expect(getSidebarWindowId()).toBeNull(); + }); + }); + + describe("sidebarPort management", () => { + afterEach(() => { + clearSidebarPort(); + }); + + it("setSidebarPort stores the port without throwing", () => { + const mockPort = { + postMessage: jest.fn(), + disconnect: jest.fn(), + } as any; + expect(() => setSidebarPort(mockPort)).not.toThrow(); + }); + + it("clearSidebarPort clears without throwing", () => { + const mockPort = { + postMessage: jest.fn(), + disconnect: jest.fn(), + } as any; + setSidebarPort(mockPort); + expect(() => clearSidebarPort()).not.toThrow(); + }); + + it("clearSidebarPort is safe to call when no port is set", () => { + expect(() => clearSidebarPort()).not.toThrow(); + }); + }); +}); diff --git a/extension/src/background/messageListener/freighterApiMessageListener.ts b/extension/src/background/messageListener/freighterApiMessageListener.ts index bccf0eea8d..2bed00ae7b 100644 --- a/extension/src/background/messageListener/freighterApiMessageListener.ts +++ b/extension/src/background/messageListener/freighterApiMessageListener.ts @@ -24,7 +24,10 @@ import { FreighterApiInternalError, FreighterApiDeclinedError, } from "@shared/api/helpers/extensionMessaging"; -import { EXTERNAL_SERVICE_TYPES } from "@shared/constants/services"; +import { + EXTERNAL_SERVICE_TYPES, + SIDEBAR_NAVIGATE, +} from "@shared/constants/services"; import { MAINNET_NETWORK_DETAILS, NetworkDetails, @@ -68,12 +71,24 @@ import { } from "./popupMessageListener"; import { QUEUE_ITEM_TTL_MS } from "background/helpers/queueCleanup"; -const SIDEBAR_NAVIGATE = "SIDEBAR_NAVIGATE"; +// Long-lived port to the sidebar, set by initSidebarConnectionListener +let sidebarPort: browser.Runtime.Port | null = null; + +export const setSidebarPort = (port: browser.Runtime.Port) => { + sidebarPort = port; +}; +export const clearSidebarPort = () => { + sidebarPort = null; +}; const openSigningWindow = async (hashRoute: string, width?: number) => { const sidebarWindowId = getSidebarWindowId(); if (sidebarWindowId !== null) { - browser.runtime.sendMessage({ type: SIDEBAR_NAVIGATE, route: hashRoute }); + // Send navigation directly to the sidebar via its long-lived port + // instead of broadcasting to all extension listeners + if (sidebarPort) { + sidebarPort.postMessage({ type: SIDEBAR_NAVIGATE, route: hashRoute }); + } try { if ((browser as any).sidebarAction) { // Firefox @@ -88,7 +103,7 @@ const openSigningWindow = async (hashRoute: string, width?: number) => { return null; } return browser.windows.create({ - url: chrome.runtime.getURL(`/index.html#${hashRoute}`), + url: browser.runtime.getURL(`/index.html#${hashRoute}`), ...WINDOW_SETTINGS, ...(width !== undefined ? { width } : {}), }); @@ -138,10 +153,17 @@ export const freighterApiMessageListener = ( const popup = await openSigningWindow(`/grant-access?${encodeOrigin}`); return new Promise((resolve) => { + let resolved = false; + const safeResolve = (value: any) => { + if (resolved) return; + resolved = true; + resolve(value); + }; + if (popup === null) { setTimeout( () => - resolve({ + safeResolve({ apiError: FreighterApiDeclinedError, error: FreighterApiDeclinedError.message, }), @@ -164,10 +186,11 @@ export const freighterApiMessageListener = ( } }, 50); - resolve({ publicKey }); + safeResolve({ publicKey }); + return; } - resolve({ + safeResolve({ // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface apiError: FreighterApiDeclinedError, error: FreighterApiDeclinedError.message, @@ -236,25 +259,33 @@ export const freighterApiMessageListener = ( const popup = await openSigningWindow(`/add-token?${encodedTokenInfo}`); return new Promise((resolve) => { + let resolved = false; + const safeResolve = (value: any) => { + if (resolved) return; + resolved = true; + resolve(value); + }; + if (popup === undefined) { - resolve({ + safeResolve({ apiError: FreighterApiInternalError, }); } else if (popup !== null) { browser.windows.onRemoved.addListener(() => - resolve({ + safeResolve({ apiError: FreighterApiDeclinedError, }), ); } const response = (success: boolean) => { if (success) { - resolve({ + safeResolve({ contractId, }); + return; } - resolve({ + safeResolve({ apiError: FreighterApiDeclinedError, }); }; @@ -383,8 +414,15 @@ export const freighterApiMessageListener = ( const popup = await openSigningWindow(`/sign-transaction?${encodedBlob}`); return new Promise((resolve) => { + let resolved = false; + const safeResolve = (value: any) => { + if (resolved) return; + resolved = true; + resolve(value); + }; + if (popup === undefined) { - resolve({ + safeResolve({ // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface apiError: FreighterApiInternalError, error: FreighterApiInternalError.message, @@ -392,7 +430,7 @@ export const freighterApiMessageListener = ( } else if (popup === null) { setTimeout( () => - resolve({ + safeResolve({ apiError: FreighterApiDeclinedError, error: FreighterApiDeclinedError.message, }), @@ -400,7 +438,7 @@ export const freighterApiMessageListener = ( ); } else { browser.windows.onRemoved.addListener(() => - resolve({ + safeResolve({ // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface apiError: FreighterApiDeclinedError, error: FreighterApiDeclinedError.message, @@ -412,10 +450,11 @@ export const freighterApiMessageListener = ( signerAddress?: string, ) => { if (signedTransaction) { - resolve({ signedTransaction, signerAddress }); + safeResolve({ signedTransaction, signerAddress }); + return; } - resolve({ + safeResolve({ // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface apiError: FreighterApiDeclinedError, error: FreighterApiDeclinedError.message, @@ -464,8 +503,15 @@ export const freighterApiMessageListener = ( const popup = await openSigningWindow(`/sign-message?${encodedBlob}`); return new Promise((resolve) => { + let resolved = false; + const safeResolve = (value: any) => { + if (resolved) return; + resolved = true; + resolve(value); + }; + if (popup === undefined) { - resolve({ + safeResolve({ // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface apiError: FreighterApiInternalError, error: FreighterApiInternalError.message, @@ -473,7 +519,7 @@ export const freighterApiMessageListener = ( } else if (popup === null) { setTimeout( () => - resolve({ + safeResolve({ apiError: FreighterApiDeclinedError, error: FreighterApiDeclinedError.message, }), @@ -481,7 +527,7 @@ export const freighterApiMessageListener = ( ); } else { browser.windows.onRemoved.addListener(() => - resolve({ + safeResolve({ // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface apiError: FreighterApiDeclinedError, error: FreighterApiDeclinedError.message, @@ -495,16 +541,17 @@ export const freighterApiMessageListener = ( ) => { if (signedBlob) { if (apiVersion && semver.gte(apiVersion, "4.0.0")) { - resolve({ + safeResolve({ signedBlob: signedBlob.toString("base64"), signerAddress, }); return; } - resolve({ signedBlob, signerAddress }); + safeResolve({ signedBlob, signerAddress }); + return; } - resolve({ + safeResolve({ // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface apiError: FreighterApiDeclinedError, error: FreighterApiDeclinedError.message, @@ -559,8 +606,15 @@ export const freighterApiMessageListener = ( ); return new Promise((resolve) => { + let resolved = false; + const safeResolve = (value: any) => { + if (resolved) return; + resolved = true; + resolve(value); + }; + if (popup === undefined) { - resolve({ + safeResolve({ // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface apiError: FreighterApiInternalError, error: FreighterApiInternalError.message, @@ -568,7 +622,7 @@ export const freighterApiMessageListener = ( } else if (popup === null) { setTimeout( () => - resolve({ + safeResolve({ apiError: FreighterApiDeclinedError, error: FreighterApiDeclinedError.message, }), @@ -576,7 +630,7 @@ export const freighterApiMessageListener = ( ); } else { browser.windows.onRemoved.addListener(() => - resolve({ + safeResolve({ // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface apiError: FreighterApiDeclinedError, error: FreighterApiDeclinedError.message, @@ -589,17 +643,18 @@ export const freighterApiMessageListener = ( ) => { if (signedAuthEntry) { if (apiVersion && semver.gte(apiVersion, "4.2.0")) { - resolve({ + safeResolve({ signedAuthEntry: Buffer.from(signedAuthEntry).toString("base64"), signerAddress, }); return; } - resolve({ signedAuthEntry, signerAddress }); + safeResolve({ signedAuthEntry, signerAddress }); + return; } - resolve({ + safeResolve({ // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface apiError: FreighterApiDeclinedError, error: FreighterApiDeclinedError.message, @@ -701,9 +756,16 @@ export const freighterApiMessageListener = ( await openSigningWindow(`/grant-access?${encodeOrigin}`, 400); return new Promise((resolve) => { + let resolved = false; + const safeResolve = (value: any) => { + if (resolved) return; + resolved = true; + resolve(value); + }; + setTimeout( () => - resolve({ + safeResolve({ apiError: FreighterApiDeclinedError, error: FreighterApiDeclinedError.message, }), @@ -722,10 +784,11 @@ export const freighterApiMessageListener = ( allowListSegment: updatedAllAccountsllowListSegment, }); - resolve({ isAllowed: isAllowedResponse }); + safeResolve({ isAllowed: isAllowedResponse }); + return; } - resolve({ + safeResolve({ // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface apiError: FreighterApiDeclinedError, error: FreighterApiDeclinedError.message, diff --git a/extension/src/background/messageListener/handlers/loadSettings.ts b/extension/src/background/messageListener/handlers/loadSettings.ts index 5f53b747fb..7ba8fcb67e 100644 --- a/extension/src/background/messageListener/handlers/loadSettings.ts +++ b/extension/src/background/messageListener/handlers/loadSettings.ts @@ -32,8 +32,7 @@ export const loadSettings = async ({ const isNonSSLEnabled = await getIsNonSSLEnabled({ localStore }); const isHideDustEnabled = await getIsHideDustEnabled({ localStore }); const isOpenSidebarByDefault = - ((await localStore.getItem(IS_OPEN_SIDEBAR_BY_DEFAULT_ID)) as boolean) ?? - false; + (await localStore.getItem(IS_OPEN_SIDEBAR_BY_DEFAULT_ID)) === "true"; const { hiddenAssets } = await getHiddenAssets({ localStore }); const overriddenBlockaidResponse = await getOverriddenBlockaidResponse({ localStore, diff --git a/extension/src/background/messageListener/handlers/saveSettings.ts b/extension/src/background/messageListener/handlers/saveSettings.ts index d58e0260b2..7e7cdea931 100644 --- a/extension/src/background/messageListener/handlers/saveSettings.ts +++ b/extension/src/background/messageListener/handlers/saveSettings.ts @@ -59,8 +59,7 @@ export const saveSettings = async ({ isSorobanPublicEnabled: featureFlags.useSorobanPublic, isNonSSLEnabled: await getIsNonSSLEnabled({ localStore }), isHideDustEnabled: await getIsHideDustEnabled({ localStore }), - isOpenSidebarByDefault: (await localStore.getItem( - IS_OPEN_SIDEBAR_BY_DEFAULT_ID, - )) as boolean ?? false, + isOpenSidebarByDefault: + (await localStore.getItem(IS_OPEN_SIDEBAR_BY_DEFAULT_ID)) === "true", }; }; diff --git a/extension/src/background/messageListener/popupMessageListener.ts b/extension/src/background/messageListener/popupMessageListener.ts index 558450ac21..2d83c9c52a 100644 --- a/extension/src/background/messageListener/popupMessageListener.ts +++ b/extension/src/background/messageListener/popupMessageListener.ts @@ -1,3 +1,4 @@ +import browser from "webextension-polyfill"; import { Store } from "redux"; import { ResponseQueue, @@ -16,7 +17,6 @@ import { RejectTransactionResponse, SignedHwPayloadResponse, MarkQueueActiveMessage, - SidebarRegisterMessage, OpenSidebarMessage, } from "@shared/api/types/message-request"; import { SERVICE_TYPES } from "@shared/constants/services"; @@ -135,10 +135,18 @@ export const popupMessageListener = ( localStore: DataStorageAccess, keyManager: KeyManager, sessionTimer: SessionTimer, + sender?: { tab?: unknown; id?: string }, ) => { const currentState = sessionStore.getState(); const publicKey = publicKeySelector(currentState); + // Content scripts (dapp pages) always carry sender.tab; extension pages do not. + // Also verify the message originates from this extension (sender.id matches), + // guarding against other extensions calling popupMessageListener handlers. + // When sender is absent (internal calls), both checks pass by default. + const isFromExtensionPage = + !sender?.tab && (!sender?.id || sender.id === browser.runtime.id); + if ( request.activePublicKey && request.activePublicKey !== publicKey && @@ -566,6 +574,7 @@ export const popupMessageListener = ( } case SERVICE_TYPES.OPEN_SIDEBAR: { + if (!isFromExtensionPage) return { error: "Unauthorized" }; const { windowId } = request as OpenSidebarMessage; return (async () => { await chrome.sidePanel @@ -578,16 +587,6 @@ export const popupMessageListener = ( })(); } - case SERVICE_TYPES.SIDEBAR_REGISTER: { - sidebarWindowId = (request as SidebarRegisterMessage).windowId; - return {}; - } - - case SERVICE_TYPES.SIDEBAR_UNREGISTER: { - sidebarWindowId = null; - return {}; - } - default: return { error: "Message type not supported" }; } diff --git a/extension/src/popup/components/SidebarSigningListener/index.tsx b/extension/src/popup/components/SidebarSigningListener/index.tsx index 14e2408186..394590b47c 100644 --- a/extension/src/popup/components/SidebarSigningListener/index.tsx +++ b/extension/src/popup/components/SidebarSigningListener/index.tsx @@ -1,8 +1,9 @@ import { useEffect } from "react"; import { useNavigate } from "react-router-dom"; +import browser from "webextension-polyfill"; import { ROUTES } from "popup/constants/routes"; +import { SIDEBAR_NAVIGATE } from "@shared/constants/services"; -const SIDEBAR_NAVIGATE = "SIDEBAR_NAVIGATE"; export const SIDEBAR_PORT_NAME = "sidebar"; export const SidebarSigningListener = () => { @@ -11,10 +12,10 @@ export const SidebarSigningListener = () => { useEffect(() => { // Open a long-lived port to the background. // The background uses onDisconnect to reliably clear sidebarWindowId when sidebar closes. - const port = chrome.runtime.connect({ name: SIDEBAR_PORT_NAME }); + const port = browser.runtime.connect({ name: SIDEBAR_PORT_NAME }); // Send window ID so the background can register this sidebar - chrome.windows.getCurrent().then((win) => { + browser.windows.getCurrent().then((win) => { port.postMessage({ windowId: win.id }); }); @@ -22,17 +23,20 @@ export const SidebarSigningListener = () => { const originalClose = window.close.bind(window); window.close = () => navigate(ROUTES.account); - const handler = (message: { type: string; route: string }) => { - if (message.type === SIDEBAR_NAVIGATE) { - navigate(message.route); + // Listen for navigation messages sent directly over the port from the + // background, scoped to this sidebar only (no broadcast to other listeners). + const portHandler = (message: unknown) => { + const { type, route } = message as { type: string; route: string }; + if (type === SIDEBAR_NAVIGATE) { + navigate(route); } }; - chrome.runtime.onMessage.addListener(handler); + port.onMessage.addListener(portHandler); return () => { window.close = originalClose; - chrome.runtime.onMessage.removeListener(handler); + port.onMessage.removeListener(portHandler); port.disconnect(); }; }, [navigate]); diff --git a/extension/src/popup/components/account/AccountHeader/index.tsx b/extension/src/popup/components/account/AccountHeader/index.tsx index ee907d06a4..efc58d6f68 100644 --- a/extension/src/popup/components/account/AccountHeader/index.tsx +++ b/extension/src/popup/components/account/AccountHeader/index.tsx @@ -2,6 +2,7 @@ import React, { useEffect, useRef, useState } from "react"; import { useDispatch, useSelector } from "react-redux"; import { useNavigate, NavLink } from "react-router-dom"; import { createPortal } from "react-dom"; +import browser from "webextension-polyfill"; import { Icon, Text, NavButton, CopyText } from "@stellar/design-system"; import { useTranslation } from "react-i18next"; @@ -181,7 +182,10 @@ export const AccountHeader = ({ - {typeof globalThis.chrome?.sidePanel?.open === "function" && ( + {(typeof globalThis.chrome?.sidePanel?.open === + "function" || + typeof (browser as any)?.sidebarAction?.open === + "function") && (
openSidebar()} diff --git a/extension/src/popup/helpers/isSidebarMode.ts b/extension/src/popup/helpers/isSidebarMode.ts index 1100522554..998e2ff933 100644 --- a/extension/src/popup/helpers/isSidebarMode.ts +++ b/extension/src/popup/helpers/isSidebarMode.ts @@ -1,2 +1,5 @@ +// Guard against non-extension contexts: extension pages always load from +// chrome-extension:// (or moz-extension://), never from web origins. export const isSidebarMode = () => + /^(chrome|moz)-extension:$/.test(window.location.protocol) && new URLSearchParams(window.location.search).get("mode") === "sidebar"; diff --git a/extension/src/popup/helpers/navigate.ts b/extension/src/popup/helpers/navigate.ts index dedbef588a..4c185b79ca 100644 --- a/extension/src/popup/helpers/navigate.ts +++ b/extension/src/popup/helpers/navigate.ts @@ -29,8 +29,8 @@ export const openSidebar = async () => { }); await chrome.sidePanel.open({ windowId: win.id! }); } + window.close(); } catch (e) { console.error("Failed to open sidebar:", e); } - window.close(); }; diff --git a/extension/src/popup/locales/en/translation.json b/extension/src/popup/locales/en/translation.json index 9dcf9861ac..2a61d8188b 100644 --- a/extension/src/popup/locales/en/translation.json +++ b/extension/src/popup/locales/en/translation.json @@ -390,6 +390,8 @@ "Only confirm if you trust this site": "Only confirm if you trust this site", "Only utilize these features if you can understand and manage the potential security risks": "Only utilize these features if you can understand and manage the potential security risks.", "Open": "Open", + "Open Freighter in sidebar instead of popup when clicking the extension icon": "Open Freighter in sidebar instead of popup when clicking the extension icon", + "Open sidebar mode by default": "Open sidebar mode by default", "Open status page": "Open status page", "Operation": "Operation", "Operations": "Operations", @@ -484,6 +486,7 @@ "Should be benign": "Should be benign", "Show collectible": "Show collectible", "Show recovery phrase": "Show recovery phrase", + "Sidebar mode": "Sidebar mode", "Signed Payload": "Signed Payload", "Signer": "Signer", "Signer Key": "Signer Key", diff --git a/extension/src/popup/locales/pt/translation.json b/extension/src/popup/locales/pt/translation.json index 6d79ea39a2..5336ec6d57 100644 --- a/extension/src/popup/locales/pt/translation.json +++ b/extension/src/popup/locales/pt/translation.json @@ -392,6 +392,8 @@ "Only confirm if you trust this site": "Apenas confirme se você confia neste site", "Only utilize these features if you can understand and manage the potential security risks": "Utilize esses recursos apenas se você puder entender e gerenciar os riscos de segurança potenciais.", "Open": "Abrir", + "Open Freighter in sidebar instead of popup when clicking the extension icon": "Open Freighter in sidebar instead of popup when clicking the extension icon", + "Open sidebar mode by default": "Open sidebar mode by default", "Open status page": "Abrir página de status", "Operation": "Operação", "Operations": "Operações", @@ -486,6 +488,7 @@ "Should be benign": "Deve ser benigno", "Show collectible": "Mostrar colecionável", "Show recovery phrase": "Mostrar frase de recuperação", + "Sidebar mode": "Sidebar mode", "Signed Payload": "Payload Assinado", "Signer": "Assinante", "Signer Key": "Chave do Assinante", From b37b32b9f9b1966d1909426f45db6b67f92f904b Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 1 Apr 2026 13:41:57 -0400 Subject: [PATCH 02/24] security: add interstitial gate and route allowlist for sidebar signing flow Prevent rapid-fire signing phishing in sidebar mode by showing an interstitial screen when a new signing request arrives while the user is already reviewing one. Also hardens the SidebarSigningListener with a route allowlist (only known signing routes are navigable) and runtime type guards on port messages. Co-Authored-By: Claude Opus 4.6 (1M context) --- extension/src/popup/Router.tsx | 5 ++ .../SidebarSigningListener/index.tsx | 47 +++++++++++++- extension/src/popup/constants/metricsNames.ts | 1 + extension/src/popup/constants/routes.ts | 1 + .../src/popup/locales/en/translation.json | 4 ++ .../src/popup/locales/pt/translation.json | 4 ++ extension/src/popup/metrics/views.ts | 1 + .../views/ConfirmSidebarRequest/index.tsx | 64 +++++++++++++++++++ .../views/ConfirmSidebarRequest/styles.scss | 49 ++++++++++++++ 9 files changed, 174 insertions(+), 2 deletions(-) create mode 100644 extension/src/popup/views/ConfirmSidebarRequest/index.tsx create mode 100644 extension/src/popup/views/ConfirmSidebarRequest/styles.scss diff --git a/extension/src/popup/Router.tsx b/extension/src/popup/Router.tsx index 93bd7d6250..5edee2e6d0 100644 --- a/extension/src/popup/Router.tsx +++ b/extension/src/popup/Router.tsx @@ -62,6 +62,7 @@ import { AccountMigration } from "popup/views/AccountMigration"; import { AddFunds } from "popup/views/AddFunds"; import { Discover } from "popup/views/Discover"; import { Wallets } from "popup/views/Wallets"; +import { ConfirmSidebarRequest } from "popup/views/ConfirmSidebarRequest"; import { DEV_SERVER } from "@shared/constants/services"; import { isSidebarMode } from "popup/helpers/isSidebarMode"; @@ -205,6 +206,10 @@ export const Router = () => ( element={} > }> + } + > } diff --git a/extension/src/popup/components/SidebarSigningListener/index.tsx b/extension/src/popup/components/SidebarSigningListener/index.tsx index 394590b47c..fc889056e8 100644 --- a/extension/src/popup/components/SidebarSigningListener/index.tsx +++ b/extension/src/popup/components/SidebarSigningListener/index.tsx @@ -6,6 +6,29 @@ import { SIDEBAR_NAVIGATE } from "@shared/constants/services"; export const SIDEBAR_PORT_NAME = "sidebar"; +// Routes that represent active signing/approval flows. +// When a new SIDEBAR_NAVIGATE arrives while the user is already on one of +// these, we show an interstitial instead of silently swapping the screen. +const SIGNING_ROUTE_PREFIXES = [ + ROUTES.signTransaction, + ROUTES.signAuthEntry, + ROUTES.signMessage, + ROUTES.grantAccess, + ROUTES.addToken, + ROUTES.reviewAuthorization, + ROUTES.confirmSidebarRequest, +]; + +// Only allow navigation to known signing-related routes (defense-in-depth). +const ALLOWED_NAV_PREFIXES = [ + ROUTES.signTransaction, + ROUTES.signAuthEntry, + ROUTES.signMessage, + ROUTES.grantAccess, + ROUTES.addToken, + ROUTES.reviewAuthorization, +]; + export const SidebarSigningListener = () => { const navigate = useNavigate(); @@ -26,8 +49,28 @@ export const SidebarSigningListener = () => { // Listen for navigation messages sent directly over the port from the // background, scoped to this sidebar only (no broadcast to other listeners). const portHandler = (message: unknown) => { - const { type, route } = message as { type: string; route: string }; - if (type === SIDEBAR_NAVIGATE) { + if (typeof message !== "object" || message === null) return; + const { type, route } = message as Record; + if (type !== SIDEBAR_NAVIGATE || typeof route !== "string") return; + + // Only allow navigation to known signing routes + if (!ALLOWED_NAV_PREFIXES.some((prefix) => route.startsWith(prefix))) { + return; + } + + // If the user is already reviewing a signing request, show an + // interstitial so they consciously acknowledge the new request + // rather than having the screen silently swap underneath them. + const currentHash = window.location.hash.replace("#", ""); + const isOnSigningRoute = SIGNING_ROUTE_PREFIXES.some((prefix) => + currentHash.startsWith(prefix), + ); + + if (isOnSigningRoute) { + navigate( + `${ROUTES.confirmSidebarRequest}?next=${encodeURIComponent(route)}`, + ); + } else { navigate(route); } }; diff --git a/extension/src/popup/constants/metricsNames.ts b/extension/src/popup/constants/metricsNames.ts index 675e8b3969..05768159a4 100644 --- a/extension/src/popup/constants/metricsNames.ts +++ b/extension/src/popup/constants/metricsNames.ts @@ -182,4 +182,5 @@ export const METRIC_NAMES = { coinbaseOnrampOpened: "coinbase onramp: opened", wallets: "loaded screen: wallets", + confirmSidebarRequest: "loaded screen: confirmSidebarRequest", }; diff --git a/extension/src/popup/constants/routes.ts b/extension/src/popup/constants/routes.ts index 69a65d11af..5cc991f943 100644 --- a/extension/src/popup/constants/routes.ts +++ b/extension/src/popup/constants/routes.ts @@ -53,6 +53,7 @@ export enum ROUTES { accountMigrationConfirmMigration = "/account-migration/confirm-migration", accountMigrationMigrationComplete = "/account-migration/migration-complete", + confirmSidebarRequest = "/confirm-sidebar-request", discover = "/discover", wallets = "/wallets", } diff --git a/extension/src/popup/locales/en/translation.json b/extension/src/popup/locales/en/translation.json index 2a61d8188b..a187ee0095 100644 --- a/extension/src/popup/locales/en/translation.json +++ b/extension/src/popup/locales/en/translation.json @@ -3,6 +3,7 @@ "* All Stellar accounts must maintain a minimum balance of lumens.": "* All Stellar accounts must maintain a minimum balance of lumens.", "* payment methods may vary based on your location": "* payment methods may vary based on your location", "A destination account requires the use of the memo field which is not present in the transaction you’re about to sign.": "A destination account requires the use of the memo field which is not present in the transaction you’re about to sign.", + "A new signing request arrived while you were reviewing another. Please review it carefully before approving.": "A new signing request arrived while you were reviewing another. Please review it carefully before approving.", "About": "About", "Account": "Account", "Account details": "Account details", @@ -366,6 +367,7 @@ "Nevermind, cancel": "Nevermind, cancel", "New account": "New account", "New password": "New password", + "New Signing Request": "New Signing Request", "No": "No", "No collectibles yet": "No collectibles yet", "No connected apps found": "No connected apps found", @@ -435,6 +437,7 @@ "Recovery Phrase": "Recovery Phrase", "Refresh": "Refresh", "Refresh metadata": "Refresh metadata", + "Reject": "Reject", "Remember, Freighter will now display accounts related to the new backup phrase that was just created.": "Remember, Freighter will now display accounts related to the new backup phrase that was just created.", "Remove": "Remove", "Remove asset": "Remove asset", @@ -446,6 +449,7 @@ "Report issue on Github": "Report issue on Github", "Reserved Balance*": "Reserved Balance*", "Review accounts to migrate": "Review accounts to migrate", + "Review Request": "Review Request", "Review Send": "Review Send", "Review swap": "Review swap", "Review transaction on device": "Review transaction on device", diff --git a/extension/src/popup/locales/pt/translation.json b/extension/src/popup/locales/pt/translation.json index 5336ec6d57..e7279801ac 100644 --- a/extension/src/popup/locales/pt/translation.json +++ b/extension/src/popup/locales/pt/translation.json @@ -3,6 +3,7 @@ "* All Stellar accounts must maintain a minimum balance of lumens.": "* Todas as contas Stellar devem manter um saldo mínimo de lumens.", "* payment methods may vary based on your location": "* Os métodos de pagamento podem variar com base na sua localização", "A destination account requires the use of the memo field which is not present in the transaction you’re about to sign.": "Uma conta de destino requer o uso do campo memo que não está presente na transação que você está prestes a assinar.", + "A new signing request arrived while you were reviewing another. Please review it carefully before approving.": "A new signing request arrived while you were reviewing another. Please review it carefully before approving.", "About": "Sobre", "Account": "Conta", "Account details": "Detalhes da conta", @@ -368,6 +369,7 @@ "Nevermind, cancel": "Deixa pra lá, cancelar", "New account": "Nova conta", "New password": "Nova senha", + "New Signing Request": "New Signing Request", "No": "Não", "No collectibles yet": "Ainda não há colecionáveis", "No connected apps found": "Nenhum app conectado encontrado", @@ -437,6 +439,7 @@ "Recovery Phrase": "Frase de Recuperação", "Refresh": "Atualizar", "Refresh metadata": "Atualizar metadados", + "Reject": "Reject", "Remember, Freighter will now display accounts related to the new backup phrase that was just created.": "Lembre-se, o Freighter agora exibirá contas relacionadas à nova frase de backup que acabou de ser criada.", "Remove": "Remover", "Remove asset": "Remover ativo", @@ -448,6 +451,7 @@ "Report issue on Github": "Reportar issue no Github", "Reserved Balance*": "Saldo Reservado*", "Review accounts to migrate": "Revisar contas para migrar", + "Review Request": "Review Request", "Review Send": "Revisar Envio", "Review swap": "Revisar troca", "Review transaction on device": "Revisar transação no dispositivo", diff --git a/extension/src/popup/metrics/views.ts b/extension/src/popup/metrics/views.ts index 0d63f6fa91..367d5003f4 100644 --- a/extension/src/popup/metrics/views.ts +++ b/extension/src/popup/metrics/views.ts @@ -68,6 +68,7 @@ const routeToEventName = { [ROUTES.addFunds]: METRIC_NAMES.viewAddFunds, [ROUTES.discover]: METRIC_NAMES.discover, [ROUTES.wallets]: METRIC_NAMES.wallets, + [ROUTES.confirmSidebarRequest]: METRIC_NAMES.confirmSidebarRequest, }; registerHandler(navigate, (_, a) => { diff --git a/extension/src/popup/views/ConfirmSidebarRequest/index.tsx b/extension/src/popup/views/ConfirmSidebarRequest/index.tsx new file mode 100644 index 0000000000..85f931e012 --- /dev/null +++ b/extension/src/popup/views/ConfirmSidebarRequest/index.tsx @@ -0,0 +1,64 @@ +import React from "react"; +import { useLocation, useNavigate } from "react-router-dom"; +import { useTranslation } from "react-i18next"; +import { Button, Icon } from "@stellar/design-system"; + +import { ROUTES } from "popup/constants/routes"; +import { View } from "popup/basics/layout/View"; + +import "./styles.scss"; + +export const ConfirmSidebarRequest = () => { + const { t } = useTranslation(); + const location = useLocation(); + const navigate = useNavigate(); + + const params = new URLSearchParams(location.search); + const next = params.get("next") || ""; + + const handleReview = () => { + if (next) { + navigate(next); + } + }; + + const handleReject = () => { + navigate(ROUTES.account); + }; + + return ( + +
+
+ +
+

+ {t("New Signing Request")} +

+

+ {t( + "A new signing request arrived while you were reviewing another. Please review it carefully before approving.", + )} +

+
+ + +
+
+ + ); +}; diff --git a/extension/src/popup/views/ConfirmSidebarRequest/styles.scss b/extension/src/popup/views/ConfirmSidebarRequest/styles.scss new file mode 100644 index 0000000000..7b3791b708 --- /dev/null +++ b/extension/src/popup/views/ConfirmSidebarRequest/styles.scss @@ -0,0 +1,49 @@ +@use "../../styles/utils.scss" as *; + +.ConfirmSidebarRequest { + display: flex; + flex-direction: column; + align-items: center; + text-align: center; + padding: pxToRem(48px) pxToRem(24px); + gap: pxToRem(16px); + + &__icon { + display: flex; + align-items: center; + justify-content: center; + width: pxToRem(56px); + height: pxToRem(56px); + border-radius: 50%; + background-color: var(--sds-clr-amber-03); + color: var(--sds-clr-amber-11); + + svg { + width: pxToRem(28px); + height: pxToRem(28px); + } + } + + &__title { + font-size: pxToRem(20px); + font-weight: 600; + color: var(--sds-clr-gray-12); + margin: 0; + } + + &__body { + font-size: pxToRem(14px); + color: var(--sds-clr-gray-11); + line-height: 1.5; + margin: 0; + max-width: pxToRem(320px); + } + + &__buttons { + display: flex; + flex-direction: column; + gap: pxToRem(8px); + width: 100%; + margin-top: pxToRem(16px); + } +} From dd8c83436c3bd3a6c25b352bb4d7688fa215885d Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 1 Apr 2026 13:48:39 -0400 Subject: [PATCH 03/24] fix: add missing Portuguese translations for sidebar mode strings Co-Authored-By: Claude Opus 4.6 (1M context) --- extension/src/popup/locales/pt/translation.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/extension/src/popup/locales/pt/translation.json b/extension/src/popup/locales/pt/translation.json index e7279801ac..4e62e43832 100644 --- a/extension/src/popup/locales/pt/translation.json +++ b/extension/src/popup/locales/pt/translation.json @@ -3,7 +3,7 @@ "* All Stellar accounts must maintain a minimum balance of lumens.": "* Todas as contas Stellar devem manter um saldo mínimo de lumens.", "* payment methods may vary based on your location": "* Os métodos de pagamento podem variar com base na sua localização", "A destination account requires the use of the memo field which is not present in the transaction you’re about to sign.": "Uma conta de destino requer o uso do campo memo que não está presente na transação que você está prestes a assinar.", - "A new signing request arrived while you were reviewing another. Please review it carefully before approving.": "A new signing request arrived while you were reviewing another. Please review it carefully before approving.", + "A new signing request arrived while you were reviewing another. Please review it carefully before approving.": "Uma nova solicitação de assinatura chegou enquanto você revisava outra. Revise-a com atenção antes de aprovar.", "About": "Sobre", "Account": "Conta", "Account details": "Detalhes da conta", @@ -369,7 +369,7 @@ "Nevermind, cancel": "Deixa pra lá, cancelar", "New account": "Nova conta", "New password": "Nova senha", - "New Signing Request": "New Signing Request", + "New Signing Request": "Nova Solicitação de Assinatura", "No": "Não", "No collectibles yet": "Ainda não há colecionáveis", "No connected apps found": "Nenhum app conectado encontrado", @@ -394,8 +394,8 @@ "Only confirm if you trust this site": "Apenas confirme se você confia neste site", "Only utilize these features if you can understand and manage the potential security risks": "Utilize esses recursos apenas se você puder entender e gerenciar os riscos de segurança potenciais.", "Open": "Abrir", - "Open Freighter in sidebar instead of popup when clicking the extension icon": "Open Freighter in sidebar instead of popup when clicking the extension icon", - "Open sidebar mode by default": "Open sidebar mode by default", + "Open Freighter in sidebar instead of popup when clicking the extension icon": "Abrir o Freighter na barra lateral em vez do popup ao clicar no ícone da extensão", + "Open sidebar mode by default": "Abrir no modo barra lateral por padrão", "Open status page": "Abrir página de status", "Operation": "Operação", "Operations": "Operações", @@ -451,7 +451,7 @@ "Report issue on Github": "Reportar issue no Github", "Reserved Balance*": "Saldo Reservado*", "Review accounts to migrate": "Revisar contas para migrar", - "Review Request": "Review Request", + "Review Request": "Revisar Solicitação", "Review Send": "Revisar Envio", "Review swap": "Revisar troca", "Review transaction on device": "Revisar transação no dispositivo", @@ -492,7 +492,7 @@ "Should be benign": "Deve ser benigno", "Show collectible": "Mostrar colecionável", "Show recovery phrase": "Mostrar frase de recuperação", - "Sidebar mode": "Sidebar mode", + "Sidebar mode": "Modo barra lateral", "Signed Payload": "Payload Assinado", "Signer": "Assinante", "Signer Key": "Chave do Assinante", From d9fe266fcde86f036eb30c6818d9a538bdc09198 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 1 Apr 2026 17:01:13 -0400 Subject: [PATCH 04/24] =?UTF-8?q?fix:=20remove=20Firefox=20"open=20sidebar?= =?UTF-8?q?=20by=20default"=20=E2=80=94=20not=20supported=20by=20platform?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Firefox requires sidebarAction.open() in a synchronous user gesture handler and has no equivalent to Chrome's setPanelBehavior. Remove the broken setPopup/onClicked approach, hide the preference toggle on Firefox, add open_at_install: false to prevent auto-opening, and fix boolean storage reads for isOpenSidebarByDefault. Co-Authored-By: Claude Opus 4.6 (1M context) --- extension/public/static/manifest/v3.json | 3 ++- extension/src/background/index.ts | 27 ++++--------------- .../messageListener/handlers/loadSettings.ts | 3 ++- .../messageListener/handlers/saveSettings.ts | 3 ++- 4 files changed, 11 insertions(+), 25 deletions(-) diff --git a/extension/public/static/manifest/v3.json b/extension/public/static/manifest/v3.json index 8653aff462..59742b4d1c 100644 --- a/extension/public/static/manifest/v3.json +++ b/extension/public/static/manifest/v3.json @@ -43,7 +43,8 @@ "default_icon": { "16": "images/icon16.png", "32": "images/icon32.png" - } + }, + "open_at_install": false }, "manifest_version": 3 } diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index 16580c57d5..231e8705da 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -191,38 +191,21 @@ export const initInstalledListener = () => { export const initSidebarBehavior = async () => { const localStore = dataStorageAccess(browserLocalStorage); - // getItem returns a string from localStorage; compare explicitly to avoid - // any truthy string (e.g. "false") being coerced to true. const val = - (await localStore.getItem(IS_OPEN_SIDEBAR_BY_DEFAULT_ID)) === "true"; + ((await localStore.getItem(IS_OPEN_SIDEBAR_BY_DEFAULT_ID)) as boolean) ?? + false; if (chrome.sidePanel?.setPanelBehavior) { // Chrome: delegate action-click to the side panel when enabled chrome.sidePanel .setPanelBehavior({ openPanelOnActionClick: val }) .catch((e) => console.error("Failed to set panel behavior:", e)); - } else if ((browser as any).sidebarAction) { - // Firefox: when "open by default" is on, clear the browser-action popup so - // clicks fire onClicked instead of opening the popup, and open the sidebar. - // When off, restore the popup so the normal popup opens on click. - if ((browser as any).browserAction?.setPopup) { - await (browser as any).browserAction - .setPopup({ popup: val ? "" : "index.html" }) - .catch((e: unknown) => - console.error("Failed to set browser action popup:", e), - ); - } } + // Firefox does not support "open sidebar by default" — sidebarAction.open() + // requires a synchronous user gesture and there is no setPanelBehavior equivalent. + // Users can still open the sidebar manually via the AccountHeader menu. }; export const initAlarmListener = () => { - // Firefox: when "open by default" is on the browser-action popup is cleared, - // so clicks fire onClicked. Open the sidebar in response. - if ((browser as any).browserAction?.onClicked) { - (browser as any).browserAction.onClicked.addListener(() => { - (browser as any).sidebarAction?.open?.(); - }); - } - browser?.alarms?.onAlarm.addListener(async ({ name }: { name: string }) => { const sessionStore = await buildStore(); const localStore = dataStorageAccess(browserLocalStorage); diff --git a/extension/src/background/messageListener/handlers/loadSettings.ts b/extension/src/background/messageListener/handlers/loadSettings.ts index 7ba8fcb67e..5f53b747fb 100644 --- a/extension/src/background/messageListener/handlers/loadSettings.ts +++ b/extension/src/background/messageListener/handlers/loadSettings.ts @@ -32,7 +32,8 @@ export const loadSettings = async ({ const isNonSSLEnabled = await getIsNonSSLEnabled({ localStore }); const isHideDustEnabled = await getIsHideDustEnabled({ localStore }); const isOpenSidebarByDefault = - (await localStore.getItem(IS_OPEN_SIDEBAR_BY_DEFAULT_ID)) === "true"; + ((await localStore.getItem(IS_OPEN_SIDEBAR_BY_DEFAULT_ID)) as boolean) ?? + false; const { hiddenAssets } = await getHiddenAssets({ localStore }); const overriddenBlockaidResponse = await getOverriddenBlockaidResponse({ localStore, diff --git a/extension/src/background/messageListener/handlers/saveSettings.ts b/extension/src/background/messageListener/handlers/saveSettings.ts index 7e7cdea931..f89d7a757f 100644 --- a/extension/src/background/messageListener/handlers/saveSettings.ts +++ b/extension/src/background/messageListener/handlers/saveSettings.ts @@ -60,6 +60,7 @@ export const saveSettings = async ({ isNonSSLEnabled: await getIsNonSSLEnabled({ localStore }), isHideDustEnabled: await getIsHideDustEnabled({ localStore }), isOpenSidebarByDefault: - (await localStore.getItem(IS_OPEN_SIDEBAR_BY_DEFAULT_ID)) === "true", + ((await localStore.getItem(IS_OPEN_SIDEBAR_BY_DEFAULT_ID)) as boolean) ?? + false, }; }; From 5e7d27fb3c22c8711afa1ce07d550eb35e72c261 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 1 Apr 2026 17:47:31 -0400 Subject: [PATCH 05/24] fix: update loadSaveSettings tests to mock booleans instead of strings browser.storage.local preserves types, so getItem returns boolean true/false, not string "true"/"false". Update test mocks to match. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../__tests__/loadSaveSettings.test.ts | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts b/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts index bfaaaf81d8..9f452cbc6c 100644 --- a/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts +++ b/extension/src/background/messageListener/__tests__/loadSaveSettings.test.ts @@ -33,11 +33,10 @@ jest.mock("../helpers/get-hidden-assets", () => ({ }; describe("loadSettings isOpenSidebarByDefault", () => { - it('returns true when localStorage value is the string "true"', async () => { + it("returns true when storage value is boolean true", async () => { const localStore = { getItem: jest.fn().mockImplementation((key: string) => { - if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) - return Promise.resolve("true"); + if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) return Promise.resolve(true); return Promise.resolve(null); }), setItem: jest.fn(), @@ -47,11 +46,11 @@ describe("loadSettings isOpenSidebarByDefault", () => { expect(result.isOpenSidebarByDefault).toBe(true); }); - it('returns false when localStorage value is the string "false"', async () => { + it("returns false when storage value is boolean false", async () => { const localStore = { getItem: jest.fn().mockImplementation((key: string) => { if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) - return Promise.resolve("false"); + return Promise.resolve(false); return Promise.resolve(null); }), setItem: jest.fn(), @@ -61,7 +60,7 @@ describe("loadSettings isOpenSidebarByDefault", () => { expect(result.isOpenSidebarByDefault).toBe(false); }); - it("returns false when localStorage value is null", async () => { + it("returns false when storage value is null", async () => { const localStore = { getItem: jest.fn().mockResolvedValue(null), setItem: jest.fn(), @@ -76,8 +75,7 @@ describe("saveSettings isOpenSidebarByDefault", () => { it("calls setPanelBehavior with the boolean from the request", async () => { const localStore = { getItem: jest.fn().mockImplementation((key: string) => { - if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) - return Promise.resolve("true"); + if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) return Promise.resolve(true); return Promise.resolve(null); }), setItem: jest.fn().mockResolvedValue(undefined), @@ -98,11 +96,11 @@ describe("saveSettings isOpenSidebarByDefault", () => { expect(result.isOpenSidebarByDefault).toBe(true); }); - it('returns boolean false (not the string "false") after saving', async () => { + it("returns boolean false after saving false", async () => { const localStore = { getItem: jest.fn().mockImplementation((key: string) => { if (key === IS_OPEN_SIDEBAR_BY_DEFAULT_ID) - return Promise.resolve("false"); + return Promise.resolve(false); return Promise.resolve(null); }), setItem: jest.fn().mockResolvedValue(undefined), @@ -117,7 +115,6 @@ describe("saveSettings isOpenSidebarByDefault", () => { const result = await saveSettings({ request, localStore }); expect(result.isOpenSidebarByDefault).toBe(false); - // Ensure it's actually a boolean, not the string "false" expect(typeof result.isOpenSidebarByDefault).toBe("boolean"); }); }); From 2e1b9c3279dcd4f17f246a8d6d9f928933a408c1 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Wed, 1 Apr 2026 17:48:31 -0400 Subject: [PATCH 06/24] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- extension/src/background/index.ts | 10 +++++++--- extension/src/popup/locales/pt/translation.json | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index 231e8705da..317ec31a89 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -80,9 +80,13 @@ export const initSidebarConnectionListener = () => { // Sidebar sends its window ID as first message port.onMessage.addListener((msg: unknown) => { - const { windowId } = msg as { windowId?: number }; - if (typeof windowId === "number") { - setSidebarWindowId(windowId); + if ( + typeof msg === "object" && + msg !== null && + "windowId" in msg && + typeof (msg as { windowId?: unknown }).windowId === "number" + ) { + setSidebarWindowId((msg as { windowId: number }).windowId); } }); diff --git a/extension/src/popup/locales/pt/translation.json b/extension/src/popup/locales/pt/translation.json index 4e62e43832..169361cac2 100644 --- a/extension/src/popup/locales/pt/translation.json +++ b/extension/src/popup/locales/pt/translation.json @@ -439,7 +439,7 @@ "Recovery Phrase": "Frase de Recuperação", "Refresh": "Atualizar", "Refresh metadata": "Atualizar metadados", - "Reject": "Reject", + "Reject": "Rejeitar", "Remember, Freighter will now display accounts related to the new backup phrase that was just created.": "Lembre-se, o Freighter agora exibirá contas relacionadas à nova frase de backup que acabou de ser criada.", "Remove": "Remover", "Remove asset": "Remover ativo", From fa12d30623dd42da8e36b1e06790e43be19c4a77 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Apr 2026 21:53:31 +0000 Subject: [PATCH 07/24] fix: use named onRemoved handler checking popup.id and self-removing after safeResolve Agent-Logs-Url: https://github.com/stellar/freighter/sessions/33844927-58ec-481b-9846-80c0e28a34b4 Co-authored-by: piyalbasu <6789586+piyalbasu@users.noreply.github.com> --- .../freighterApiMessageListener.ts | 68 ++++++++++++------- 1 file changed, 42 insertions(+), 26 deletions(-) diff --git a/extension/src/background/messageListener/freighterApiMessageListener.ts b/extension/src/background/messageListener/freighterApiMessageListener.ts index 2bed00ae7b..8505e04baf 100644 --- a/extension/src/background/messageListener/freighterApiMessageListener.ts +++ b/extension/src/background/messageListener/freighterApiMessageListener.ts @@ -271,11 +271,15 @@ export const freighterApiMessageListener = ( apiError: FreighterApiInternalError, }); } else if (popup !== null) { - browser.windows.onRemoved.addListener(() => - safeResolve({ - apiError: FreighterApiDeclinedError, - }), - ); + const onWindowRemoved = (removedWindowId: number) => { + if (removedWindowId === popup.id) { + browser.windows.onRemoved.removeListener(onWindowRemoved); + safeResolve({ + apiError: FreighterApiDeclinedError, + }); + } + }; + browser.windows.onRemoved.addListener(onWindowRemoved); } const response = (success: boolean) => { if (success) { @@ -437,13 +441,17 @@ export const freighterApiMessageListener = ( QUEUE_ITEM_TTL_MS, ); } else { - browser.windows.onRemoved.addListener(() => - safeResolve({ - // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface - apiError: FreighterApiDeclinedError, - error: FreighterApiDeclinedError.message, - }), - ); + const onWindowRemoved = (removedWindowId: number) => { + if (removedWindowId === popup.id) { + browser.windows.onRemoved.removeListener(onWindowRemoved); + safeResolve({ + // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface + apiError: FreighterApiDeclinedError, + error: FreighterApiDeclinedError.message, + }); + } + }; + browser.windows.onRemoved.addListener(onWindowRemoved); } const response = ( signedTransaction: string, @@ -526,13 +534,17 @@ export const freighterApiMessageListener = ( QUEUE_ITEM_TTL_MS, ); } else { - browser.windows.onRemoved.addListener(() => - safeResolve({ - // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface - apiError: FreighterApiDeclinedError, - error: FreighterApiDeclinedError.message, - }), - ); + const onWindowRemoved = (removedWindowId: number) => { + if (removedWindowId === popup.id) { + browser.windows.onRemoved.removeListener(onWindowRemoved); + safeResolve({ + // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface + apiError: FreighterApiDeclinedError, + error: FreighterApiDeclinedError.message, + }); + } + }; + browser.windows.onRemoved.addListener(onWindowRemoved); } const response = ( @@ -629,13 +641,17 @@ export const freighterApiMessageListener = ( QUEUE_ITEM_TTL_MS, ); } else { - browser.windows.onRemoved.addListener(() => - safeResolve({ - // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface - apiError: FreighterApiDeclinedError, - error: FreighterApiDeclinedError.message, - }), - ); + const onWindowRemoved = (removedWindowId: number) => { + if (removedWindowId === popup.id) { + browser.windows.onRemoved.removeListener(onWindowRemoved); + safeResolve({ + // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface + apiError: FreighterApiDeclinedError, + error: FreighterApiDeclinedError.message, + }); + } + }; + browser.windows.onRemoved.addListener(onWindowRemoved); } const response = ( signedAuthEntry: SignAuthEntryResponse, From ab9b3bee2a59624caa6d688fd8d776a082f7a542 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Thu, 2 Apr 2026 13:09:49 -0400 Subject: [PATCH 08/24] redesign ConfirmSidebarRequest to match updated Figma spec Left-align layout, use smaller square icon, pill-shaped buttons, updated button variants and text per new design. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/popup/locales/en/translation.json | 1 + .../src/popup/locales/pt/translation.json | 1 + .../views/ConfirmSidebarRequest/index.tsx | 30 +++++++------ .../views/ConfirmSidebarRequest/styles.scss | 44 ++++++++++++------- 4 files changed, 46 insertions(+), 30 deletions(-) diff --git a/extension/src/popup/locales/en/translation.json b/extension/src/popup/locales/en/translation.json index a187ee0095..973dc26c0a 100644 --- a/extension/src/popup/locales/en/translation.json +++ b/extension/src/popup/locales/en/translation.json @@ -128,6 +128,7 @@ "CONNECTION ERROR": "CONNECTION ERROR", "Connection Request": "Connection Request", "Continue": "Continue", + "Continue to review": "Continue to review", "Contract Address": "Contract Address", "Contract creation": "Contract creation", "Contract Function": "Contract Function", diff --git a/extension/src/popup/locales/pt/translation.json b/extension/src/popup/locales/pt/translation.json index 169361cac2..717f9da2bd 100644 --- a/extension/src/popup/locales/pt/translation.json +++ b/extension/src/popup/locales/pt/translation.json @@ -128,6 +128,7 @@ "CONNECTION ERROR": "ERRO DE CONEXÃO", "Connection Request": "Solicitação de Conexão", "Continue": "Continuar", + "Continue to review": "Continue to review", "Contract Address": "Endereço do Contrato", "Contract creation": "Criação de contrato", "Contract Function": "Função do Contrato", diff --git a/extension/src/popup/views/ConfirmSidebarRequest/index.tsx b/extension/src/popup/views/ConfirmSidebarRequest/index.tsx index 85f931e012..2193aa7ba9 100644 --- a/extension/src/popup/views/ConfirmSidebarRequest/index.tsx +++ b/extension/src/popup/views/ConfirmSidebarRequest/index.tsx @@ -27,23 +27,25 @@ export const ConfirmSidebarRequest = () => { }; return ( - +
-
- +
+
+ +
+

+ {t("New Signing Request")} +

+
+ {t( + "A new signing request arrived while you were reviewing another. Please review it carefully before approving.", + )} +
-

- {t("New Signing Request")} -

-

- {t( - "A new signing request arrived while you were reviewing another. Please review it carefully before approving.", - )} -

diff --git a/extension/src/popup/views/ConfirmSidebarRequest/styles.scss b/extension/src/popup/views/ConfirmSidebarRequest/styles.scss index 7b3791b708..3a8b50a06e 100644 --- a/extension/src/popup/views/ConfirmSidebarRequest/styles.scss +++ b/extension/src/popup/views/ConfirmSidebarRequest/styles.scss @@ -3,47 +3,59 @@ .ConfirmSidebarRequest { display: flex; flex-direction: column; - align-items: center; - text-align: center; - padding: pxToRem(48px) pxToRem(24px); - gap: pxToRem(16px); + gap: pxToRem(32px); + padding: pxToRem(24px); + height: 100%; + + &__header { + display: flex; + flex-direction: column; + gap: pxToRem(16px); + } &__icon { display: flex; align-items: center; justify-content: center; - width: pxToRem(56px); - height: pxToRem(56px); - border-radius: 50%; + width: pxToRem(32px); + height: pxToRem(32px); + border-radius: pxToRem(8px); background-color: var(--sds-clr-amber-03); - color: var(--sds-clr-amber-11); + border: pxToRem(1px) solid var(--sds-clr-amber-07); svg { - width: pxToRem(28px); - height: pxToRem(28px); + width: pxToRem(20px); + height: pxToRem(20px); + color: var(--sds-clr-amber-11); } } &__title { - font-size: pxToRem(20px); - font-weight: 600; + font-size: pxToRem(18px); + font-weight: 500; + line-height: pxToRem(26px); color: var(--sds-clr-gray-12); margin: 0; } &__body { + font-family: "Inter", sans-serif; font-size: pxToRem(14px); + font-weight: 400; + font-style: normal; + line-height: pxToRem(20px); color: var(--sds-clr-gray-11); - line-height: 1.5; margin: 0; - max-width: pxToRem(320px); } &__buttons { display: flex; flex-direction: column; - gap: pxToRem(8px); + gap: pxToRem(12px); width: 100%; - margin-top: pxToRem(16px); + + button { + border-radius: pxToRem(100px); + } } } From 780d8521ef2f80d0fc030bd0cd86cba54d31ab29 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Thu, 2 Apr 2026 13:38:49 -0400 Subject: [PATCH 09/24] Update extension/src/popup/locales/pt/translation.json Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- extension/src/popup/locales/pt/translation.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/src/popup/locales/pt/translation.json b/extension/src/popup/locales/pt/translation.json index 717f9da2bd..69b3bf01f9 100644 --- a/extension/src/popup/locales/pt/translation.json +++ b/extension/src/popup/locales/pt/translation.json @@ -128,7 +128,7 @@ "CONNECTION ERROR": "ERRO DE CONEXÃO", "Connection Request": "Solicitação de Conexão", "Continue": "Continuar", - "Continue to review": "Continue to review", + "Continue to review": "Continuar a revisar", "Contract Address": "Endereço do Contrato", "Contract creation": "Criação de contrato", "Contract Function": "Função do Contrato", From 69d1a8f7a711a7187d3998633d49f1f5d36716fd Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Thu, 2 Apr 2026 13:39:00 -0400 Subject: [PATCH 10/24] Update extension/src/popup/views/ConfirmSidebarRequest/index.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../views/ConfirmSidebarRequest/index.tsx | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/extension/src/popup/views/ConfirmSidebarRequest/index.tsx b/extension/src/popup/views/ConfirmSidebarRequest/index.tsx index 2193aa7ba9..b7c7fb9d5f 100644 --- a/extension/src/popup/views/ConfirmSidebarRequest/index.tsx +++ b/extension/src/popup/views/ConfirmSidebarRequest/index.tsx @@ -16,12 +16,27 @@ export const ConfirmSidebarRequest = () => { const params = new URLSearchParams(location.search); const next = params.get("next") || ""; - const handleReview = () => { - if (next) { - navigate(next); + // Only allow safe, in-extension routes for "next"; fall back to account route. + const isValidNextRoute = (value: string) => { + if (!value) { + return false; + } + // Require a single leading "/" (internal path), disallow "//" and any URI scheme. + if (!value.startsWith("/") || value.startsWith("//")) { + return false; + } + // Disallow strings that look like they start with a URI scheme (e.g., "http:", "javascript:"). + if (/^[a-zA-Z][a-zA-Z0-9+\-.]*:/.test(value)) { + return false; } + return true; }; + const safeNext = isValidNextRoute(next) ? next : ROUTES.account; + + const handleReview = () => { + navigate(safeNext); + }; const handleReject = () => { navigate(ROUTES.account); }; From 26200d01927b38224c323f509d754e2b224dbc91 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 18:32:36 +0000 Subject: [PATCH 11/24] fix: track sidebar UUIDs separately and guard port/windowId clear on disconnect Agent-Logs-Url: https://github.com/stellar/freighter/sessions/7e15d7bf-0539-4ceb-80a3-eb9ae1ce7a9b Co-authored-by: piyalbasu <6789586+piyalbasu@users.noreply.github.com> --- .../src/background/helpers/queueCleanup.ts | 5 +++ extension/src/background/index.ts | 20 ++++++++---- .../messageListener/__tests__/sidebar.test.ts | 32 +++++++++++++++++++ .../freighterApiMessageListener.ts | 16 ++++++++-- 4 files changed, 65 insertions(+), 8 deletions(-) diff --git a/extension/src/background/helpers/queueCleanup.ts b/extension/src/background/helpers/queueCleanup.ts index b9b56618a1..8c7fdac6b6 100644 --- a/extension/src/background/helpers/queueCleanup.ts +++ b/extension/src/background/helpers/queueCleanup.ts @@ -15,6 +15,11 @@ export const CLEANUP_INTERVAL_MS = 60 * 1000; // In MV3, this resets when the service worker restarts, which also resets the queues. export const activeQueueUuids: Set = new Set(); +// Set of UUIDs for signing requests that are being handled by the sidebar +// (as opposed to a standalone popup window). Only these should be rejected +// when the sidebar disconnects. +export const sidebarQueueUuids: Set = new Set(); + /** * Removes expired items from a queue based on their createdAt timestamp. * Items older than the TTL are removed. Items without createdAt are also removed diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index 317ec31a89..a72653ba62 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -20,10 +20,11 @@ import { import { setSidebarPort, clearSidebarPort, + getSidebarPort, } from "./messageListener/freighterApiMessageListener"; import { freighterApiMessageListener } from "./messageListener/freighterApiMessageListener"; import { SIDEBAR_PORT_NAME } from "popup/components/SidebarSigningListener"; -import { activeQueueUuids } from "./helpers/queueCleanup"; +import { sidebarQueueUuids } from "./helpers/queueCleanup"; import { SESSION_ALARM_NAME, SessionTimer, @@ -93,11 +94,18 @@ export const initSidebarConnectionListener = () => { // When sidebar closes (for any reason), clear the window ID // and reject any pending signing requests port.onDisconnect.addListener(() => { - clearSidebarPort(); - clearSidebarWindowId(); + // Only clear the stored port/windowId if this is still the active sidebar port. + // An older port disconnecting after a newer sidebar connected must not + // evict the newer sidebar's state. + if (getSidebarPort() === port) { + clearSidebarPort(); + clearSidebarWindowId(); + } - // Reject all active signing requests that were open in the sidebar - for (const uuid of activeQueueUuids) { + // Reject only the signing requests that were routed to this sidebar. + // Requests handled by standalone popup windows have their own + // onWindowRemoved listeners and must not be cancelled here. + for (const uuid of sidebarQueueUuids) { const responseIndex = responseQueue.findIndex( (item) => item.uuid === uuid, ); @@ -123,7 +131,7 @@ export const initSidebarConnectionListener = () => { const tokenIndex = tokenQueue.findIndex((item) => item.uuid === uuid); if (tokenIndex !== -1) tokenQueue.splice(tokenIndex, 1); } - activeQueueUuids.clear(); + sidebarQueueUuids.clear(); }); }); }; diff --git a/extension/src/background/messageListener/__tests__/sidebar.test.ts b/extension/src/background/messageListener/__tests__/sidebar.test.ts index c27db5b795..19420b85da 100644 --- a/extension/src/background/messageListener/__tests__/sidebar.test.ts +++ b/extension/src/background/messageListener/__tests__/sidebar.test.ts @@ -3,6 +3,7 @@ import { popupMessageListener } from "background/messageListener/popupMessageLis import { setSidebarPort, clearSidebarPort, + getSidebarPort, } from "background/messageListener/freighterApiMessageListener"; // Mock chrome.sidePanel API @@ -135,5 +136,36 @@ describe("sidebar message handlers", () => { it("clearSidebarPort is safe to call when no port is set", () => { expect(() => clearSidebarPort()).not.toThrow(); }); + + it("getSidebarPort returns null after clearSidebarPort", () => { + const mockPort = { postMessage: jest.fn(), disconnect: jest.fn() } as any; + setSidebarPort(mockPort); + clearSidebarPort(); + expect(getSidebarPort()).toBeNull(); + }); + + it("getSidebarPort returns the currently stored port", () => { + const mockPort = { postMessage: jest.fn(), disconnect: jest.fn() } as any; + setSidebarPort(mockPort); + expect(getSidebarPort()).toBe(mockPort); + }); + + it("an older port disconnecting does not evict a newer sidebar port", () => { + const portA = { postMessage: jest.fn(), disconnect: jest.fn() } as any; + const portB = { postMessage: jest.fn(), disconnect: jest.fn() } as any; + + // Connect portA then portB (simulates a second sidebar window opening) + setSidebarPort(portA); + setSidebarPort(portB); + + // Simulate portA's disconnect handler: only clear if portA is still the active port + const disconnectingPort = portA; + if (getSidebarPort() === disconnectingPort) { + clearSidebarPort(); + } + + // portA disconnected but portB is still active – must not be cleared + expect(getSidebarPort()).toBe(portB); + }); }); }); diff --git a/extension/src/background/messageListener/freighterApiMessageListener.ts b/extension/src/background/messageListener/freighterApiMessageListener.ts index 8505e04baf..ad4d7e26a2 100644 --- a/extension/src/background/messageListener/freighterApiMessageListener.ts +++ b/extension/src/background/messageListener/freighterApiMessageListener.ts @@ -69,7 +69,7 @@ import { tokenQueue, transactionQueue, } from "./popupMessageListener"; -import { QUEUE_ITEM_TTL_MS } from "background/helpers/queueCleanup"; +import { QUEUE_ITEM_TTL_MS, sidebarQueueUuids } from "background/helpers/queueCleanup"; // Long-lived port to the sidebar, set by initSidebarConnectionListener let sidebarPort: browser.Runtime.Port | null = null; @@ -80,6 +80,7 @@ export const setSidebarPort = (port: browser.Runtime.Port) => { export const clearSidebarPort = () => { sidebarPort = null; }; +export const getSidebarPort = () => sidebarPort; const openSigningWindow = async (hashRoute: string, width?: number) => { const sidebarWindowId = getSidebarWindowId(); @@ -157,10 +158,12 @@ export const freighterApiMessageListener = ( const safeResolve = (value: any) => { if (resolved) return; resolved = true; + sidebarQueueUuids.delete(uuid); resolve(value); }; if (popup === null) { + sidebarQueueUuids.add(uuid); setTimeout( () => safeResolve({ @@ -263,6 +266,7 @@ export const freighterApiMessageListener = ( const safeResolve = (value: any) => { if (resolved) return; resolved = true; + sidebarQueueUuids.delete(uuid); resolve(value); }; @@ -270,7 +274,9 @@ export const freighterApiMessageListener = ( safeResolve({ apiError: FreighterApiInternalError, }); - } else if (popup !== null) { + } else if (popup === null) { + sidebarQueueUuids.add(uuid); + } else { const onWindowRemoved = (removedWindowId: number) => { if (removedWindowId === popup.id) { browser.windows.onRemoved.removeListener(onWindowRemoved); @@ -422,6 +428,7 @@ export const freighterApiMessageListener = ( const safeResolve = (value: any) => { if (resolved) return; resolved = true; + sidebarQueueUuids.delete(uuid); resolve(value); }; @@ -432,6 +439,7 @@ export const freighterApiMessageListener = ( error: FreighterApiInternalError.message, }); } else if (popup === null) { + sidebarQueueUuids.add(uuid); setTimeout( () => safeResolve({ @@ -515,6 +523,7 @@ export const freighterApiMessageListener = ( const safeResolve = (value: any) => { if (resolved) return; resolved = true; + sidebarQueueUuids.delete(uuid); resolve(value); }; @@ -525,6 +534,7 @@ export const freighterApiMessageListener = ( error: FreighterApiInternalError.message, }); } else if (popup === null) { + sidebarQueueUuids.add(uuid); setTimeout( () => safeResolve({ @@ -622,6 +632,7 @@ export const freighterApiMessageListener = ( const safeResolve = (value: any) => { if (resolved) return; resolved = true; + sidebarQueueUuids.delete(uuid); resolve(value); }; @@ -632,6 +643,7 @@ export const freighterApiMessageListener = ( error: FreighterApiInternalError.message, }); } else if (popup === null) { + sidebarQueueUuids.add(uuid); setTimeout( () => safeResolve({ From 58ca5ff68f461298072335c0db3dd1af49b2fd6a Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Thu, 2 Apr 2026 15:37:53 -0400 Subject: [PATCH 12/24] fix: address code review issues for sidebar signing flow - Add sidebarQueueUuids tracking to setAllowedStatus handler so sidebar disconnect properly rejects the pending promise instead of hanging - ConfirmSidebarRequest "Reject" now extracts UUID from the next route and calls rejectAccess so the dapp promise resolves immediately - Ensure window.close() runs in the catch block of openSidebar() so the popup is closed even when the side panel API throws Co-Authored-By: Claude Opus 4.6 (1M context) --- .../freighterApiMessageListener.ts | 27 ++++++++++++------- extension/src/popup/helpers/navigate.ts | 1 + .../views/ConfirmSidebarRequest/index.tsx | 18 ++++++++++++- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/extension/src/background/messageListener/freighterApiMessageListener.ts b/extension/src/background/messageListener/freighterApiMessageListener.ts index ad4d7e26a2..9d299a7751 100644 --- a/extension/src/background/messageListener/freighterApiMessageListener.ts +++ b/extension/src/background/messageListener/freighterApiMessageListener.ts @@ -69,7 +69,10 @@ import { tokenQueue, transactionQueue, } from "./popupMessageListener"; -import { QUEUE_ITEM_TTL_MS, sidebarQueueUuids } from "background/helpers/queueCleanup"; +import { + QUEUE_ITEM_TTL_MS, + sidebarQueueUuids, +} from "background/helpers/queueCleanup"; // Long-lived port to the sidebar, set by initSidebarConnectionListener let sidebarPort: browser.Runtime.Port | null = null; @@ -781,24 +784,28 @@ export const freighterApiMessageListener = ( const uuid = crypto.randomUUID(); const encodeOrigin = encodeObject({ tab, url: tabUrl, uuid }); - await openSigningWindow(`/grant-access?${encodeOrigin}`, 400); + const popup = await openSigningWindow(`/grant-access?${encodeOrigin}`, 400); return new Promise((resolve) => { let resolved = false; const safeResolve = (value: any) => { if (resolved) return; resolved = true; + sidebarQueueUuids.delete(uuid); resolve(value); }; - setTimeout( - () => - safeResolve({ - apiError: FreighterApiDeclinedError, - error: FreighterApiDeclinedError.message, - }), - QUEUE_ITEM_TTL_MS, - ); + if (popup === null) { + sidebarQueueUuids.add(uuid); + setTimeout( + () => + safeResolve({ + apiError: FreighterApiDeclinedError, + error: FreighterApiDeclinedError.message, + }), + QUEUE_ITEM_TTL_MS, + ); + } const response = async (url?: string) => { // queue it up, we'll let user confirm the url looks okay and then we'll say it's okay if (url === tabUrl) { diff --git a/extension/src/popup/helpers/navigate.ts b/extension/src/popup/helpers/navigate.ts index 4c185b79ca..7a95dc1644 100644 --- a/extension/src/popup/helpers/navigate.ts +++ b/extension/src/popup/helpers/navigate.ts @@ -32,5 +32,6 @@ export const openSidebar = async () => { window.close(); } catch (e) { console.error("Failed to open sidebar:", e); + window.close(); } }; diff --git a/extension/src/popup/views/ConfirmSidebarRequest/index.tsx b/extension/src/popup/views/ConfirmSidebarRequest/index.tsx index b7c7fb9d5f..f62618d15a 100644 --- a/extension/src/popup/views/ConfirmSidebarRequest/index.tsx +++ b/extension/src/popup/views/ConfirmSidebarRequest/index.tsx @@ -5,6 +5,8 @@ import { Button, Icon } from "@stellar/design-system"; import { ROUTES } from "popup/constants/routes"; import { View } from "popup/basics/layout/View"; +import { rejectAccess as internalRejectAccess } from "@shared/api/internal"; +import { parsedSearchParam } from "helpers/urls"; import "./styles.scss"; @@ -37,7 +39,21 @@ export const ConfirmSidebarRequest = () => { const handleReview = () => { navigate(safeNext); }; - const handleReject = () => { + const handleReject = async () => { + // Extract the UUID from the incoming request's encoded route so we can + // explicitly reject it, preventing the dapp's promise from hanging. + try { + const nextRoute = isValidNextRoute(next) ? next : ""; + const queryString = nextRoute.split("?")[1] || ""; + if (queryString) { + const parsed = parsedSearchParam(queryString); + if (parsed.uuid) { + await internalRejectAccess({ uuid: parsed.uuid }); + } + } + } catch { + // Best-effort — navigate home regardless + } navigate(ROUTES.account); }; From 3aeefb57cfbe66eebd694285cc8936de4a9bfd86 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Thu, 2 Apr 2026 16:46:04 -0400 Subject: [PATCH 13/24] fix: defer sidebar disconnect cleanup to avoid rejecting during page reload chrome.sidePanel.open() can reload the sidebar page, causing a brief port disconnect/reconnect. The disconnect handler was immediately rejecting all in-flight signing requests before the new port could connect. Use a cancellable deferred cleanup: schedule rejection on disconnect, but cancel it if a new sidebar port connects before it fires. Co-Authored-By: Claude Opus 4.6 (1M context) --- extension/src/background/index.ts | 81 ++++++++++++++++++------------- 1 file changed, 47 insertions(+), 34 deletions(-) diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index a72653ba62..4447718b56 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -63,6 +63,11 @@ export const initContentScriptMessageListener = () => { }; export const initSidebarConnectionListener = () => { + // Pending cleanup scheduled when a sidebar port disconnects. + // Cancelled if a new port connects before it fires (sidebar reloaded + // rather than closed, e.g. from chrome.sidePanel.open()). + let pendingCleanup: ReturnType | null = null; + browser.runtime.onConnect.addListener((port) => { if (port.name !== SIDEBAR_PORT_NAME) return; @@ -76,6 +81,13 @@ export const initSidebarConnectionListener = () => { return; } + // A new sidebar connected — cancel any pending cleanup from a + // previous port disconnect so in-flight requests stay alive. + if (pendingCleanup !== null) { + clearTimeout(pendingCleanup); + pendingCleanup = null; + } + // Store port reference so openSigningWindow can send messages directly setSidebarPort(port); @@ -91,47 +103,48 @@ export const initSidebarConnectionListener = () => { } }); - // When sidebar closes (for any reason), clear the window ID - // and reject any pending signing requests + // When sidebar closes, clear port state and schedule cleanup. + // The cleanup is deferred because chrome.sidePanel.open() can + // reload the sidebar page, causing a brief disconnect/reconnect. port.onDisconnect.addListener(() => { - // Only clear the stored port/windowId if this is still the active sidebar port. - // An older port disconnecting after a newer sidebar connected must not - // evict the newer sidebar's state. if (getSidebarPort() === port) { clearSidebarPort(); clearSidebarWindowId(); } - // Reject only the signing requests that were routed to this sidebar. - // Requests handled by standalone popup windows have their own - // onWindowRemoved listeners and must not be cancelled here. - for (const uuid of sidebarQueueUuids) { - const responseIndex = responseQueue.findIndex( - (item) => item.uuid === uuid, - ); - if (responseIndex !== -1) { - const responseQueueItem = responseQueue.splice(responseIndex, 1)[0]; - responseQueueItem.response(undefined); + pendingCleanup = setTimeout(() => { + pendingCleanup = null; + + // Reject only the signing requests that were routed to the sidebar. + // Requests handled by standalone popup windows have their own + // onWindowRemoved listeners and must not be cancelled here. + for (const uuid of sidebarQueueUuids) { + const responseIndex = responseQueue.findIndex( + (item) => item.uuid === uuid, + ); + if (responseIndex !== -1) { + const responseQueueItem = responseQueue.splice(responseIndex, 1)[0]; + responseQueueItem.response(undefined); + } + + const txIndex = transactionQueue.findIndex( + (item) => item.uuid === uuid, + ); + if (txIndex !== -1) transactionQueue.splice(txIndex, 1); + + const blobIndex = blobQueue.findIndex((item) => item.uuid === uuid); + if (blobIndex !== -1) blobQueue.splice(blobIndex, 1); + + const authIndex = authEntryQueue.findIndex( + (item) => item.uuid === uuid, + ); + if (authIndex !== -1) authEntryQueue.splice(authIndex, 1); + + const tokenIndex = tokenQueue.findIndex((item) => item.uuid === uuid); + if (tokenIndex !== -1) tokenQueue.splice(tokenIndex, 1); } - - // Clean up the data queues - const txIndex = transactionQueue.findIndex( - (item) => item.uuid === uuid, - ); - if (txIndex !== -1) transactionQueue.splice(txIndex, 1); - - const blobIndex = blobQueue.findIndex((item) => item.uuid === uuid); - if (blobIndex !== -1) blobQueue.splice(blobIndex, 1); - - const authIndex = authEntryQueue.findIndex( - (item) => item.uuid === uuid, - ); - if (authIndex !== -1) authEntryQueue.splice(authIndex, 1); - - const tokenIndex = tokenQueue.findIndex((item) => item.uuid === uuid); - if (tokenIndex !== -1) tokenQueue.splice(tokenIndex, 1); - } - sidebarQueueUuids.clear(); + sidebarQueueUuids.clear(); + }, 500); }); }); }; From bfea660c5fc6eeb7b734294df26e474eccfec17d Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Thu, 2 Apr 2026 16:47:49 -0400 Subject: [PATCH 14/24] Update extension/src/popup/constants/metricsNames.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- extension/src/popup/constants/metricsNames.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/src/popup/constants/metricsNames.ts b/extension/src/popup/constants/metricsNames.ts index 05768159a4..4132b5fce8 100644 --- a/extension/src/popup/constants/metricsNames.ts +++ b/extension/src/popup/constants/metricsNames.ts @@ -182,5 +182,5 @@ export const METRIC_NAMES = { coinbaseOnrampOpened: "coinbase onramp: opened", wallets: "loaded screen: wallets", - confirmSidebarRequest: "loaded screen: confirmSidebarRequest", + confirmSidebarRequest: "loaded screen: confirm sidebar request", }; From c6e0f8895ae6e95cd2113544405cceeec1656359 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 20:56:15 +0000 Subject: [PATCH 15/24] fix: add REJECT_SIGNING_REQUEST handler to clean all queues on ConfirmSidebarRequest reject Agent-Logs-Url: https://github.com/stellar/freighter/sessions/da099665-1448-4e91-b2eb-4daa952a09bb Co-authored-by: piyalbasu <6789586+piyalbasu@users.noreply.github.com> --- @shared/api/internal.ts | 16 ++++ @shared/api/types/message-request.ts | 8 +- @shared/constants/services.ts | 1 + .../handlers/rejectSigningRequest.ts | 77 +++++++++++++++++++ .../messageListener/popupMessageListener.ts | 11 +++ .../views/ConfirmSidebarRequest/index.tsx | 10 ++- 6 files changed, 118 insertions(+), 5 deletions(-) create mode 100644 extension/src/background/messageListener/handlers/rejectSigningRequest.ts diff --git a/@shared/api/internal.ts b/@shared/api/internal.ts index 85fd2b576d..30e53805ff 100644 --- a/@shared/api/internal.ts +++ b/@shared/api/internal.ts @@ -2407,3 +2407,19 @@ export const markQueueActive = async ({ console.error(e); } }; + +export const rejectSigningRequest = async ({ + uuid, +}: { + uuid: string; +}): Promise => { + try { + await sendMessageToBackground({ + activePublicKey: null, + uuid, + type: SERVICE_TYPES.REJECT_SIGNING_REQUEST, + }); + } catch (e) { + console.error(e); + } +}; diff --git a/@shared/api/types/message-request.ts b/@shared/api/types/message-request.ts index 932a65b1c7..a1bd20f86d 100644 --- a/@shared/api/types/message-request.ts +++ b/@shared/api/types/message-request.ts @@ -450,6 +450,11 @@ export interface OpenSidebarMessage extends BaseMessage { windowId: number; } +export interface RejectSigningRequestMessage extends BaseMessage { + type: SERVICE_TYPES.REJECT_SIGNING_REQUEST; + uuid: string; +} + export type ServiceMessageRequest = | FundAccountMessage | CreateAccountMessage @@ -515,4 +520,5 @@ export type ServiceMessageRequest = | ChangeCollectibleVisibilityMessage | GetHiddenCollectiblesMessage | MarkQueueActiveMessage - | OpenSidebarMessage; + | OpenSidebarMessage + | RejectSigningRequestMessage; diff --git a/@shared/constants/services.ts b/@shared/constants/services.ts index 0f5b3bf751..93f3032ab9 100644 --- a/@shared/constants/services.ts +++ b/@shared/constants/services.ts @@ -64,6 +64,7 @@ export enum SERVICE_TYPES { GET_HIDDEN_COLLECTIBLES = "GET_HIDDEN_COLLECTIBLES", MARK_QUEUE_ACTIVE = "MARK_QUEUE_ACTIVE", OPEN_SIDEBAR = "OPEN_SIDEBAR", + REJECT_SIGNING_REQUEST = "REJECT_SIGNING_REQUEST", } export const SIDEBAR_NAVIGATE = "SIDEBAR_NAVIGATE"; diff --git a/extension/src/background/messageListener/handlers/rejectSigningRequest.ts b/extension/src/background/messageListener/handlers/rejectSigningRequest.ts new file mode 100644 index 0000000000..3ccfe02970 --- /dev/null +++ b/extension/src/background/messageListener/handlers/rejectSigningRequest.ts @@ -0,0 +1,77 @@ +import { + RejectSigningRequestMessage, + ResponseQueue, + TransactionQueue, + BlobQueue, + EntryQueue, + TokenQueue, + RequestAccessResponse, + SignTransactionResponse, + SignBlobResponse, + SignAuthEntryResponse, + AddTokenResponse, + SetAllowedStatusResponse, +} from "@shared/api/types/message-request"; +import { captureException } from "@sentry/browser"; + +type AnySigningResponse = + | RequestAccessResponse + | SignTransactionResponse + | SignBlobResponse + | SignAuthEntryResponse + | AddTokenResponse + | SetAllowedStatusResponse + | undefined; + +/** + * Rejects a pending signing request by UUID, removing it from every queue + * (responseQueue, transactionQueue, blobQueue, authEntryQueue, tokenQueue). + * This ensures the dapp's promise resolves immediately rather than waiting + * for the TTL timeout. + */ +export const rejectSigningRequest = ({ + request, + responseQueue, + transactionQueue, + blobQueue, + authEntryQueue, + tokenQueue, +}: { + request: RejectSigningRequestMessage; + responseQueue: ResponseQueue; + transactionQueue: TransactionQueue; + blobQueue: BlobQueue; + authEntryQueue: EntryQueue; + tokenQueue: TokenQueue; +}) => { + const { uuid } = request; + + if (!uuid) { + captureException("rejectSigningRequest: missing uuid in request"); + return; + } + + // Resolve (reject) the dapp's pending promise + const responseIndex = responseQueue.findIndex((item) => item.uuid === uuid); + if (responseIndex !== -1) { + const responseQueueItem = responseQueue.splice(responseIndex, 1)[0]; + responseQueueItem.response(undefined); + } else { + captureException( + `rejectSigningRequest: no matching response found for uuid ${uuid}`, + ); + } + + // Clean up all data queues so stale entries don't accumulate + const txIndex = transactionQueue.findIndex((item) => item.uuid === uuid); + if (txIndex !== -1) transactionQueue.splice(txIndex, 1); + + const blobIndex = blobQueue.findIndex((item) => item.uuid === uuid); + if (blobIndex !== -1) blobQueue.splice(blobIndex, 1); + + const authIndex = authEntryQueue.findIndex((item) => item.uuid === uuid); + if (authIndex !== -1) authEntryQueue.splice(authIndex, 1); + + const tokenIndex = tokenQueue.findIndex((item) => item.uuid === uuid); + if (tokenIndex !== -1) tokenQueue.splice(tokenIndex, 1); +}; diff --git a/extension/src/background/messageListener/popupMessageListener.ts b/extension/src/background/messageListener/popupMessageListener.ts index 2d83c9c52a..bd7fb245ca 100644 --- a/extension/src/background/messageListener/popupMessageListener.ts +++ b/extension/src/background/messageListener/popupMessageListener.ts @@ -55,6 +55,7 @@ import { signTransaction } from "./handlers/signTransaction"; import { signBlob } from "./handlers/signBlob"; import { signAuthEntry } from "./handlers/signAuthEntry"; import { rejectTransaction } from "./handlers/rejectTransaction"; +import { rejectSigningRequest } from "./handlers/rejectSigningRequest"; import { signFreighterTransaction } from "./handlers/signFreighterTransaction"; import { addRecentAddress } from "./handlers/addRecentAddress"; import { loadRecentAddresses } from "./handlers/loadRecentAddresses"; @@ -350,6 +351,16 @@ export const popupMessageListener = ( transactionQueue, }); } + case SERVICE_TYPES.REJECT_SIGNING_REQUEST: { + return rejectSigningRequest({ + request, + responseQueue, + transactionQueue, + blobQueue, + authEntryQueue, + tokenQueue, + }); + } case SERVICE_TYPES.SIGN_FREIGHTER_TRANSACTION: { return signFreighterTransaction({ request, diff --git a/extension/src/popup/views/ConfirmSidebarRequest/index.tsx b/extension/src/popup/views/ConfirmSidebarRequest/index.tsx index f62618d15a..e8f0d9f73e 100644 --- a/extension/src/popup/views/ConfirmSidebarRequest/index.tsx +++ b/extension/src/popup/views/ConfirmSidebarRequest/index.tsx @@ -5,7 +5,7 @@ import { Button, Icon } from "@stellar/design-system"; import { ROUTES } from "popup/constants/routes"; import { View } from "popup/basics/layout/View"; -import { rejectAccess as internalRejectAccess } from "@shared/api/internal"; +import { rejectSigningRequest } from "@shared/api/internal"; import { parsedSearchParam } from "helpers/urls"; import "./styles.scss"; @@ -40,15 +40,17 @@ export const ConfirmSidebarRequest = () => { navigate(safeNext); }; const handleReject = async () => { - // Extract the UUID from the incoming request's encoded route so we can - // explicitly reject it, preventing the dapp's promise from hanging. + // Extract the UUID from the incoming request's encoded route and reject it + // via a dedicated handler that cleans up ALL queues (responseQueue, + // transactionQueue, blobQueue, authEntryQueue, tokenQueue) so the dapp's + // promise resolves immediately instead of hanging until the TTL fires. try { const nextRoute = isValidNextRoute(next) ? next : ""; const queryString = nextRoute.split("?")[1] || ""; if (queryString) { const parsed = parsedSearchParam(queryString); if (parsed.uuid) { - await internalRejectAccess({ uuid: parsed.uuid }); + await rejectSigningRequest({ uuid: parsed.uuid }); } } } catch { From 1ba4ef0afeaeb2374838b7bac6b29618cc558167 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Thu, 2 Apr 2026 16:58:48 -0400 Subject: [PATCH 16/24] refactor: unify sidebarQueueUuids lifecycle with activeQueueUuids Move sidebarQueueUuids population from freighterApiMessageListener (eager, on openSigningWindow return) into the MARK_QUEUE_ACTIVE handler in popupMessageListener (on signing view mount/unmount), matching how activeQueueUuids is managed. Extract duplicated onWindowRemoved logic into a shared rejectOnWindowClose helper. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../freighterApiMessageListener.ts | 87 ++++++------------- .../messageListener/popupMessageListener.ts | 6 ++ 2 files changed, 33 insertions(+), 60 deletions(-) diff --git a/extension/src/background/messageListener/freighterApiMessageListener.ts b/extension/src/background/messageListener/freighterApiMessageListener.ts index 9d299a7751..e1e8f3f188 100644 --- a/extension/src/background/messageListener/freighterApiMessageListener.ts +++ b/extension/src/background/messageListener/freighterApiMessageListener.ts @@ -69,10 +69,7 @@ import { tokenQueue, transactionQueue, } from "./popupMessageListener"; -import { - QUEUE_ITEM_TTL_MS, - sidebarQueueUuids, -} from "background/helpers/queueCleanup"; +import { QUEUE_ITEM_TTL_MS } from "background/helpers/queueCleanup"; // Long-lived port to the sidebar, set by initSidebarConnectionListener let sidebarPort: browser.Runtime.Port | null = null; @@ -112,6 +109,25 @@ const openSigningWindow = async (hashRoute: string, width?: number) => { ...(width !== undefined ? { width } : {}), }); }; + +/** Reject the dapp's pending request when the popup window is closed. */ +const rejectOnWindowClose = ( + windowId: number, + safeResolve: (value: any) => void, + rejectValue: Record = { + apiError: FreighterApiDeclinedError, + error: FreighterApiDeclinedError.message, + }, +) => { + const onWindowRemoved = (removedWindowId: number) => { + if (removedWindowId === windowId) { + browser.windows.onRemoved.removeListener(onWindowRemoved); + safeResolve(rejectValue); + } + }; + browser.windows.onRemoved.addListener(onWindowRemoved); +}; + import { DataStorageAccess } from "background/helpers/dataStorageAccess"; interface WindowParams { @@ -161,12 +177,10 @@ export const freighterApiMessageListener = ( const safeResolve = (value: any) => { if (resolved) return; resolved = true; - sidebarQueueUuids.delete(uuid); resolve(value); }; if (popup === null) { - sidebarQueueUuids.add(uuid); setTimeout( () => safeResolve({ @@ -269,7 +283,6 @@ export const freighterApiMessageListener = ( const safeResolve = (value: any) => { if (resolved) return; resolved = true; - sidebarQueueUuids.delete(uuid); resolve(value); }; @@ -277,18 +290,10 @@ export const freighterApiMessageListener = ( safeResolve({ apiError: FreighterApiInternalError, }); - } else if (popup === null) { - sidebarQueueUuids.add(uuid); - } else { - const onWindowRemoved = (removedWindowId: number) => { - if (removedWindowId === popup.id) { - browser.windows.onRemoved.removeListener(onWindowRemoved); - safeResolve({ - apiError: FreighterApiDeclinedError, - }); - } - }; - browser.windows.onRemoved.addListener(onWindowRemoved); + } else if (popup !== null) { + rejectOnWindowClose(popup.id!, safeResolve, { + apiError: FreighterApiDeclinedError, + }); } const response = (success: boolean) => { if (success) { @@ -431,7 +436,6 @@ export const freighterApiMessageListener = ( const safeResolve = (value: any) => { if (resolved) return; resolved = true; - sidebarQueueUuids.delete(uuid); resolve(value); }; @@ -442,7 +446,6 @@ export const freighterApiMessageListener = ( error: FreighterApiInternalError.message, }); } else if (popup === null) { - sidebarQueueUuids.add(uuid); setTimeout( () => safeResolve({ @@ -452,17 +455,7 @@ export const freighterApiMessageListener = ( QUEUE_ITEM_TTL_MS, ); } else { - const onWindowRemoved = (removedWindowId: number) => { - if (removedWindowId === popup.id) { - browser.windows.onRemoved.removeListener(onWindowRemoved); - safeResolve({ - // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface - apiError: FreighterApiDeclinedError, - error: FreighterApiDeclinedError.message, - }); - } - }; - browser.windows.onRemoved.addListener(onWindowRemoved); + rejectOnWindowClose(popup.id!, safeResolve); } const response = ( signedTransaction: string, @@ -526,7 +519,6 @@ export const freighterApiMessageListener = ( const safeResolve = (value: any) => { if (resolved) return; resolved = true; - sidebarQueueUuids.delete(uuid); resolve(value); }; @@ -537,7 +529,6 @@ export const freighterApiMessageListener = ( error: FreighterApiInternalError.message, }); } else if (popup === null) { - sidebarQueueUuids.add(uuid); setTimeout( () => safeResolve({ @@ -547,17 +538,7 @@ export const freighterApiMessageListener = ( QUEUE_ITEM_TTL_MS, ); } else { - const onWindowRemoved = (removedWindowId: number) => { - if (removedWindowId === popup.id) { - browser.windows.onRemoved.removeListener(onWindowRemoved); - safeResolve({ - // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface - apiError: FreighterApiDeclinedError, - error: FreighterApiDeclinedError.message, - }); - } - }; - browser.windows.onRemoved.addListener(onWindowRemoved); + rejectOnWindowClose(popup.id!, safeResolve); } const response = ( @@ -635,7 +616,6 @@ export const freighterApiMessageListener = ( const safeResolve = (value: any) => { if (resolved) return; resolved = true; - sidebarQueueUuids.delete(uuid); resolve(value); }; @@ -646,7 +626,6 @@ export const freighterApiMessageListener = ( error: FreighterApiInternalError.message, }); } else if (popup === null) { - sidebarQueueUuids.add(uuid); setTimeout( () => safeResolve({ @@ -656,17 +635,7 @@ export const freighterApiMessageListener = ( QUEUE_ITEM_TTL_MS, ); } else { - const onWindowRemoved = (removedWindowId: number) => { - if (removedWindowId === popup.id) { - browser.windows.onRemoved.removeListener(onWindowRemoved); - safeResolve({ - // return 2 error formats: one for clients running older versions of freighter-api, and one to adhere to the standard wallet interface - apiError: FreighterApiDeclinedError, - error: FreighterApiDeclinedError.message, - }); - } - }; - browser.windows.onRemoved.addListener(onWindowRemoved); + rejectOnWindowClose(popup.id!, safeResolve); } const response = ( signedAuthEntry: SignAuthEntryResponse, @@ -791,12 +760,10 @@ export const freighterApiMessageListener = ( const safeResolve = (value: any) => { if (resolved) return; resolved = true; - sidebarQueueUuids.delete(uuid); resolve(value); }; if (popup === null) { - sidebarQueueUuids.add(uuid); setTimeout( () => safeResolve({ diff --git a/extension/src/background/messageListener/popupMessageListener.ts b/extension/src/background/messageListener/popupMessageListener.ts index bd7fb245ca..f82adf80f4 100644 --- a/extension/src/background/messageListener/popupMessageListener.ts +++ b/extension/src/background/messageListener/popupMessageListener.ts @@ -27,7 +27,9 @@ import { publicKeySelector } from "background/ducks/session"; import { startQueueCleanup, activeQueueUuids, + sidebarQueueUuids, } from "background/helpers/queueCleanup"; +import { getSidebarPort } from "./freighterApiMessageListener"; import { fundAccount } from "./handlers/fundAccount"; import { createAccount } from "./handlers/createAccount"; @@ -578,8 +580,12 @@ export const popupMessageListener = ( const { uuid, isActive } = request as MarkQueueActiveMessage; if (isActive) { activeQueueUuids.add(uuid); + if (getSidebarPort() !== null) { + sidebarQueueUuids.add(uuid); + } } else { activeQueueUuids.delete(uuid); + sidebarQueueUuids.delete(uuid); } return {}; } From 95d251ffe50184a2a2ea8444563c10b23200bc8a Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Thu, 2 Apr 2026 17:08:30 -0400 Subject: [PATCH 17/24] test: add coverage for sidebar disconnect cleanup and MARK_QUEUE_ACTIVE 16 tests covering deferred disconnect cleanup (cancellation on reconnect, queue rejection, stale port handling) and sidebarQueueUuids management via MARK_QUEUE_ACTIVE. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../__tests__/sidebarDisconnect.test.ts | 427 ++++++++++++++++++ 1 file changed, 427 insertions(+) create mode 100644 extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts diff --git a/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts b/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts new file mode 100644 index 0000000000..e4f435ff1a --- /dev/null +++ b/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts @@ -0,0 +1,427 @@ +import browser from "webextension-polyfill"; +import { + setSidebarPort, + clearSidebarPort, + getSidebarPort, +} from "background/messageListener/freighterApiMessageListener"; +import { + clearSidebarWindowId, + responseQueue, + transactionQueue, + blobQueue, + authEntryQueue, + tokenQueue, +} from "background/messageListener/popupMessageListener"; +import { + sidebarQueueUuids, + activeQueueUuids, +} from "background/helpers/queueCleanup"; +import { initSidebarConnectionListener } from "background/index"; +import { SIDEBAR_PORT_NAME } from "popup/components/SidebarSigningListener"; + +// Mock browser API +jest.mock("webextension-polyfill", () => ({ + runtime: { + onConnect: { + addListener: jest.fn(), + }, + getURL: (path: string) => `chrome-extension://fake-id${path}`, + }, + windows: { + onRemoved: { + addListener: jest.fn(), + removeListener: jest.fn(), + }, + }, +})); + +// Suppress buildStore and other background imports +jest.mock("background/store", () => ({ + buildStore: jest.fn(), +})); + +function createMockPort( + name: string, + sender?: { tab?: { id: number }; url?: string }, +): browser.Runtime.Port { + const messageListeners: Array<(msg: unknown) => void> = []; + const disconnectListeners: Array<() => void> = []; + + return { + name, + sender, + postMessage: jest.fn(), + disconnect: jest.fn(), + onMessage: { + addListener: (fn: (msg: unknown) => void) => messageListeners.push(fn), + removeListener: jest.fn(), + hasListener: jest.fn(), + hasListeners: jest.fn(), + }, + onDisconnect: { + addListener: (fn: () => void) => disconnectListeners.push(fn), + removeListener: jest.fn(), + hasListener: jest.fn(), + hasListeners: jest.fn(), + }, + // Helpers for tests to trigger events + _fireMessage: (msg: unknown) => messageListeners.forEach((fn) => fn(msg)), + _fireDisconnect: () => disconnectListeners.forEach((fn) => fn()), + } as unknown as browser.Runtime.Port & { + _fireMessage: (msg: unknown) => void; + _fireDisconnect: () => void; + }; +} + +type MockPort = ReturnType; + +describe("initSidebarConnectionListener", () => { + let onConnectCallback: (port: browser.Runtime.Port) => void; + + beforeEach(() => { + jest.useFakeTimers(); + jest.clearAllMocks(); + + // Clear global state + clearSidebarPort(); + clearSidebarWindowId(); + sidebarQueueUuids.clear(); + activeQueueUuids.clear(); + responseQueue.length = 0; + transactionQueue.length = 0; + blobQueue.length = 0; + authEntryQueue.length = 0; + tokenQueue.length = 0; + + // Register the listener + initSidebarConnectionListener(); + onConnectCallback = (browser.runtime.onConnect.addListener as jest.Mock) + .mock.calls[0][0]; + }); + + afterEach(() => { + jest.useRealTimers(); + clearSidebarPort(); + clearSidebarWindowId(); + sidebarQueueUuids.clear(); + activeQueueUuids.clear(); + }); + + function connectSidebarPort(sender?: { + tab?: { id: number }; + url?: string; + }): MockPort { + const port = createMockPort( + SIDEBAR_PORT_NAME, + sender ?? { + url: "chrome-extension://fake-id/index.html", + }, + ); + onConnectCallback(port); + return port as unknown as MockPort; + } + + describe("port connection", () => { + it("ignores ports with non-sidebar names", () => { + const port = createMockPort("other-port"); + onConnectCallback(port); + expect(getSidebarPort()).toBeNull(); + }); + + it("rejects connections from content scripts (has sender.tab)", () => { + const port = createMockPort(SIDEBAR_PORT_NAME, { + tab: { id: 1 }, + url: "https://evil.com", + }); + onConnectCallback(port); + expect(port.disconnect).toHaveBeenCalled(); + expect(getSidebarPort()).toBeNull(); + }); + + it("accepts connections from extension pages", () => { + const port = connectSidebarPort(); + expect(getSidebarPort()).toBe(port); + }); + + it("sets sidebarWindowId from the first message", () => { + const { + getSidebarWindowId, + } = require("background/messageListener/popupMessageListener"); + const port = connectSidebarPort(); + (port as any)._fireMessage({ windowId: 42 }); + expect(getSidebarWindowId()).toBe(42); + }); + + it("ignores malformed windowId messages", () => { + const { + getSidebarWindowId, + } = require("background/messageListener/popupMessageListener"); + clearSidebarWindowId(); + const port = connectSidebarPort(); + (port as any)._fireMessage({ windowId: "not-a-number" }); + expect(getSidebarWindowId()).toBeNull(); + (port as any)._fireMessage({ foo: "bar" }); + expect(getSidebarWindowId()).toBeNull(); + }); + }); + + describe("disconnect cleanup", () => { + it("clears port and windowId when active port disconnects", () => { + const { + getSidebarWindowId, + } = require("background/messageListener/popupMessageListener"); + const port = connectSidebarPort(); + (port as any)._fireMessage({ windowId: 42 }); + + (port as any)._fireDisconnect(); + + expect(getSidebarPort()).toBeNull(); + expect(getSidebarWindowId()).toBeNull(); + }); + + it("rejects sidebar requests after the deferred cleanup fires", () => { + const port = connectSidebarPort(); + const mockResponse = jest.fn(); + + sidebarQueueUuids.add("uuid-1"); + responseQueue.push({ + response: mockResponse, + uuid: "uuid-1", + createdAt: Date.now(), + } as any); + transactionQueue.push({ + transaction: {} as any, + uuid: "uuid-1", + createdAt: Date.now(), + }); + + (port as any)._fireDisconnect(); + + // Before timeout fires, request should still be in queue + expect(mockResponse).not.toHaveBeenCalled(); + expect(responseQueue).toHaveLength(1); + + // After timeout fires, request should be rejected + jest.advanceTimersByTime(500); + + expect(mockResponse).toHaveBeenCalledWith(undefined); + expect(responseQueue).toHaveLength(0); + expect(transactionQueue).toHaveLength(0); + expect(sidebarQueueUuids.size).toBe(0); + }); + + it("cleans up all queue types on disconnect", () => { + const port = connectSidebarPort(); + const uuid = "uuid-all-queues"; + + sidebarQueueUuids.add(uuid); + responseQueue.push({ + response: jest.fn(), + uuid, + createdAt: Date.now(), + } as any); + transactionQueue.push({ + transaction: {} as any, + uuid, + createdAt: Date.now(), + }); + blobQueue.push({ blob: {} as any, uuid, createdAt: Date.now() }); + authEntryQueue.push({ + authEntry: {} as any, + uuid, + createdAt: Date.now(), + }); + tokenQueue.push({ token: {} as any, uuid, createdAt: Date.now() }); + + (port as any)._fireDisconnect(); + jest.advanceTimersByTime(500); + + expect(responseQueue).toHaveLength(0); + expect(transactionQueue).toHaveLength(0); + expect(blobQueue).toHaveLength(0); + expect(authEntryQueue).toHaveLength(0); + expect(tokenQueue).toHaveLength(0); + }); + + it("does not reject popup-originated requests (not in sidebarQueueUuids)", () => { + const port = connectSidebarPort(); + const mockResponse = jest.fn(); + + // UUID is in responseQueue but NOT in sidebarQueueUuids + activeQueueUuids.add("popup-uuid"); + responseQueue.push({ + response: mockResponse, + uuid: "popup-uuid", + createdAt: Date.now(), + } as any); + + (port as any)._fireDisconnect(); + jest.advanceTimersByTime(500); + + expect(mockResponse).not.toHaveBeenCalled(); + expect(responseQueue).toHaveLength(1); + }); + }); + + describe("reconnect cancels pending cleanup", () => { + it("cancels rejection when a new port connects before timeout", () => { + const portA = connectSidebarPort(); + const mockResponse = jest.fn(); + + sidebarQueueUuids.add("uuid-1"); + responseQueue.push({ + response: mockResponse, + uuid: "uuid-1", + createdAt: Date.now(), + } as any); + + // Disconnect old port (schedules cleanup) + (portA as any)._fireDisconnect(); + + // New port connects before timeout fires (sidebar reloaded) + const portB = connectSidebarPort(); + + // Advance past the cleanup timeout + jest.advanceTimersByTime(500); + + // Request should NOT have been rejected + expect(mockResponse).not.toHaveBeenCalled(); + expect(responseQueue).toHaveLength(1); + expect(sidebarQueueUuids.has("uuid-1")).toBe(true); + expect(getSidebarPort()).toBe(portB); + }); + + it("rejects after reconnect if the second port also disconnects", () => { + const portA = connectSidebarPort(); + const mockResponse = jest.fn(); + + sidebarQueueUuids.add("uuid-1"); + responseQueue.push({ + response: mockResponse, + uuid: "uuid-1", + createdAt: Date.now(), + } as any); + + // Disconnect portA + (portA as any)._fireDisconnect(); + + // Reconnect portB (cancels portA's cleanup) + const portB = connectSidebarPort(); + + // Advance past portA's cleanup — should be cancelled + jest.advanceTimersByTime(500); + expect(mockResponse).not.toHaveBeenCalled(); + + // Now portB disconnects too (sidebar actually closed) + (portB as any)._fireDisconnect(); + jest.advanceTimersByTime(500); + + expect(mockResponse).toHaveBeenCalledWith(undefined); + expect(responseQueue).toHaveLength(0); + expect(sidebarQueueUuids.size).toBe(0); + }); + + it("stale port disconnect does not clear a newer port's state", () => { + const portA = connectSidebarPort(); + setSidebarPort(portA as any); + + // New port connects (simulates sidebar reload) + const portB = connectSidebarPort(); + + // portA's disconnect fires late + (portA as any)._fireDisconnect(); + + // portB should still be the active port + expect(getSidebarPort()).toBe(portB); + }); + }); +}); + +describe("MARK_QUEUE_ACTIVE with sidebarQueueUuids", () => { + const { + popupMessageListener, + } = require("background/messageListener/popupMessageListener"); + + const mockSessionStore = { + getState: jest.fn().mockReturnValue({ session: { publicKey: "" } }), + } as any; + const mockLocalStore = { + getItem: jest.fn().mockResolvedValue(null), + setItem: jest.fn(), + } as any; + const mockKeyManager = {} as any; + const mockSessionTimer = {} as any; + + beforeEach(() => { + clearSidebarPort(); + sidebarQueueUuids.clear(); + activeQueueUuids.clear(); + }); + + afterEach(() => { + clearSidebarPort(); + sidebarQueueUuids.clear(); + activeQueueUuids.clear(); + }); + + it("adds to both activeQueueUuids and sidebarQueueUuids when sidebar port is connected", async () => { + const mockPort = { postMessage: jest.fn(), disconnect: jest.fn() } as any; + setSidebarPort(mockPort); + + await popupMessageListener( + { type: "MARK_QUEUE_ACTIVE", uuid: "test-uuid", isActive: true } as any, + mockSessionStore, + mockLocalStore, + mockKeyManager, + mockSessionTimer, + ); + + expect(activeQueueUuids.has("test-uuid")).toBe(true); + expect(sidebarQueueUuids.has("test-uuid")).toBe(true); + }); + + it("adds to activeQueueUuids only when no sidebar port is connected", async () => { + await popupMessageListener( + { type: "MARK_QUEUE_ACTIVE", uuid: "test-uuid", isActive: true } as any, + mockSessionStore, + mockLocalStore, + mockKeyManager, + mockSessionTimer, + ); + + expect(activeQueueUuids.has("test-uuid")).toBe(true); + expect(sidebarQueueUuids.has("test-uuid")).toBe(false); + }); + + it("removes from both sets when isActive is false", async () => { + activeQueueUuids.add("test-uuid"); + sidebarQueueUuids.add("test-uuid"); + + await popupMessageListener( + { type: "MARK_QUEUE_ACTIVE", uuid: "test-uuid", isActive: false } as any, + mockSessionStore, + mockLocalStore, + mockKeyManager, + mockSessionTimer, + ); + + expect(activeQueueUuids.has("test-uuid")).toBe(false); + expect(sidebarQueueUuids.has("test-uuid")).toBe(false); + }); + + it("removes from sidebarQueueUuids even if sidebar port is no longer connected", async () => { + sidebarQueueUuids.add("test-uuid"); + activeQueueUuids.add("test-uuid"); + + // No sidebar port connected + await popupMessageListener( + { type: "MARK_QUEUE_ACTIVE", uuid: "test-uuid", isActive: false } as any, + mockSessionStore, + mockLocalStore, + mockKeyManager, + mockSessionTimer, + ); + + expect(sidebarQueueUuids.has("test-uuid")).toBe(false); + }); +}); From 161e8bc3b15df84010e1c0be819a65bad73691a3 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Thu, 2 Apr 2026 17:21:27 -0400 Subject: [PATCH 18/24] refactor: move sidebar port helpers to background/helpers/sidebarPort Extract setSidebarPort, clearSidebarPort, getSidebarPort from freighterApiMessageListener into their own module to reduce coupling. Update all imports across source and test files. Co-Authored-By: Claude Opus 4.6 (1M context) --- extension/src/background/helpers/sidebarPort.ts | 12 ++++++++++++ extension/src/background/index.ts | 2 +- .../messageListener/__tests__/sidebar.test.ts | 2 +- .../__tests__/sidebarDisconnect.test.ts | 2 +- .../freighterApiMessageListener.ts | 16 ++++------------ .../messageListener/popupMessageListener.ts | 2 +- 6 files changed, 20 insertions(+), 16 deletions(-) create mode 100644 extension/src/background/helpers/sidebarPort.ts diff --git a/extension/src/background/helpers/sidebarPort.ts b/extension/src/background/helpers/sidebarPort.ts new file mode 100644 index 0000000000..9ee21f3767 --- /dev/null +++ b/extension/src/background/helpers/sidebarPort.ts @@ -0,0 +1,12 @@ +import browser from "webextension-polyfill"; + +// Long-lived port to the sidebar, set by initSidebarConnectionListener +let sidebarPort: browser.Runtime.Port | null = null; + +export const setSidebarPort = (port: browser.Runtime.Port) => { + sidebarPort = port; +}; +export const clearSidebarPort = () => { + sidebarPort = null; +}; +export const getSidebarPort = () => sidebarPort; diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index 4447718b56..14f4549d97 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -21,7 +21,7 @@ import { setSidebarPort, clearSidebarPort, getSidebarPort, -} from "./messageListener/freighterApiMessageListener"; +} from "./helpers/sidebarPort"; import { freighterApiMessageListener } from "./messageListener/freighterApiMessageListener"; import { SIDEBAR_PORT_NAME } from "popup/components/SidebarSigningListener"; import { sidebarQueueUuids } from "./helpers/queueCleanup"; diff --git a/extension/src/background/messageListener/__tests__/sidebar.test.ts b/extension/src/background/messageListener/__tests__/sidebar.test.ts index 19420b85da..f91c32c8f6 100644 --- a/extension/src/background/messageListener/__tests__/sidebar.test.ts +++ b/extension/src/background/messageListener/__tests__/sidebar.test.ts @@ -4,7 +4,7 @@ import { setSidebarPort, clearSidebarPort, getSidebarPort, -} from "background/messageListener/freighterApiMessageListener"; +} from "background/helpers/sidebarPort"; // Mock chrome.sidePanel API const mockSetOptions = jest.fn().mockResolvedValue(undefined); diff --git a/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts b/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts index e4f435ff1a..dd5a5857e5 100644 --- a/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts +++ b/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts @@ -3,7 +3,7 @@ import { setSidebarPort, clearSidebarPort, getSidebarPort, -} from "background/messageListener/freighterApiMessageListener"; +} from "background/helpers/sidebarPort"; import { clearSidebarWindowId, responseQueue, diff --git a/extension/src/background/messageListener/freighterApiMessageListener.ts b/extension/src/background/messageListener/freighterApiMessageListener.ts index e1e8f3f188..763763093b 100644 --- a/extension/src/background/messageListener/freighterApiMessageListener.ts +++ b/extension/src/background/messageListener/freighterApiMessageListener.ts @@ -71,24 +71,16 @@ import { } from "./popupMessageListener"; import { QUEUE_ITEM_TTL_MS } from "background/helpers/queueCleanup"; -// Long-lived port to the sidebar, set by initSidebarConnectionListener -let sidebarPort: browser.Runtime.Port | null = null; - -export const setSidebarPort = (port: browser.Runtime.Port) => { - sidebarPort = port; -}; -export const clearSidebarPort = () => { - sidebarPort = null; -}; -export const getSidebarPort = () => sidebarPort; +import { getSidebarPort } from "background/helpers/sidebarPort"; const openSigningWindow = async (hashRoute: string, width?: number) => { const sidebarWindowId = getSidebarWindowId(); if (sidebarWindowId !== null) { // Send navigation directly to the sidebar via its long-lived port // instead of broadcasting to all extension listeners - if (sidebarPort) { - sidebarPort.postMessage({ type: SIDEBAR_NAVIGATE, route: hashRoute }); + const currentPort = getSidebarPort(); + if (currentPort) { + currentPort.postMessage({ type: SIDEBAR_NAVIGATE, route: hashRoute }); } try { if ((browser as any).sidebarAction) { diff --git a/extension/src/background/messageListener/popupMessageListener.ts b/extension/src/background/messageListener/popupMessageListener.ts index f82adf80f4..efd2274d3a 100644 --- a/extension/src/background/messageListener/popupMessageListener.ts +++ b/extension/src/background/messageListener/popupMessageListener.ts @@ -29,7 +29,7 @@ import { activeQueueUuids, sidebarQueueUuids, } from "background/helpers/queueCleanup"; -import { getSidebarPort } from "./freighterApiMessageListener"; +import { getSidebarPort } from "background/helpers/sidebarPort"; import { fundAccount } from "./handlers/fundAccount"; import { createAccount } from "./handlers/createAccount"; From b409632c3bfa7a4bcf1c4ec035099aff320eb0ad Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 21:26:17 +0000 Subject: [PATCH 19/24] fix: rejectSigningRequest returns consistent empty object instead of void Agent-Logs-Url: https://github.com/stellar/freighter/sessions/2d47d7af-dc71-4412-8251-1486f2c3b5df Co-authored-by: piyalbasu <6789586+piyalbasu@users.noreply.github.com> --- .../messageListener/handlers/rejectSigningRequest.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/extension/src/background/messageListener/handlers/rejectSigningRequest.ts b/extension/src/background/messageListener/handlers/rejectSigningRequest.ts index 3ccfe02970..7a0e63db23 100644 --- a/extension/src/background/messageListener/handlers/rejectSigningRequest.ts +++ b/extension/src/background/messageListener/handlers/rejectSigningRequest.ts @@ -48,7 +48,7 @@ export const rejectSigningRequest = ({ if (!uuid) { captureException("rejectSigningRequest: missing uuid in request"); - return; + return {}; } // Resolve (reject) the dapp's pending promise @@ -74,4 +74,6 @@ export const rejectSigningRequest = ({ const tokenIndex = tokenQueue.findIndex((item) => item.uuid === uuid); if (tokenIndex !== -1) tokenQueue.splice(tokenIndex, 1); + + return {}; }; From 9f46630e91fa879e91d4bbe9191a431c12907e85 Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Fri, 3 Apr 2026 11:28:36 -0400 Subject: [PATCH 20/24] docs: add sidebar mode implementation spec Add CLAUDE.md pointing to specs/ directory and a comprehensive sidebar mode spec covering architecture, signing flow, Firefox limitations, and E2E testing constraints. Co-Authored-By: Claude Opus 4.6 (1M context) --- extension/CLAUDE.md | 7 ++ extension/specs/SIDEBAR_MODE.md | 151 ++++++++++++++++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 extension/CLAUDE.md create mode 100644 extension/specs/SIDEBAR_MODE.md diff --git a/extension/CLAUDE.md b/extension/CLAUDE.md new file mode 100644 index 0000000000..77b6dd9405 --- /dev/null +++ b/extension/CLAUDE.md @@ -0,0 +1,7 @@ +# CLAUDE.md + +This file provides guidance to Claude Code when working with the Freighter browser extension. + +## Specs + +Implementation specs for complex features live in `extension/specs/`. Check there for architectural context before deep codebase exploration. diff --git a/extension/specs/SIDEBAR_MODE.md b/extension/specs/SIDEBAR_MODE.md new file mode 100644 index 0000000000..baafdc4fb7 --- /dev/null +++ b/extension/specs/SIDEBAR_MODE.md @@ -0,0 +1,151 @@ +# Sidebar Mode Implementation Spec + +## Overview + +Freighter's sidebar mode allows the extension to render in the browser's side panel instead of a popup window. When active, signing requests navigate within the sidebar rather than opening separate popup windows. + +## How Sidebar Mode Is Detected + +**File:** `src/popup/helpers/isSidebarMode.ts` + +```ts +export const isSidebarMode = () => + /^(chrome|moz)-extension:$/.test(window.location.protocol) && + new URLSearchParams(window.location.search).get("mode") === "sidebar"; +``` + +The URL `index.html?mode=sidebar` is set in the manifest and passed by `openSidebar()`. + +## How Sidebar Mode Is Activated + +**UI entry point:** `src/popup/components/account/AccountHeader/index.tsx` (lines 185-200) + +The account options dropdown (test ID: `account-options-dropdown`) shows a "Sidebar mode" menu item, conditionally rendered only when the browser supports it (`chrome.sidePanel.open` or `browser.sidebarAction.open`). + +**Activation function:** `src/popup/helpers/navigate.ts` — `openSidebar()` + +- **Chrome:** Calls `chrome.sidePanel.setOptions({ path: "index.html?mode=sidebar" })` then `chrome.sidePanel.open({ windowId })`. +- **Firefox:** Calls `browser.sidebarAction.open()`. +- In both cases, `window.close()` is called afterward to close the popup. + +**Manifest config:** + +```json +"permissions": ["storage", "alarms", "sidePanel"], +"side_panel": { "default_path": "index.html?mode=sidebar" }, +"sidebar_action": { + "default_panel": "index.html?mode=sidebar", + "open_at_install": false +} +``` + +## Architecture: Signing Flow in Sidebar Mode + +### 1. Sidebar connects to background + +When `isSidebarMode()` returns true, `Router.tsx` mounts ``. + +**File:** `src/popup/components/SidebarSigningListener/index.tsx` + +On mount, the component: + +1. Opens a long-lived port to the background: `browser.runtime.connect({ name: "sidebar" })` +2. Sends the window ID to the background: `port.postMessage({ windowId: win.id })` +3. Overrides `window.close()` to navigate to the account route instead of closing the panel +4. Listens for `SIDEBAR_NAVIGATE` messages on the port + +### 2. Background registers the sidebar + +**File:** `src/background/index.ts` — `initSidebarConnectionListener()` (line 65) + +When the background receives a port connection named "sidebar": + +1. **Validates the sender** — rejects content scripts (`!port.sender?.tab` check ensures only extension pages connect) +2. Stores the port via `setSidebarPort(port)` (in `src/background/helpers/sidebarPort.ts`) +3. Stores the window ID via `setSidebarWindowId()` when the sidebar sends its first message +4. On disconnect, clears state and schedules a 500ms deferred cleanup (to allow quick sidebar reloads without dropping requests) + +### 3. Signing request routing + +**File:** `src/background/messageListener/freighterApiMessageListener.ts` — `openSigningWindow()` (line 76) + +When a dApp triggers a signing request: + +1. Checks `getSidebarWindowId()` — if not null, sidebar is active +2. **Sidebar path:** Sends `{ type: SIDEBAR_NAVIGATE, route: hashRoute }` over the port, then calls `chrome.sidePanel.open()` to focus it. Returns `null` (no popup created). +3. **Popup fallback:** If no sidebar, creates a standalone popup window via `browser.windows.create()` + +### 4. Sidebar handles navigation + +Back in `SidebarSigningListener`, the port handler receives the `SIDEBAR_NAVIGATE` message: + +1. **Route validation:** Only allows navigation to known signing routes (`signTransaction`, `signAuthEntry`, `signMessage`, `grantAccess`, `addToken`, `reviewAuthorization`) +2. **Concurrent request handling:** If the user is already on a signing route, navigates to `ConfirmSidebarRequest` interstitial instead of silently swapping the screen +3. **Otherwise:** Navigates directly to the signing route via React Router + +### 5. Concurrent request interstitial + +**File:** `src/popup/views/ConfirmSidebarRequest/index.tsx` + +Shows "New Signing Request" with two options: + +- **Reject:** Extracts the UUID from the pending route's query string, calls `rejectSigningRequest()` to clean up all queues, navigates to account +- **Continue to review:** Navigates to the new signing route (passed via `?next=` query param, validated against open redirect) + +## Queue Tracking + +**File:** `src/background/helpers/queueCleanup.ts` + +`sidebarQueueUuids: Set` tracks which signing requests were routed to the sidebar. When the sidebar disconnects (and doesn't reconnect within 500ms), only these UUIDs are rejected — standalone popup requests have their own `onWindowRemoved` cleanup. + +## Key Constants + +| Constant | Value | Location | +| ------------------------------ | ---------------------------- | ---------------------------------- | +| `SIDEBAR_PORT_NAME` | `"sidebar"` | `SidebarSigningListener/index.tsx` | +| `SIDEBAR_NAVIGATE` | `"SIDEBAR_NAVIGATE"` | `@shared/constants/services.ts` | +| `ROUTES.confirmSidebarRequest` | `"/confirm-sidebar-request"` | `popup/constants/routes.ts` | + +## "Open Sidebar Mode by Default" — Chrome Only + +The Preferences page (`src/popup/views/Preferences/index.tsx`, line 185) has a toggle labeled "Open sidebar mode by default" that makes the extension icon click open the sidebar instead of the popup. This toggle is **only shown on Chrome** — it's gated behind `typeof globalThis.chrome?.sidePanel?.open === "function"`. + +**Why it's hidden on Firefox:** + +The toggle works by calling `chrome.sidePanel.setPanelBehavior({ openPanelOnActionClick: true })` (in `src/background/index.ts` — `initSidebarBehavior()`, line 217, and `src/background/messageListener/handlers/saveSettings.ts`, line 44). This is a Chrome-only API that tells the browser to open the side panel instead of the popup when the user clicks the extension's toolbar icon. + +Firefox has no equivalent API. `browser.sidebarAction` can open/close the sidebar programmatically, but: + +1. `sidebarAction.open()` requires a synchronous user gesture (it cannot be called from a background script in response to an action click) +2. There is no `setPanelBehavior` equivalent to redirect action clicks to the sidebar + +The setting is persisted in `localStorage` under `IS_OPEN_SIDEBAR_BY_DEFAULT_ID` (`"isOpenSidebarByDefault"`). On Chrome, `initSidebarBehavior()` reads this on startup and applies it via `setPanelBehavior`. On Firefox, the function is a no-op — the comment at line 228 documents this explicitly. + +Firefox users can still open sidebar mode manually via the "Sidebar mode" menu item in the account dropdown, or via the browser's native sidebar toggle. + +## E2E Testing Limitations + +**Playwright cannot test true sidebar mode.** The background's `initSidebarConnectionListener` rejects port connections from pages that have `port.sender.tab` set. In Playwright, the extension page runs in a regular browser tab (not a real side panel), so the port is always rejected. This means: + +- `getSidebarWindowId()` is always null in Playwright +- `openSigningWindow()` always falls back to creating popup windows +- The `SidebarSigningListener` component mounts but its port connection is rejected + +To properly E2E test sidebar signing flow, Playwright would need to support Chrome's Side Panel API so the extension page runs without a tab context. + +**What was tried and why it failed:** We attempted to create sidebar-specific E2E tests by navigating to `?mode=sidebar` in Playwright. While `isSidebarMode()` returns true on the frontend, the background rejects the `SidebarSigningListener` port connection because the page runs in a tab (`port.sender.tab` is set). All signing requests fall back to popup windows, making the tests functionally identical to the existing popup tests. True sidebar E2E tests require either Playwright support for Chrome's Side Panel API, or removing the `!port.sender.tab` guard in the background (which would weaken security). + +## File Reference + +| File | Role | +| --------------------------------------------------------------- | -------------------------------------------------------------------------------- | +| `src/popup/helpers/isSidebarMode.ts` | Detects sidebar mode via URL param | +| `src/popup/helpers/navigate.ts` | `openSidebar()` — opens side panel | +| `src/popup/components/SidebarSigningListener/index.tsx` | Port connection, navigation listener, window.close override | +| `src/popup/views/ConfirmSidebarRequest/index.tsx` | Concurrent request interstitial | +| `src/popup/Router.tsx` | Conditionally mounts SidebarSigningListener, defines confirmSidebarRequest route | +| `src/popup/components/account/AccountHeader/index.tsx` | "Sidebar mode" dropdown menu item | +| `src/background/index.ts` | `initSidebarConnectionListener()` — port validation, state management, cleanup | +| `src/background/helpers/sidebarPort.ts` | Global sidebar port state | +| `src/background/helpers/queueCleanup.ts` | `sidebarQueueUuids` set | +| `src/background/messageListener/freighterApiMessageListener.ts` | `openSigningWindow()` — routes to sidebar or popup | From f5dc8c88014d247e7e92a8a1a814f8dad7f6eccd Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Fri, 3 Apr 2026 12:06:49 -0400 Subject: [PATCH 21/24] fix: track sidebar-routed UUIDs at routing time and guard disconnect cleanup Two fixes to sidebar queue management: 1. Move sidebarQueueUuids tracking from MARK_QUEUE_ACTIVE (view mount time) to openSigningWindow (routing time). This ensures requests behind the ConfirmSidebarRequest interstitial are properly cleaned up if the sidebar disconnects before the signing view mounts. 2. Only schedule deferred disconnect cleanup when the disconnecting port is the currently active sidebar port. Previously, a stale port disconnecting after a newer port connected would still schedule cleanup, potentially rejecting requests on the live sidebar. Co-Authored-By: Claude Opus 4.6 (1M context) --- extension/src/background/index.ts | 11 +++-- .../__tests__/sidebarDisconnect.test.ts | 35 ++++++++------- .../freighterApiMessageListener.ts | 43 ++++++++++++++++--- .../messageListener/popupMessageListener.ts | 7 ++- 4 files changed, 66 insertions(+), 30 deletions(-) diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index 14f4549d97..8aa2d04384 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -106,12 +106,17 @@ export const initSidebarConnectionListener = () => { // When sidebar closes, clear port state and schedule cleanup. // The cleanup is deferred because chrome.sidePanel.open() can // reload the sidebar page, causing a brief disconnect/reconnect. + // Only schedule cleanup when the disconnecting port is the currently + // active sidebar port — a stale port disconnecting should not trigger + // cleanup while a newer port is still connected. port.onDisconnect.addListener(() => { - if (getSidebarPort() === port) { - clearSidebarPort(); - clearSidebarWindowId(); + if (getSidebarPort() !== port) { + return; } + clearSidebarPort(); + clearSidebarWindowId(); + pendingCleanup = setTimeout(() => { pendingCleanup = null; diff --git a/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts b/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts index dd5a5857e5..9fc833db6d 100644 --- a/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts +++ b/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts @@ -321,18 +321,33 @@ describe("initSidebarConnectionListener", () => { expect(sidebarQueueUuids.size).toBe(0); }); - it("stale port disconnect does not clear a newer port's state", () => { + it("stale port disconnect does not clear a newer port's state or schedule cleanup", () => { const portA = connectSidebarPort(); setSidebarPort(portA as any); // New port connects (simulates sidebar reload) const portB = connectSidebarPort(); - // portA's disconnect fires late + const mockResponse = jest.fn(); + sidebarQueueUuids.add("uuid-1"); + responseQueue.push({ + response: mockResponse, + uuid: "uuid-1", + createdAt: Date.now(), + } as any); + + // portA's disconnect fires late — should be a no-op (portA as any)._fireDisconnect(); // portB should still be the active port expect(getSidebarPort()).toBe(portB); + + // Advance past cleanup timeout — nothing should be rejected + // because stale port disconnect doesn't schedule cleanup + jest.advanceTimersByTime(500); + expect(mockResponse).not.toHaveBeenCalled(); + expect(responseQueue).toHaveLength(1); + expect(sidebarQueueUuids.has("uuid-1")).toBe(true); }); }); }); @@ -364,7 +379,7 @@ describe("MARK_QUEUE_ACTIVE with sidebarQueueUuids", () => { activeQueueUuids.clear(); }); - it("adds to both activeQueueUuids and sidebarQueueUuids when sidebar port is connected", async () => { + it("adds to activeQueueUuids only — sidebarQueueUuids is populated at routing time in openSigningWindow", async () => { const mockPort = { postMessage: jest.fn(), disconnect: jest.fn() } as any; setSidebarPort(mockPort); @@ -377,19 +392,7 @@ describe("MARK_QUEUE_ACTIVE with sidebarQueueUuids", () => { ); expect(activeQueueUuids.has("test-uuid")).toBe(true); - expect(sidebarQueueUuids.has("test-uuid")).toBe(true); - }); - - it("adds to activeQueueUuids only when no sidebar port is connected", async () => { - await popupMessageListener( - { type: "MARK_QUEUE_ACTIVE", uuid: "test-uuid", isActive: true } as any, - mockSessionStore, - mockLocalStore, - mockKeyManager, - mockSessionTimer, - ); - - expect(activeQueueUuids.has("test-uuid")).toBe(true); + // sidebarQueueUuids is no longer set here; it's set in openSigningWindow expect(sidebarQueueUuids.has("test-uuid")).toBe(false); }); diff --git a/extension/src/background/messageListener/freighterApiMessageListener.ts b/extension/src/background/messageListener/freighterApiMessageListener.ts index 763763093b..74a82fe3c5 100644 --- a/extension/src/background/messageListener/freighterApiMessageListener.ts +++ b/extension/src/background/messageListener/freighterApiMessageListener.ts @@ -69,13 +69,25 @@ import { tokenQueue, transactionQueue, } from "./popupMessageListener"; -import { QUEUE_ITEM_TTL_MS } from "background/helpers/queueCleanup"; +import { + QUEUE_ITEM_TTL_MS, + sidebarQueueUuids, +} from "background/helpers/queueCleanup"; import { getSidebarPort } from "background/helpers/sidebarPort"; -const openSigningWindow = async (hashRoute: string, width?: number) => { +const openSigningWindow = async ( + hashRoute: string, + uuid: string, + width?: number, +) => { const sidebarWindowId = getSidebarWindowId(); if (sidebarWindowId !== null) { + // Track this UUID as sidebar-routed at routing time so it gets + // cleaned up on sidebar disconnect, even if the signing view + // hasn't mounted yet (e.g. ConfirmSidebarRequest interstitial). + sidebarQueueUuids.add(uuid); + // Send navigation directly to the sidebar via its long-lived port // instead of broadcasting to all extension listeners const currentPort = getSidebarPort(); @@ -162,7 +174,10 @@ export const freighterApiMessageListener = ( const uuid = crypto.randomUUID(); const encodeOrigin = encodeObject({ tab, url: tabUrl, uuid }); - const popup = await openSigningWindow(`/grant-access?${encodeOrigin}`); + const popup = await openSigningWindow( + `/grant-access?${encodeOrigin}`, + uuid, + ); return new Promise((resolve) => { let resolved = false; @@ -268,7 +283,10 @@ export const freighterApiMessageListener = ( tokenQueue.push({ token: tokenInfo, uuid, createdAt: Date.now() }); const encodedTokenInfo = encodeObject(tokenInfo); - const popup = await openSigningWindow(`/add-token?${encodedTokenInfo}`); + const popup = await openSigningWindow( + `/add-token?${encodedTokenInfo}`, + uuid, + ); return new Promise((resolve) => { let resolved = false; @@ -421,7 +439,10 @@ export const freighterApiMessageListener = ( }); const encodedBlob = encodeObject(transactionInfo); - const popup = await openSigningWindow(`/sign-transaction?${encodedBlob}`); + const popup = await openSigningWindow( + `/sign-transaction?${encodedBlob}`, + uuid, + ); return new Promise((resolve) => { let resolved = false; @@ -504,7 +525,10 @@ export const freighterApiMessageListener = ( blobQueue.push({ blob: blobData, uuid, createdAt: Date.now() }); const encodedBlob = encodeObject(blobData); - const popup = await openSigningWindow(`/sign-message?${encodedBlob}`); + const popup = await openSigningWindow( + `/sign-message?${encodedBlob}`, + uuid, + ); return new Promise((resolve) => { let resolved = false; @@ -601,6 +625,7 @@ export const freighterApiMessageListener = ( const encodedAuthEntry = encodeObject(authEntry); const popup = await openSigningWindow( `/sign-auth-entry?${encodedAuthEntry}`, + uuid, ); return new Promise((resolve) => { @@ -745,7 +770,11 @@ export const freighterApiMessageListener = ( const uuid = crypto.randomUUID(); const encodeOrigin = encodeObject({ tab, url: tabUrl, uuid }); - const popup = await openSigningWindow(`/grant-access?${encodeOrigin}`, 400); + const popup = await openSigningWindow( + `/grant-access?${encodeOrigin}`, + uuid, + 400, + ); return new Promise((resolve) => { let resolved = false; diff --git a/extension/src/background/messageListener/popupMessageListener.ts b/extension/src/background/messageListener/popupMessageListener.ts index efd2274d3a..eab6e85b1a 100644 --- a/extension/src/background/messageListener/popupMessageListener.ts +++ b/extension/src/background/messageListener/popupMessageListener.ts @@ -29,7 +29,6 @@ import { activeQueueUuids, sidebarQueueUuids, } from "background/helpers/queueCleanup"; -import { getSidebarPort } from "background/helpers/sidebarPort"; import { fundAccount } from "./handlers/fundAccount"; import { createAccount } from "./handlers/createAccount"; @@ -580,9 +579,9 @@ export const popupMessageListener = ( const { uuid, isActive } = request as MarkQueueActiveMessage; if (isActive) { activeQueueUuids.add(uuid); - if (getSidebarPort() !== null) { - sidebarQueueUuids.add(uuid); - } + // sidebarQueueUuids is populated at routing time in + // openSigningWindow, not here — this avoids missing requests + // that are behind the ConfirmSidebarRequest interstitial. } else { activeQueueUuids.delete(uuid); sidebarQueueUuids.delete(uuid); From ed555b194a7ba14bc508a3df6627655b33828d2f Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Fri, 3 Apr 2026 12:08:36 -0400 Subject: [PATCH 22/24] docs: update sidebar spec to reflect queue tracking changes Co-Authored-By: Claude Opus 4.6 (1M context) --- extension/specs/SIDEBAR_MODE.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/extension/specs/SIDEBAR_MODE.md b/extension/specs/SIDEBAR_MODE.md index baafdc4fb7..4661a85a9f 100644 --- a/extension/specs/SIDEBAR_MODE.md +++ b/extension/specs/SIDEBAR_MODE.md @@ -96,7 +96,9 @@ Shows "New Signing Request" with two options: **File:** `src/background/helpers/queueCleanup.ts` -`sidebarQueueUuids: Set` tracks which signing requests were routed to the sidebar. When the sidebar disconnects (and doesn't reconnect within 500ms), only these UUIDs are rejected — standalone popup requests have their own `onWindowRemoved` cleanup. +`sidebarQueueUuids: Set` tracks which signing requests were routed to the sidebar. UUIDs are added at routing time in `openSigningWindow()` (in `freighterApiMessageListener.ts`) when the sidebar path is chosen — not at view mount time. This ensures requests behind the `ConfirmSidebarRequest` interstitial are tracked before the signing view mounts. + +When the sidebar disconnects (and doesn't reconnect within 500ms), only these UUIDs are rejected — standalone popup requests have their own `onWindowRemoved` cleanup. The disconnect cleanup only fires when the disconnecting port is the currently active sidebar port; stale port disconnects are ignored. ## Key Constants From 25bf3cb76ea3330a6f353741856c8053d8b0f3cf Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Fri, 3 Apr 2026 12:35:33 -0400 Subject: [PATCH 23/24] feat: add sidebarMode property to signing view metrics Includes isSidebarMode() in the view metrics for signTransaction, signAuthEntry, signMessage, grantAccess, and addToken so we can distinguish sidebar vs popup signing requests in analytics. Co-Authored-By: Claude Opus 4.6 (1M context) --- extension/src/popup/metrics/views.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/extension/src/popup/metrics/views.ts b/extension/src/popup/metrics/views.ts index 367d5003f4..1bcd9191f7 100644 --- a/extension/src/popup/metrics/views.ts +++ b/extension/src/popup/metrics/views.ts @@ -3,6 +3,7 @@ import { METRIC_NAMES } from "popup/constants/metricsNames"; import { registerHandler, emitMetric } from "helpers/metrics"; import { getTransactionInfo } from "helpers/stellar"; import { parsedSearchParam, getUrlHostname, getUrlDomain } from "helpers/urls"; +import { isSidebarMode } from "popup/helpers/isSidebarMode"; import { navigate } from "popup/ducks/views"; import { AppState } from "popup/App"; @@ -83,11 +84,14 @@ registerHandler(navigate, (_, a) => { } // "/sign-transaction" and "/grant-access" require additional metrics on loaded page + const isSidebarModeActivated = isSidebarMode(); + if (pathname === ROUTES.grantAccess) { const { url } = parsedSearchParam(search); const METRIC_OPTION_DOMAIN = { domain: getUrlDomain(url), subdomain: getUrlHostname(url), + sidebarMode: isSidebarModeActivated, }; emitMetric(eventName, METRIC_OPTION_DOMAIN); @@ -96,6 +100,7 @@ registerHandler(navigate, (_, a) => { const METRIC_OPTIONS = { domain: getUrlDomain(url), subdomain: getUrlHostname(url), + sidebarMode: isSidebarModeActivated, }; emitMetric(eventName, METRIC_OPTIONS); @@ -107,7 +112,7 @@ registerHandler(navigate, (_, a) => { const METRIC_OPTIONS = { domain: getUrlDomain(url), subdomain: getUrlHostname(url), - + sidebarMode: isSidebarModeActivated, number_of_operations: operations.length, operationTypes, }; @@ -122,6 +127,7 @@ registerHandler(navigate, (_, a) => { const METRIC_OPTIONS = { domain: getUrlDomain(url), subdomain: getUrlHostname(url), + sidebarMode: isSidebarModeActivated, }; emitMetric(eventName, METRIC_OPTIONS); From 7357a8aae9f458ba619c8e4f5affc3d4e21501dc Mon Sep 17 00:00:00 2001 From: Piyal Basu Date: Fri, 3 Apr 2026 14:40:55 -0400 Subject: [PATCH 24/24] fix: harden sidebar message handling per security review - M1: Make sender required in popupMessageListener so omitting it fails-closed instead of bypassing the extension-page guard - M2: Add sender.id check to sidebar port validation for defense-in-depth alongside the existing tab and URL checks - M3: Add isFromExtensionPage guard to REJECT_SIGNING_REQUEST handler - M4: Add runtime windowId type validation in OPEN_SIDEBAR handler Co-Authored-By: Claude Opus 4.6 (1M context) --- extension/src/background/index.ts | 1 + .../messageListener/__tests__/createAccount.test.ts | 5 +++++ .../background/messageListener/__tests__/sidebar.test.ts | 3 ++- .../messageListener/__tests__/sidebarDisconnect.test.ts | 3 +++ .../src/background/messageListener/popupMessageListener.ts | 7 ++++--- 5 files changed, 15 insertions(+), 4 deletions(-) diff --git a/extension/src/background/index.ts b/extension/src/background/index.ts index 8aa2d04384..32278e084f 100644 --- a/extension/src/background/index.ts +++ b/extension/src/background/index.ts @@ -75,6 +75,7 @@ export const initSidebarConnectionListener = () => { // Extension pages have no associated tab and load from the extension origin. const isExtensionPage = !port.sender?.tab && + port.sender?.id === browser.runtime.id && port.sender?.url?.startsWith(browser.runtime.getURL("")); if (!isExtensionPage) { port.disconnect(); diff --git a/extension/src/background/messageListener/__tests__/createAccount.test.ts b/extension/src/background/messageListener/__tests__/createAccount.test.ts index 2ac13131ef..44a6dd24bd 100644 --- a/extension/src/background/messageListener/__tests__/createAccount.test.ts +++ b/extension/src/background/messageListener/__tests__/createAccount.test.ts @@ -48,6 +48,7 @@ describe("Create account message listener", () => { mockDataStorage, mockKeyManager, testAlarm, + { id: "fake-extension-id" }, )) as Awaited>; const { session } = mockSessionStore.getState(); @@ -74,6 +75,7 @@ describe("Create account message listener", () => { mockDataStorage, mockKeyManager, testAlarm, + { id: "fake-extension-id" }, )) as Awaited>; const keyId = await mockDataStorage.getItem(KEY_ID); @@ -87,6 +89,7 @@ describe("Create account message listener", () => { mockDataStorage, mockKeyManager, testAlarm, + { id: "fake-extension-id" }, )) as Awaited>; const secondKeyId = await mockDataStorage.getItem(KEY_ID); const keyIdList = await mockDataStorage.getItem(KEY_ID_LIST); @@ -109,6 +112,7 @@ describe("Create account message listener", () => { mockDataStorage, mockKeyManager, testAlarm, + { id: "fake-extension-id" }, )) as Awaited>; const keyId = await mockDataStorage.getItem(KEY_ID); @@ -122,6 +126,7 @@ describe("Create account message listener", () => { mockDataStorage, mockKeyManager, testAlarm, + { id: "fake-extension-id" }, )) as Awaited>; const secondKeyId = await mockDataStorage.getItem(KEY_ID); const keyIdList = await mockDataStorage.getItem(KEY_ID_LIST); diff --git a/extension/src/background/messageListener/__tests__/sidebar.test.ts b/extension/src/background/messageListener/__tests__/sidebar.test.ts index f91c32c8f6..815939d7db 100644 --- a/extension/src/background/messageListener/__tests__/sidebar.test.ts +++ b/extension/src/background/messageListener/__tests__/sidebar.test.ts @@ -74,13 +74,14 @@ describe("sidebar message handlers", () => { expect(mockOpen).toHaveBeenCalledWith({ windowId: 42 }); }); - it("opens the sidebar when no sender is provided", async () => { + it("opens the sidebar when sender is from this extension", async () => { const result = await popupMessageListener( request as any, mockSessionStore, mockLocalStore, mockKeyManager, mockSessionTimer, + {}, ); expect(result).toEqual({}); expect(mockSetOptions).toHaveBeenCalled(); diff --git a/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts b/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts index 9fc833db6d..445c979fab 100644 --- a/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts +++ b/extension/src/background/messageListener/__tests__/sidebarDisconnect.test.ts @@ -389,6 +389,7 @@ describe("MARK_QUEUE_ACTIVE with sidebarQueueUuids", () => { mockLocalStore, mockKeyManager, mockSessionTimer, + { id: "fake-extension-id" }, ); expect(activeQueueUuids.has("test-uuid")).toBe(true); @@ -406,6 +407,7 @@ describe("MARK_QUEUE_ACTIVE with sidebarQueueUuids", () => { mockLocalStore, mockKeyManager, mockSessionTimer, + { id: "fake-extension-id" }, ); expect(activeQueueUuids.has("test-uuid")).toBe(false); @@ -423,6 +425,7 @@ describe("MARK_QUEUE_ACTIVE with sidebarQueueUuids", () => { mockLocalStore, mockKeyManager, mockSessionTimer, + { id: "fake-extension-id" }, ); expect(sidebarQueueUuids.has("test-uuid")).toBe(false); diff --git a/extension/src/background/messageListener/popupMessageListener.ts b/extension/src/background/messageListener/popupMessageListener.ts index eab6e85b1a..01fb74c407 100644 --- a/extension/src/background/messageListener/popupMessageListener.ts +++ b/extension/src/background/messageListener/popupMessageListener.ts @@ -137,7 +137,7 @@ export const popupMessageListener = ( localStore: DataStorageAccess, keyManager: KeyManager, sessionTimer: SessionTimer, - sender?: { tab?: unknown; id?: string }, + sender: { tab?: unknown; id?: string }, ) => { const currentState = sessionStore.getState(); const publicKey = publicKeySelector(currentState); @@ -145,9 +145,8 @@ export const popupMessageListener = ( // Content scripts (dapp pages) always carry sender.tab; extension pages do not. // Also verify the message originates from this extension (sender.id matches), // guarding against other extensions calling popupMessageListener handlers. - // When sender is absent (internal calls), both checks pass by default. const isFromExtensionPage = - !sender?.tab && (!sender?.id || sender.id === browser.runtime.id); + !sender.tab && (!sender.id || sender.id === browser?.runtime?.id); if ( request.activePublicKey && @@ -353,6 +352,7 @@ export const popupMessageListener = ( }); } case SERVICE_TYPES.REJECT_SIGNING_REQUEST: { + if (!isFromExtensionPage) return { error: "Unauthorized" }; return rejectSigningRequest({ request, responseQueue, @@ -592,6 +592,7 @@ export const popupMessageListener = ( case SERVICE_TYPES.OPEN_SIDEBAR: { if (!isFromExtensionPage) return { error: "Unauthorized" }; const { windowId } = request as OpenSidebarMessage; + if (typeof windowId !== "number") return { error: "Invalid windowId" }; return (async () => { await chrome.sidePanel .setOptions({ path: "index.html?mode=sidebar", enabled: true })