From 0eb9a022aa950457c60998c7808c749164160690 Mon Sep 17 00:00:00 2001
From: Alison Gale <[email protected]>
Date: Thu, 09 Jul 2026 08:01:22 -0700
Subject: [PATCH] [Reading List] Only open urls from the reading list

- Use webui util for getting BWI
- Early return if the url isn't from the reading list

This prevents exploits where navigations can be sent from the wrong
context to trigger JS alerts. Since alert calls can't be in the reading
list, this will handle the issue.

Bug: 517394060
Change-Id: I6ce0d56f13c88ffd5aee5795090533d49715b9cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8060466
Commit-Queue: Alison Gale <[email protected]>
Reviewed-by: Caroline Rising <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1659552}
---

diff --git a/chrome/browser/ui/webui/side_panel/reading_list/BUILD.gn b/chrome/browser/ui/webui/side_panel/reading_list/BUILD.gn
index 9a5dce2..f3019566 100644
--- a/chrome/browser/ui/webui/side_panel/reading_list/BUILD.gn
+++ b/chrome/browser/ui/webui/side_panel/reading_list/BUILD.gn
@@ -52,6 +52,7 @@
     "//chrome/browser/ui/profiles",
     "//chrome/browser/ui/tabs:tab_strip",
     "//chrome/browser/ui/webui:favicon_source",
+    "//chrome/browser/ui/webui:webui_util",
     "//chrome/browser/ui/webui_browser",
     "//chrome/common",
     "//components/favicon_base",
@@ -73,6 +74,7 @@
     "//base/test:test_support",
     "//chrome/browser/prefs",
     "//chrome/browser/reading_list",
+    "//chrome/browser/ui/webui:webui_util",
     "//chrome/test:test_support",
     "//components/reading_list/core:test_support",
     "//content/test:test_support",
diff --git a/chrome/browser/ui/webui/side_panel/reading_list/reading_list_page_handler.cc b/chrome/browser/ui/webui/side_panel/reading_list/reading_list_page_handler.cc
index 90c6ec7a..b8d682ac 100644
--- a/chrome/browser/ui/webui/side_panel/reading_list/reading_list_page_handler.cc
+++ b/chrome/browser/ui/webui/side_panel/reading_list/reading_list_page_handler.cc
@@ -32,6 +32,7 @@
 #include "chrome/browser/ui/profiles/profile_view_utils.h"
 #include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/webui/side_panel/reading_list/reading_list_ui.h"
