From 9ae39759da3c0ee89b53ef45989ffbf52488c48a Mon Sep 17 00:00:00 2001
From: Alex Rudenko <[email protected]>
Date: Wed, 23 Sep 2026 07:56:43 -0700
Subject: [PATCH] [DevTools] Redact inaccessible URLs in Page.frameStartedNavigating

A client that is not allowed to navigate the page to a URL must not
learn it via Page.frameStartedNavigating either. Check
CheckNavigationAllowed() in both PageHandler::Enable() and
PageHandler::DidStartNavigating() and redact the URL to an empty string
when disallowed, while still emitting the event so client navigation
bookkeeping continues to work.

Fixed: 563819290
Change-Id: I9a799b866c71d116006a28cf930a6da7eaddb76e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8439039
Auto-Submit: Alex Rudenko <[email protected]>
Commit-Queue: Maksim Sadym <[email protected]>
Reviewed-by: Maksim Sadym <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1703435}
---
diff --git a/content/browser/devtools/protocol/page_handler.cc b/content/browser/devtools/protocol/page_handler.cc
index 078c16a..c750e9a 100644
--- a/content/browser/devtools/protocol/page_handler.cc
+++ b/content/browser/devtools/protocol/page_handler.cc
@@ -683,11 +683,17 @@
     FrameTreeNode* frame_tree_node = host_->frame_tree_node();
     NavigationRequest* navigation_request =
         host_->frame_tree_node()->navigation_request();
+    // A client that is not allowed to navigate the page to a URL must not
+    // learn it either (see GetNavigationHistory()). Redact the URL rather
+    // than omitting the event, so that navigation bookkeeping in clients
+    // keeps working.
+    const GURL& pending_url = navigation_request->common_params().url;
+    const bool may_disclose = !CheckNavigationAllowed(pending_url).IsError();
     frontend_->FrameStartedNavigating(
         frame_tree_node->current_frame_host()
             ->devtools_frame_token()
             .ToString(),
-        navigation_request->common_params().url.spec(),
+        may_disclose ? pending_url.spec() : std::string(),
         navigation_request->devtools_navigation_token().ToString(),
         GetFrameStartedNavigatingNavigationTypeString(
             navigation_request->common_params().navigation_type));
@@ -1188,9 +1194,16 @@
     return;
   }
 
+  // A client that is not allowed to navigate the page to a URL must not
+  // learn it either (see GetNavigationHistory()). Redact the URL rather than
+  // omitting the event, so that navigation bookkeeping in clients keeps
+  // working. This event fires at FrameTreeNode::TakeNavigationRequest() time,
+  // BEFORE NavigationRequest::OnStartChecksComplete() force-detaches sessions
+  // whose client fails MayAttachToURL(), so it must gate disclosure itself.
+  const bool may_disclose = host_ && !CheckNavigationAllowed(url).IsError();
   frontend_->FrameStartedNavigating(
-      ftn.current_frame_host()->devtools_frame_token().ToString(), url.spec(),
-      loader_id.ToString(),
+      ftn.current_frame_host()->devtools_frame_token().ToString(),
+      may_disclose ? url.spec() : std::string(), loader_id.ToString(),
       GetFrameStartedNavigatingNavigationTypeString(navigation_type));
 }
 
diff --git a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
index 384ce53..c717343 100644
--- a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
+++ b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
@@ -2116,6 +2116,59 @@
 }
 
 IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest,
