From ae41c6e4f6a8a98c536cb2e70b4b71c4e560c2c6 Mon Sep 17 00:00:00 2001
From: Dibyajyoti Pal <dibyapal@google.com>
Date: Tue, 14 Apr 2026 15:19:26 -0700
Subject: [PATCH] [PWA/Shortcuts] Disallow shortcut creation for sensitive chrome urls

The "Create Shortcut" flow would previously only work for sites that had
http/https in them, and wouldn't allow chrome:// sites. This CL
streamlines that behavior by using startup::ValidateUrl, which
allowlists urls that are non-sensitive and can be launched by Chrome.

For shortcuts created on Mac via .crwebloc files, there was nothing
stopping Chrome from opening Chrome sensitive urls if shortcuts were
created on them, so this CL fixes that, by ensuring that .crwebloc files
do not open these urls on Mac at all.

Verified with an automated test, and manually testing that this behavior
is not possible on Linux and Windows, and hence doesn't need fixing
there. On CrOS, the concept of desktop shortcuts don't exist.

Fixed: 502069297
Include-Ci-Only-Tests: chromium.mac:mac15-x64-rel-tests|browser_tests
Change-Id: I7b48645dea7f7d66acd20389d9cf0ad20517f413
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7757382
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Dibyajyoti Pal <dibyapal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1614742}
---

diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm
index 77e2f77..1bb7d48 100644
--- a/chrome/browser/app_controller_mac.mm
+++ b/chrome/browser/app_controller_mac.mm
@@ -127,6 +127,7 @@
 #include "content/public/browser/download_manager.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
+#include "extensions/buildflags/buildflags.h"
 #include "net/base/apple/url_conversions.h"
 #include "net/base/filename_util.h"
 #import "ui/base/cocoa/nsmenu_additions.h"