+#include "chrome/browser/ui/webui/webui_embedding_context.h"
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/profile_metrics/browser_profile_type.h"
@@ -179,9 +180,10 @@
 void ReadingListPageHandler::OpenURL(
     const GURL& url,
     ui::mojom::ClickModifiersPtr click_modifiers) {
-  BrowserWindowInterface* browser =
-      GlobalBrowserCollection::GetInstance()->GetLastActiveBrowser();
-  if (!browser) {
+  // Only support opening reading list entry URLS.
+  scoped_refptr<const ReadingListEntry> entry =
+      reading_list_model_->GetEntryByURL(url);
+  if (!entry) {
     return;
   }
 
@@ -193,18 +195,18 @@
 
   content::OpenURLParams params(url, content::Referrer(), open_location,
                                 ui::PAGE_TRANSITION_AUTO_BOOKMARK, false);
-  browser->GetBrowserForMigrationOnly()->OpenURL(
-      params,
-      /*navigation_handle_callback=*/{});
 
-  scoped_refptr<const ReadingListEntry> entry =
-      reading_list_model_->GetEntryByURL(url);
-  if (entry) {
-    base::RecordAction(base::UserMetricsAction(
-        entry->IsRead() ? "DesktopReadingList.Navigation.FromReadList"
-                        : "DesktopReadingList.Navigation.FromUnreadList"));
+  auto* browser_window_interface =
+      webui::GetBrowserWindowInterface(web_contents());
+  if (!browser_window_interface) {
+    return;
   }
+  browser_window_interface->OpenURL(params,
+                                    /*navigation_handle_callback=*/{});
 
+  base::RecordAction(base::UserMetricsAction(
+      entry->IsRead() ? "DesktopReadingList.Navigation.FromReadList"
+                      : "DesktopReadingList.Navigation.FromUnreadList"));
   base::RecordAction(
       base::UserMetricsAction("SidePanel.ReadingList.Navigation"));
   RecordBookmarkLaunch(
diff --git a/chrome/browser/ui/webui/side_panel/reading_list/reading_list_page_handler_unittest.cc b/chrome/browser/ui/webui/side_panel/reading_list/reading_list_page_handler_unittest.cc
index 8f9ffdc9..865d99a 100644
--- a/chrome/browser/ui/webui/side_panel/reading_list/reading_list_page_handler_unittest.cc
+++ b/chrome/browser/ui/webui/side_panel/reading_list/reading_list_page_handler_unittest.cc
@@ -10,11 +10,13 @@
 
 #include "base/memory/raw_ptr.h"
 #include "base/test/bind.h"
+#include "base/test/metrics/user_action_tester.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/reading_list/reading_list_model_factory.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/webui/webui_embedding_context.h"
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "chrome/test/base/test_browser_window.h"
@@ -99,6 +101,7 @@
 
     web_contents_ = content::WebContents::Create(
         content::WebContents::CreateParams(profile()));
+    webui::SetBrowserWindowInterface(web_contents_.get(), browser());
     test_web_ui_ = std::make_unique<content::TestWebUI>();
     test_web_ui_->set_web_contents(web_contents_.get());
 
@@ -124,6 +127,7 @@
   }
 
   void TearDown() override {
+    webui::SetBrowserWindowInterface(web_contents_.get(), nullptr);
     incognito_browser_.reset();
     handler_.reset();
     test_web_ui_.reset();
@@ -262,6 +266,53 @@
       /* expected_read_data= */ {});
 }
 
+TEST_F(TestReadingListPageHandlerTest, OpenURLNotInReadingList) {
+  const GURL not_in_reading_list_url("http://not-in-reading-list.com");
+  EXPECT_FALSE(model()->GetEntryByURL(not_in_reading_list_url));
+
+  base::UserActionTester user_action_tester;
+
+  const int tab_count_before = browser()->tab_strip_model()->count();
+  content::WebContents* active_web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  const GURL active_url_before = active_web_contents->GetVisibleURL();
+
+  // Try to open a URL that is not in the reading list.
+  handler()->OpenURL(not_in_reading_list_url, GetClickModifiers());
+
+  // Check that the URL was not opened (tab count and active tab URL remain
+  // unchanged).
+  EXPECT_EQ(browser()->tab_strip_model()->count(), tab_count_before);
+  EXPECT_EQ(active_web_contents->GetVisibleURL(), active_url_before);
+
+  // Try to open with middle click (which would open a new tab if URL were in
+  // reading list).
+  auto click_modifiers = GetClickModifiers();
+  click_modifiers->middle_button = true;
+  handler()->OpenURL(not_in_reading_list_url, std::move(click_modifiers));
+
+  // Check that no new tab was opened.
+  EXPECT_EQ(browser()->tab_strip_model()->count(), tab_count_before);
+
+  // Verify that no navigation metrics were recorded.
+  EXPECT_EQ(
+      user_action_tester.GetActionCount("SidePanel.ReadingList.Navigation"), 0);
+
+  // Expect ItemsChanged to be called four times from the two AddEntry calls in
+  // SetUp().
+  EXPECT_CALL(page_, ItemsChanged(testing::_)).Times(4);
+  // Expect CurrentPageActionButtonStateChanged to be called once.
+  EXPECT_CALL(page_, CurrentPageActionButtonStateChanged(testing::_)).Times(1);
+
+  // Get Read later entries.
+  GetAndVerifyReadLaterEntries(
+      /* unread_size= */ 2u, /* read_size= */ 0u,
+      /* expected_unread_data= */
+      {std::make_pair(GURL(kTabUrl3), kTabName3),
+       std::make_pair(GURL(kTabUrl1), kTabName1)},
+      /* expected_read_data= */ {});
+}
+
 TEST_F(TestReadingListPageHandlerTest, UpdateReadStatus) {
   handler()->UpdateReadStatus(GURL(kTabUrl3), true);
 

diff --git a/chrome/browser/ui/webui/side_panel/reading_list/reading_list_page_handler_unittest.cc b/chrome/browser/ui/webui/side_panel/reading_list/reading_list_page_handler_unittest.cc
index 8f9ffdc9..865d99a 100644
--- a/chrome/browser/ui/webui/side_panel/reading_list/reading_list_page_handler_unittest.cc
+++ b/chrome/browser/ui/webui/side_panel/reading_list/reading_list_page_handler_unittest.cc
@@ -10,11 +10,13 @@
 
 #include "base/memory/raw_ptr.h"
 #include "base/test/bind.h"
+#include "base/test/metrics/user_action_tester.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/reading_list/reading_list_model_factory.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/webui/webui_embedding_context.h"
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "chrome/test/base/test_browser_window.h"
@@ -99,6 +101,7 @@
 
     web_contents_ = content::WebContents::Create(
         content::WebContents::CreateParams(profile()));
+    webui::SetBrowserWindowInterface(web_contents_.get(), browser());
     test_web_ui_ = std::make_unique<content::TestWebUI>();
     test_web_ui_->set_web_contents(web_contents_.get());
 
@@ -124,6 +127,7 @@
   }
 
   void TearDown() override {
+    webui::SetBrowserWindowInterface(web_contents_.get(), nullptr);
     incognito_browser_.reset();
     handler_.reset();
     test_web_ui_.reset();
@@ -262,6 +266,53 @@
       /* expected_read_data= */ {});
 }
 
+TEST_F(TestReadingListPageHandlerTest, OpenURLNotInReadingList) {
+  const GURL not_in_reading_list_url("http://not-in-reading-list.com");
+  EXPECT_FALSE(model()->GetEntryByURL(not_in_reading_list_url));
+
+  base::UserActionTester user_action_tester;
+
+  const int tab_count_before = browser()->tab_strip_model()->count();
+  content::WebContents* active_web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  const GURL active_url_before = active_web_contents->GetVisibleURL();
+
+  // Try to open a URL that is not in the reading list.
+  handler()->OpenURL(not_in_reading_list_url, GetClickModifiers());
+
+  // Check that the URL was not opened (tab count and active tab URL remain
+  // unchanged).
+  EXPECT_EQ(browser()->tab_strip_model()->count(), tab_count_before);
+  EXPECT_EQ(active_web_contents->GetVisibleURL(), active_url_before);
+
+  // Try to open with middle click (which would open a new tab if URL were in
+  // reading list).
+  auto click_modifiers = GetClickModifiers();
+  click_modifiers->middle_button = true;
+  handler()->OpenURL(not_in_reading_list_url, std::move(click_modifiers));
+
+  // Check that no new tab was opened.
+  EXPECT_EQ(browser()->tab_strip_model()->count(), tab_count_before);
+
+  // Verify that no navigation metrics were recorded.
+  EXPECT_EQ(
+      user_action_tester.GetActionCount("SidePanel.ReadingList.Navigation"), 0);
+
+  // Expect ItemsChanged to be called four times from the two AddEntry calls in
+  // SetUp().
+  EXPECT_CALL(page_, ItemsChanged(testing::_)).Times(4);
+  // Expect CurrentPageActionButtonStateChanged to be called once.
+  EXPECT_CALL(page_, CurrentPageActionButtonStateChanged(testing::_)).Times(1);
+
+  // Get Read later entries.
+  GetAndVerifyReadLaterEntries(
+      /* unread_size= */ 2u, /* read_size= */ 0u,
+      /* expected_unread_data= */
+      {std::make_pair(GURL(kTabUrl3), kTabName3),
+       std::make_pair(GURL(kTabUrl1), kTabName1)},
+      /* expected_read_data= */ {});
+}
+
 TEST_F(TestReadingListPageHandlerTest, UpdateReadStatus) {
   handler()->UpdateReadStatus(GURL(kTabUrl3), true);