+                       FrameStartedNavigatingRedactsInaccessibleUrls) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  const GURL file_url = GetTestUrl("devtools", "navigation.html");
+  const GURL http_url = embedded_test_server()->GetURL("/title1.html");
+  const GURL allowed_url = embedded_test_server()->GetURL("/title2.html");
+  const GURL disallowed_url =
+      embedded_test_server()->GetURL("disallowed.test", "/title1.html");
+
+  NavigateToURLBlockUntilNavigationsComplete(shell(), http_url, 1);
+
+  set_agent_host_can_close();
+  SetMayReadLocalFiles(false);
+  SetNotAttachableHosts({"disallowed.test"});
+  Attach();
+  SendCommandSync("Page.enable");
+
+  // Accessible URLs are disclosed in Page.frameStartedNavigating.
+  NavigateToURLBlockUntilNavigationsComplete(shell(), allowed_url, 1);
+  base::DictValue notification =
+      WaitForNotification("Page.frameStartedNavigating", true);
+  EXPECT_EQ(allowed_url.spec(), *notification.FindString("url"));
+
+  // Navigating to a file:// URL without local file access still emits
+  // Page.frameStartedNavigating, but redacts the URL.
+  ClearNotifications();
+  NavigateToURLBlockUntilNavigationsComplete(shell(), file_url, 1);
+  notification = WaitForNotification("Page.frameStartedNavigating", true);
+  EXPECT_EQ("", *notification.FindString("url"));
+  EXPECT_FALSE(notification.FindString("loaderId")->empty());
+
+  // Calling Page.enable while a navigation to an inaccessible URL is already
+  // pending also redacts the URL in the catch-up FrameStartedNavigating event.
+  Detach();
+  NavigateToURLBlockUntilNavigationsComplete(shell(), http_url, 1);
+  TestNavigationManager pending_file_nav(shell()->web_contents(), file_url);
+  shell()->LoadURL(file_url);
+  ASSERT_TRUE(pending_file_nav.WaitForRequestStart());
+
+  Attach();
+  SendCommandAsync("Page.enable");
+  notification = WaitForNotification("Page.frameStartedNavigating", true);
+  EXPECT_EQ("", *notification.FindString("url"));
+  ASSERT_TRUE(pending_file_nav.WaitForNavigationFinished());
+
+  // Navigating to a host that fails MayAttachToURL() fires DidStartNavigating()
+  // before the session is force-detached; the URL must be redacted there too.
+  ClearNotifications();
+  shell()->LoadURL(disallowed_url);
+  notification = WaitForNotification("Page.frameStartedNavigating", true);
+  EXPECT_EQ("", *notification.FindString("url"));
+}
+
+IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest,
                        NavigateToHistoryEntryWithFileUrlRequiresFileAccess) {
   ASSERT_TRUE(embedded_test_server()->Start());
   const GURL file_url = GetTestUrl("devtools", "navigation.html");

diff --git a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
index 384ce53..c717343 100644
--- a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
+++ b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
@@ -2116,6 +2116,59 @@
 }
 
 IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest,
+                       FrameStartedNavigatingRedactsInaccessibleUrls) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  const GURL file_url = GetTestUrl("devtools", "navigation.html");
+  const GURL http_url = embedded_test_server()->GetURL("/title1.html");
+  const GURL allowed_url = embedded_test_server()->GetURL("/title2.html");
+  const GURL disallowed_url =
+      embedded_test_server()->GetURL("disallowed.test", "/title1.html");
+
+  NavigateToURLBlockUntilNavigationsComplete(shell(), http_url, 1);
+
+  set_agent_host_can_close();
+  SetMayReadLocalFiles(false);
+  SetNotAttachableHosts({"disallowed.test"});
+  Attach();
+  SendCommandSync("Page.enable");
+
+  // Accessible URLs are disclosed in Page.frameStartedNavigating.
+  NavigateToURLBlockUntilNavigationsComplete(shell(), allowed_url, 1);
+  base::DictValue notification =
+      WaitForNotification("Page.frameStartedNavigating", true);
+  EXPECT_EQ(allowed_url.spec(), *notification.FindString("url"));
+
+  // Navigating to a file:// URL without local file access still emits
+  // Page.frameStartedNavigating, but redacts the URL.
+  ClearNotifications();
+  NavigateToURLBlockUntilNavigationsComplete(shell(), file_url, 1);
+  notification = WaitForNotification("Page.frameStartedNavigating", true);
+  EXPECT_EQ("", *notification.FindString("url"));
+  EXPECT_FALSE(notification.FindString("loaderId")->empty());
+
+  // Calling Page.enable while a navigation to an inaccessible URL is already
+  // pending also redacts the URL in the catch-up FrameStartedNavigating event.
+  Detach();
+  NavigateToURLBlockUntilNavigationsComplete(shell(), http_url, 1);
+  TestNavigationManager pending_file_nav(shell()->web_contents(), file_url);
+  shell()->LoadURL(file_url);
+  ASSERT_TRUE(pending_file_nav.WaitForRequestStart());
+
+  Attach();
+  SendCommandAsync("Page.enable");
+  notification = WaitForNotification("Page.frameStartedNavigating", true);
+  EXPECT_EQ("", *notification.FindString("url"));
+  ASSERT_TRUE(pending_file_nav.WaitForNavigationFinished());
+
+  // Navigating to a host that fails MayAttachToURL() fires DidStartNavigating()
+  // before the session is force-detached; the URL must be redacted there too.
+  ClearNotifications();
+  shell()->LoadURL(disallowed_url);
+  notification = WaitForNotification("Page.frameStartedNavigating", true);
+  EXPECT_EQ("", *notification.FindString("url"));
+}
+
+IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest,
                        NavigateToHistoryEntryWithFileUrlRequiresFileAccess) {
   ASSERT_TRUE(embedded_test_server()->Start());
   const GURL file_url = GetTestUrl("devtools", "navigation.html");