From 40ec682b782ff7ae1ac57857c1a648786d1bc7f6 Mon Sep 17 00:00:00 2001
From: Matt Gibson <mgibson@bitwarden.com>
Date: Fri, 7 Nov 2025 16:58:35 +0000
Subject: [PATCH] [PM-27888] [PM-27889][PM-27914][PM-27820] Use frame Id as
 test for internal source. (#17266)

* Use frame Id as test for internal source.

* prefer strong equality

* Fix tests
---
 .../src/platform/browser/browser-api.ts       | 38 ++++++++++++++-----
 .../local-backed-session-storage.service.ts   |  2 +-
 .../background-task-scheduler.service.spec.ts |  1 +
 .../background-task-scheduler.service.ts      |  2 +-
 4 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/apps/browser/src/platform/browser/browser-api.ts b/apps/browser/src/platform/browser/browser-api.ts
index 25486d4556..76ec18f496 100644
--- a/apps/browser/src/platform/browser/browser-api.ts
+++ b/apps/browser/src/platform/browser/browser-api.ts
@@ -5,6 +5,7 @@ import { Observable } from "rxjs";
 import { BrowserClientVendors } from "@bitwarden/common/autofill/constants";
 import { BrowserClientVendor } from "@bitwarden/common/autofill/types";
 import { DeviceType } from "@bitwarden/common/enums";
+import { LogService } from "@bitwarden/logging";
 import { isBrowserSafariApi } from "@bitwarden/platform";
 
 import { TabMessage } from "../../types/tab-messages";
@@ -32,8 +33,20 @@ export class BrowserApi {
     return BrowserApi.manifestVersion === expectedVersion;
   }
 
-  static senderIsInternal(sender: chrome.runtime.MessageSender | undefined): boolean {
-    if (!sender?.url) {
+  /**
+   * Helper method that attempts to distinguish whether a message sender is internal to the extension or not.
+   *
+   * Currently this is done through source origin matching, and frameId checking (only top-level frames are internal).
+   * @param sender a message sender
+   * @param logger an optional logger to log validation results
+   * @returns whether or not the sender appears to be internal to the extension
+   */
+  static senderIsInternal(
+    sender: chrome.runtime.MessageSender | undefined,
+    logger?: LogService,
+  ): boolean {
+    if (!sender?.origin) {
+      logger?.warning("[BrowserApi] Message sender has no origin");
       return false;
     }
     const extensionUrl =
@@ -42,23 +55,28 @@ export class BrowserApi {
       "";
 
     if (!extensionUrl) {
+      logger?.warning("[BrowserApi] Unable to determine extension URL");
       return false;
     }
 
-    if (!sender.url.startsWith(extensionUrl)) {
+    // Normalize both URLs by removing trailing slashes
+    const normalizedOrigin = sender.origin.replace(/\/$/, "");
+    const normalizedExtensionUrl = extensionUrl.replace(/\/$/, "");
+
+    if (!normalizedOrigin.startsWith(normalizedExtensionUrl)) {
+      logger?.warning(
+        `[BrowserApi] Message sender origin (${normalizedOrigin}) does not match extension URL (${normalizedExtensionUrl})`,
+      );
       return false;
     }
 
-    // these are all properties on externally initiated messages, not internal ones
-    if (
-      "tab" in sender ||
-      "documentId" in sender ||
-      "documentLifecycle" in sender ||
-      "frameId" in sender
-    ) {
+    // We only send messages from the top-level frame, but frameId is only set if tab is set, which for popups it is not.
+    if ("frameId" in sender && sender.frameId !== 0) {
+      logger?.warning("[BrowserApi] Message sender is not from the top-level frame");
       return false;
     }
 
+    logger?.info("[BrowserApi] Message sender appears to be internal");
     return true;
   }
 
diff --git a/apps/browser/src/platform/services/local-backed-session-storage.service.ts b/apps/browser/src/platform/services/local-backed-session-storage.service.ts
index 26605fefd8..d0613ee644 100644
--- a/apps/browser/src/platform/services/local-backed-session-storage.service.ts
+++ b/apps/browser/src/platform/services/local-backed-session-storage.service.ts
@@ -43,7 +43,7 @@ export class LocalBackedSessionStorageService
       if (port.name !== portName(chrome.storage.session)) {
         return;
       }
-      if (!BrowserApi.senderIsInternal(port.sender)) {
+      if (!BrowserApi.senderIsInternal(port.sender, this.logService)) {
         return;
       }
 
diff --git a/apps/browser/src/platform/services/task-scheduler/background-task-scheduler.service.spec.ts b/apps/browser/src/platform/services/task-scheduler/background-task-scheduler.service.spec.ts
index aef1a5231f..46f5528d41 100644
--- a/apps/browser/src/platform/services/task-scheduler/background-task-scheduler.service.spec.ts
+++ b/apps/browser/src/platform/services/task-scheduler/background-task-scheduler.service.spec.ts
@@ -33,6 +33,7 @@ function createInternalPortSpyMock(name: string) {
     disconnect: jest.fn(),
     sender: {
       url: chrome.runtime.getURL(""),
+      origin: chrome.runtime.getURL(""),
     },
   });
 }
diff --git a/apps/browser/src/platform/services/task-scheduler/background-task-scheduler.service.ts b/apps/browser/src/platform/services/task-scheduler/background-task-scheduler.service.ts
index 746344a83f..5a18b42eb5 100644
--- a/apps/browser/src/platform/services/task-scheduler/background-task-scheduler.service.ts
+++ b/apps/browser/src/platform/services/task-scheduler/background-task-scheduler.service.ts
@@ -30,7 +30,7 @@ export class BackgroundTaskSchedulerService extends BrowserTaskSchedulerServiceI
     if (port.name !== BrowserTaskSchedulerPortName) {
       return;
     }
-    if (!BrowserApi.senderIsInternal(port.sender)) {
+    if (!BrowserApi.senderIsInternal(port.sender, this.logService)) {
       return;
     }