@@ -434,8 +435,22 @@
               base::flat_map<base::FilePath, std::vector<GURL>> profile_url_map;
               for (const auto& path : shortcuts) {
                 auto shortcut = shortcuts::ChromeWeblocFile::LoadFromFile(path);
+                // TODO: Consider opening the original file URL?
                 if (!shortcut.has_value()) {
-                  // TODO: Consider opening the original file URL?
+                  continue;
+                }
+                bool is_shortcut_url_valid =
+                    startup::ValidateUrl(shortcut->target_url());
+        // Do not allow chrome sensitive urls to be launched from a .crwebloc
+        // file.
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+                is_shortcut_url_valid =
+                    is_shortcut_url_valid || shortcut->target_url().SchemeIs(
+                                                 extensions::kExtensionScheme);
+#endif  // BUILDFLAG(ENABLE_EXTENSIONS)
+                if (!is_shortcut_url_valid) {
+                  LOG(ERROR) << "Not allowed to open target url: "
+                             << shortcut->target_url();
                   continue;
                 }
                 profile_url_map[shortcut->profile_path_name().path()].push_back(
diff --git a/chrome/browser/app_controller_mac_browsertest.mm b/chrome/browser/app_controller_mac_browsertest.mm
index 919af1f..8b3119f 100644
--- a/chrome/browser/app_controller_mac_browsertest.mm
+++ b/chrome/browser/app_controller_mac_browsertest.mm
@@ -25,6 +25,7 @@
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/single_thread_task_runner.h"
+#include "base/task/thread_pool/thread_pool_instance.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_restrictions.h"
@@ -964,6 +965,45 @@
 }
 
 IN_PROC_BROWSER_TEST_F(AppControllerShortcutsNotAppsBrowserTest,
+                       DisallowChromeUrlWeblocFile) {
+  // Ensure the AppController is the NSApp delegate.
+  std::ignore = AppController.sharedController;
+
+  // Create and open a .crwebloc file with a chrome:// URL.
+  GURL chrome_url("chrome://settings/");
+  base::ScopedTempDir temp_dir;
+  base::FilePath crwebloc_file;
+  {
+    base::ScopedAllowBlockingForTesting allow_blocking;
+    ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+    crwebloc_file = temp_dir.GetPath().AppendASCII("test_shortcut.crwebloc");
+    ASSERT_TRUE(shortcuts::ChromeWeblocFile(
+                    chrome_url, *base::SafeBaseName::Create(
+                                    browser()->profile()->GetPath()))
+                    .SaveToFile(crwebloc_file));
+  }
+
+  int initial_tab_count = browser()->tab_strip_model()->count();
+  content::WebContents* current_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  SendOpenUrlToAppController(net::FilePathToFileURL(crwebloc_file));
+  // Wait for any background tasks to complete and send its replies to the UI
+  // thread. This helps ensure that SendOpenUrlToAppController() completes.
+  base::ThreadPoolInstance::Get()->FlushForTesting();
+  base::RunLoop().RunUntilIdle();
+
+  // It should not be opened in the browser.
+  EXPECT_EQ(initial_tab_count, browser()->tab_strip_model()->count());
+  EXPECT_NE(chrome_url, current_contents->GetLastCommittedURL());
+
+  {
+    base::ScopedAllowBlockingForTesting allow_blocking;
+    EXPECT_TRUE(temp_dir.Delete());
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(AppControllerShortcutsNotAppsBrowserTest,
                        OpenChromeWeblocFileInSecondProfile) {
   ASSERT_TRUE(embedded_test_server()->Start());
   // Ensure the AppController is the NSApp delegate.

diff --git a/chrome/browser/app_controller_mac_browsertest.mm b/chrome/browser/app_controller_mac_browsertest.mm
index 919af1f..8b3119f 100644
--- a/chrome/browser/app_controller_mac_browsertest.mm
+++ b/chrome/browser/app_controller_mac_browsertest.mm
@@ -25,6 +25,7 @@
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/single_thread_task_runner.h"
+#include "base/task/thread_pool/thread_pool_instance.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_restrictions.h"
@@ -964,6 +965,45 @@
 }
 
 IN_PROC_BROWSER_TEST_F(AppControllerShortcutsNotAppsBrowserTest,
+                       DisallowChromeUrlWeblocFile) {
+  // Ensure the AppController is the NSApp delegate.
+  std::ignore = AppController.sharedController;
+
+  // Create and open a .crwebloc file with a chrome:// URL.
+  GURL chrome_url("chrome://settings/");
+  base::ScopedTempDir temp_dir;
+  base::FilePath crwebloc_file;
+  {
+    base::ScopedAllowBlockingForTesting allow_blocking;
+    ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+    crwebloc_file = temp_dir.GetPath().AppendASCII("test_shortcut.crwebloc");
+    ASSERT_TRUE(shortcuts::ChromeWeblocFile(
+                    chrome_url, *base::SafeBaseName::Create(
+                                    browser()->profile()->GetPath()))
+                    .SaveToFile(crwebloc_file));
+  }
+
+  int initial_tab_count = browser()->tab_strip_model()->count();
+  content::WebContents* current_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  SendOpenUrlToAppController(net::FilePathToFileURL(crwebloc_file));
+  // Wait for any background tasks to complete and send its replies to the UI
+  // thread. This helps ensure that SendOpenUrlToAppController() completes.
+  base::ThreadPoolInstance::Get()->FlushForTesting();
+  base::RunLoop().RunUntilIdle();
+
+  // It should not be opened in the browser.
+  EXPECT_EQ(initial_tab_count, browser()->tab_strip_model()->count());
+  EXPECT_NE(chrome_url, current_contents->GetLastCommittedURL());
+
+  {
+    base::ScopedAllowBlockingForTesting allow_blocking;
+    EXPECT_TRUE(temp_dir.Delete());
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(AppControllerShortcutsNotAppsBrowserTest,
                        OpenChromeWeblocFileInSecondProfile) {
   ASSERT_TRUE(embedded_test_server()->Start());
   // Ensure the AppController is the NSApp delegate.