From 04125fe8c8e2368ec8a2849fe2c73365e3083341 Mon Sep 17 00:00:00 2001
From: Chris Fredrickson <cfredric@chromium.org>
Date: Fri, 25 Sep 2026 08:32:43 -0700
Subject: [PATCH] [glic] Handle loaderless navigations and about-blank properly

Navigations to about:blank do not go through navigation gating prior to
this CL, since those navigations do not use URLLoaders and the
navigation throttle is not registered for loaderless navigations. This
CL fixes that by registering the throttle and adding the appropriate
override.

This CL also fixes ExecutionEngine's usage of GetLastCommittedOrigin and
GetLastCommittedUrl to take tainted about:blank pages into account
during origin gating decisions.

Fixed: 563004214
Change-Id: Id01a541ec02092e79f3f49bd75b84f12fdc5d92d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8451924
Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1704765}
---
diff --git a/chrome/browser/actor/actor_navigation_throttle.cc b/chrome/browser/actor/actor_navigation_throttle.cc
index 397ad25..39d8f41a 100644
--- a/chrome/browser/actor/actor_navigation_throttle.cc
+++ b/chrome/browser/actor/actor_navigation_throttle.cc
@@ -114,6 +114,24 @@
   return content::NavigationThrottle::DEFER;
 }
 
+content::NavigationThrottle::ThrottleCheckResult
+ActorNavigationThrottle::WillCommitWithoutUrlLoader() {
+  if (navigation_handle()->IsSameDocument()) {
+    return content::NavigationThrottle::PROCEED;
+  }
+  if (!navigation_handle()->IsRendererInitiated()) {
+    return WillStartOrRedirectRequest(/*is_redirection=*/false);
+  }
+  if (!execution_engine_) {
+    return content::NavigationThrottle::CANCEL_AND_IGNORE;
+  }
+  execution_engine_->ShouldNavigationCommit(
+      *navigation_handle(),
+      base::BindOnce(&ActorNavigationThrottle::OnNavigationConfirmationDecision,
+                     weak_factory_.GetWeakPtr()));
+  return content::NavigationThrottle::DEFER;
+}
+
 void ActorNavigationThrottle::OnNavigationConfirmationDecision(
     MayActOnUrlBlockReason block_reason) {
   if (block_reason == MayActOnUrlBlockReason::kAllowed) {
diff --git a/chrome/browser/actor/actor_navigation_throttle.h b/chrome/browser/actor/actor_navigation_throttle.h
index 1f8de5f..d59ae22 100644
--- a/chrome/browser/actor/actor_navigation_throttle.h
+++ b/chrome/browser/actor/actor_navigation_throttle.h
@@ -69,6 +69,8 @@
       override;
   content::NavigationThrottle::ThrottleCheckResult WillProcessResponse()
       override;
+  content::NavigationThrottle::ThrottleCheckResult WillCommitWithoutUrlLoader()
+      override;
   const char* GetNameForLogging() override;
 
  private:
diff --git a/chrome/browser/actor/execution_engine.cc b/chrome/browser/actor/execution_engine.cc
index 77a0a38c..4a790a90 100644
--- a/chrome/browser/actor/execution_engine.cc
+++ b/chrome/browser/actor/execution_engine.cc
@@ -535,6 +535,27 @@
       origin.GetTupleOrPrecursorTupleIfOpaque().GetURL());
 }
 
+// Returns `url_to_commit` if its origin/precursor already matches
+// `origin_to_commit`'s tuple or precursor (preserving path/query for URL-level
+// policy checks), or if `origin_to_commit` has no tuple/precursor (e.g.
+// browser-initiated about:blank).  Otherwise (e.g. renderer-initiated
+// about:blank or about:srcdoc that inherited a tuple/precursor origin), returns
+// the precursor/tuple origin's GURL so gating evaluates the true security
+// principal rather than "about:blank".
+GURL GetEffectiveUrlForGating(const GURL& url_to_commit,
+                              const url::Origin& origin_to_commit) {
+  const url::Origin effective_origin =
+      OriginOrPrecursorIfOpaque(origin_to_commit);
+  if (effective_origin.opaque()) {
+    return url_to_commit;
+  }
+  if (OriginOrPrecursorIfOpaque(url::Origin::Create(url_to_commit)) ==
+      effective_origin) {
+    return url_to_commit;
+  }
+  return effective_origin.GetURL();
+}
+
 ExecutionEngine::GatingDecision MapGatingDecisionToEngineDecision(
     const origin_gating::GatingDecision& decision) {
   switch (decision.attribution.type()) {
@@ -902,6 +923,12 @@
 
   const url::Origin source_origin = OriginOrPrecursorIfOpaque(
       GetPrimaryMainFrame(navigation_handle)->GetLastCommittedOrigin());
+  const GURL destination_url = GetEffectiveUrlForGating(
+      navigation_handle.GetURL(),
+      url::Origin::Resolve(
+          navigation_handle.GetURL(),
+          navigation_handle.GetInitiatorOrigin().value_or(url::Origin())));
+  const url::Origin destination_origin = url::Origin::Create(destination_url);
   auto event = GateableEvent::kNavigationResponse;
   auto wrapped_callback = TrackPendingNavigation(
       pending_navigation_cancellations_, std::move(callback),
@@ -911,15 +938,15 @@
           GetPrimaryMainFrame(navigation_handle)->GetPageUkmSourceId(),
           navigation_handle.IsInPrerenderedMainFrame(), std::move(timer),
           ExtractMimeType(navigation_handle)),
-      event, source_origin.GetURL(), navigation_handle.GetURL(),
+      event, source_origin.GetURL(), destination_url,
       base::BindOnce(
           &ExecutionEngine::OnComputedGatingDecision, GetWeakPtr(),
           std::move(wrapped_callback),
-          journal_->CreatePendingAsyncEntry(
-              navigation_handle.GetURL(), task_->id(),
-              MakeBrowserTrackUUID(task_->id()), "OriginGatingDecision", {}),
-          source_origin, url::Origin::Create(navigation_handle.GetURL()),
-          state_, navigation_handle.GetInitiatorOrigin(), event));
+          journal_->CreatePendingAsyncEntry(destination_url, task_->id(),
+                                            MakeBrowserTrackUUID(task_->id()),
+                                            "OriginGatingDecision", {}),
+          source_origin, destination_origin, state_,
+          navigation_handle.GetInitiatorOrigin(), event));
 }
 
 void ExecutionEngine::CancelPendingNavigations() {
@@ -1407,15 +1434,11 @@
     return;
   }
 
-  // Asynchronously check if we can act on the tab. NOTE that the check uses
-  // `GetLastCommittedURL()` from the tab. For opaque origins, this means that
-  // we'll get the precursor URL. For this reason, we previously added the
-  // precursor to `origin_gating_cache()` to ensure the optimization guide
-  // sensitive origin check would be skipped as expected.
-
   // TODO(mcnee): Add UMA for the outcomes.
   content::WebContents& web_contents = *(tab->GetContents());
-  const GURL& url = web_contents.GetPrimaryMainFrame()->GetLastCommittedURL();
+  content::RenderFrameHost* main_frame = web_contents.GetPrimaryMainFrame();
+  const GURL url = GetEffectiveUrlForGating(
+      main_frame->GetLastCommittedURL(), main_frame->GetLastCommittedOrigin());
   auto event = GateableEvent::kPageAction;
   GetOriginGatingChecker().ComputeGatingDecision(
       std::make_unique<PageActionGatingContext>(web_contents.GetWeakPtr()),
@@ -1431,13 +1454,11 @@
           }))
           .Then(base::BindOnce(&ExecutionEngine::DidFinishAsyncSafetyChecks,
                                GetActionSequenceWeakPtr(),
-                               tab->GetContents()
-                                   ->GetPrimaryMainFrame()
-                                   ->GetLastCommittedOrigin())));
+                               main_frame->GetLastCommittedOrigin())));
 }
 
 void ExecutionEngine::DidFinishAsyncSafetyChecks(
-    const url::Origin& evaluated_origin,
+    const url::Origin& committed_origin,
     mojom::ActionResultCode result_code) {
   TRACE_EVENT0("actor", "ExecutionEngine::DidFinishAsyncSafetyChecks");
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -1458,7 +1479,7 @@
   }
 
   TaskId task_id = task_->id();
-  if (!evaluated_origin.IsSameOriginWith(tab->GetContents()
+  if (!committed_origin.IsSameOriginWith(tab->GetContents()
                                              ->GetPrimaryMainFrame()
                                              ->GetLastCommittedOrigin())) {
     // A cross-origin navigation occurred before we got permission. The result
diff --git a/chrome/browser/actor/execution_engine.h b/chrome/browser/actor/execution_engine.h
index e17108b9..eca65740 100644
--- a/chrome/browser/actor/execution_engine.h
+++ b/chrome/browser/actor/execution_engine.h
@@ -371,7 +371,7 @@
 
   // Performs synchronous safety checks for the next action. If everything
   // passes calls tool_controller_.Invoke().
-  void DidFinishAsyncSafetyChecks(const url::Origin& evaluated_origin,
+  void DidFinishAsyncSafetyChecks(const url::Origin& committed_origin,
                                   mojom::ActionResultCode result_code);
 
   // If a failure occurs before the next action starts, we associate the tab
diff --git a/chrome/browser/chrome_content_browser_client_navigation_throttles.cc b/chrome/browser/chrome_content_browser_client_navigation_throttles.cc
index 5fe14ca..dfa341d 100644
--- a/chrome/browser/chrome_content_browser_client_navigation_throttles.cc
+++ b/chrome/browser/chrome_content_browser_client_navigation_throttles.cc
@@ -647,6 +647,8 @@
         registry);
   }
 
+  actor::ActorNavigationThrottle::MaybeCreateAndAdd(registry);
+
   // PwcNavigationThrottle must also cancel off-allowlist main-frame
   // navigations that commit without a URL loader (e.g. a subframe navigating
   // the main frame to about:blank), which never reach WillStartRequest().
diff --git a/chrome/browser/actor/execution_engine_origin_gating_browsertest.cc b/chrome/browser/actor/execution_engine_origin_gating_browsertest.cc
index ecb0bd4d..bc33d2b1 100644
--- a/chrome/browser/actor/execution_engine_origin_gating_browsertest.cc
+++ b/chrome/browser/actor/execution_engine_origin_gating_browsertest.cc
@@ -3162,4 +3162,118 @@
           url::Origin::Create(sensitive_url)))));
 }
 
+// Regression test for crbug.com/563004214: a cross-origin opener navigating an
+// acted-on tab to about:blank (which commits without a URL loader) must be
+// gated by ActorNavigationThrottle and must not poison the tab's committed
+// origin to bypass new-origin confirmation on subsequent navigations.
+IN_PROC_BROWSER_TEST_F(ExecutionEngineOriginGatingBrowserTest,
+                       OpenerCrossOriginAboutBlankNavigationGated) {
+  const GURL evil_url =
+      embedded_https_test_server().GetURL("foo.com", "/actor/blank.html");
+  const GURL start_url =
+      embedded_https_test_server().GetURL("example.com", "/actor/link.html");
+
+  content::WebContents* acted_contents = web_contents();
+  ASSERT_TRUE(content::NavigateToURL(acted_contents, start_url));
+
+  // Open a popup on foo.com before starting the actor task so foo.com holds a
+  // `window.opener` reference to `acted_contents`.
+  content::TestNavigationObserver popup_observer(evil_url);
+  popup_observer.StartWatchingNewWebContents();
+  ASSERT_TRUE(content::ExecJs(
+      acted_contents, content::JsReplace("window.open($1);", evil_url)));
+  popup_observer.Wait();
+  content::WebContents* popup_contents = web_contents();
+  ASSERT_NE(popup_contents, acted_contents);
+
+  // Reactivate tab 0 (`acted_contents`) before opening Glic and starting the
+  // actor task on it.
+  browser()->tab_strip_model()->ActivateTabAt(0);
+  ASSERT_EQ(web_contents(), acted_contents);
+
+  OpenGlicAndCreateTask();
+  actor_task().AddTab(active_tab()->GetHandle(), /*stop_task_on_detach=*/true,
+                      base::DoNothing());
+  ASSERT_TRUE(actor_task().HasTab(active_tab()->GetHandle()));
+
+  // Deny any navigation confirmation requests.
+  RunTestSequence(CreateMockWebClientRequest(
+      content::JsReplace(kHandleNavigationConfirmationTempl, false)));
+
+  // Step 1: Cross-origin popup attempts to navigate the acted-on tab to
+  // about:blank via window.opener. This must be gated against foo.com and
+  // denied.
+  content::TestNavigationObserver about_blank_observer(
+      acted_contents, content::MessageLoopRunner::QuitMode::IMMEDIATE,
+      /*ignore_uncommitted_navigations=*/false);
+  EXPECT_TRUE(content::ExecJs(popup_contents,
+                              "window.opener.location = 'about:blank';"));
+  about_blank_observer.Wait();
+
+  EXPECT_FALSE(about_blank_observer.last_navigation_succeeded());
+  EXPECT_EQ(about_blank_observer.last_initiator_origin(),
+            url::Origin::Create(evil_url));
+  EXPECT_EQ(acted_contents->GetLastCommittedURL(), start_url);
+  EXPECT_EQ(acted_contents->GetPrimaryMainFrame()->GetLastCommittedOrigin(),
+            url::Origin::Create(start_url));
+
+  RunTestSequence(VerifyNavigationConfirmationRequest(
+      base::test::ParseJsonDict(content::JsReplace(
+          R"({"navigationOrigin": $1, "taskId": $2})",
+          url::Origin::Create(evil_url), actor_task().id().value()))));
+
+  // Step 2: Same-origin navigation to about:blank initiated by the acted-on tab
+  // itself must still be allowed via kAllowSameOrigin without prompting.
+  content::TestNavigationObserver same_origin_blank_observer(acted_contents);
+  EXPECT_TRUE(
+      content::ExecJs(acted_contents, "window.location = 'about:blank';"));
+  same_origin_blank_observer.Wait();
+  EXPECT_TRUE(same_origin_blank_observer.last_navigation_succeeded());
+  EXPECT_EQ(same_origin_blank_observer.last_initiator_origin(),
+            url::Origin::Create(start_url));
+  EXPECT_EQ(acted_contents->GetLastCommittedURL(), GURL(url::kAboutBlankURL));
+  EXPECT_EQ(acted_contents->GetPrimaryMainFrame()->GetLastCommittedOrigin(),
+            url::Origin::Create(start_url));
+}
+
+// Regression test for crbug.com/563004214: if an acted-on tab is at about:blank
+// with an inherited sensitive origin, page action safety checks must evaluate
+// the effective origin rather than auto-allowing via kAllowAboutBlank.
+IN_PROC_BROWSER_TEST_F(ExecutionEngineHardcodedSensitiveSiteBrowserTest,
+                       PageActionOnInheritedAboutBlankPromptsUser) {
+  const GURL sensitive_url =
+      embedded_https_test_server().GetURL("bar.com", "/actor/blank.html");
+  ASSERT_TRUE(content::NavigateToURL(web_contents(), sensitive_url));
+
+  // Navigate from bar.com to about:blank so GetLastCommittedURL() is
+  // about:blank while GetLastCommittedOrigin() is https://bar.com.
+  content::TestNavigationObserver blank_observer(web_contents());
+  ASSERT_TRUE(
+      content::ExecJs(web_contents(), "window.location = 'about:blank';"));
+  blank_observer.Wait();
+  ASSERT_EQ(web_contents()->GetLastCommittedURL(), GURL(url::kAboutBlankURL));
+  ASSERT_EQ(web_contents()->GetPrimaryMainFrame()->GetLastCommittedOrigin(),
+            url::Origin::Create(sensitive_url));
+  EXPECT_EQ(blank_observer.last_initiator_origin(),
+            url::Origin::Create(sensitive_url));
+
+  OpenGlicAndCreateTask();
+
+  RunTestSequence(CreateMockWebClientRequest(
+      content::JsReplace(kHandleUserConfirmationDialogTempl, false)));
+
+  WaitTool::SetNoDelayForTesting();
+  std::unique_ptr<ToolRequest> tool_request = MakeWaitRequest(active_tab());
+  ASSERT_TRUE(tool_request->RequiresUrlCheckInCurrentTab());
+
+  ActResultFuture result;
+  actor_task().Act(ToRequestList(tool_request), result.GetCallback());
+  ExpectErrorResult(result, mojom::ActionResultCode::kUrlBlocked);
+
+  RunTestSequence(VerifyUserConfirmationDialogRequest(
+      base::test::ParseJsonDict(content::JsReplace(
+          R"({"navigationOrigin": $1, "forBlocklistedOrigin": true})",
+          url::Origin::Create(sensitive_url)))));
+}
+
 }  // namespace actor

diff --git a/chrome/browser/actor/execution_engine_origin_gating_browsertest.cc b/chrome/browser/actor/execution_engine_origin_gating_browsertest.cc
index ecb0bd4d..bc33d2b1 100644
--- a/chrome/browser/actor/execution_engine_origin_gating_browsertest.cc
+++ b/chrome/browser/actor/execution_engine_origin_gating_browsertest.cc
@@ -3162,4 +3162,118 @@
           url::Origin::Create(sensitive_url)))));
 }
 
+// Regression test for crbug.com/563004214: a cross-origin opener navigating an
+// acted-on tab to about:blank (which commits without a URL loader) must be
+// gated by ActorNavigationThrottle and must not poison the tab's committed
+// origin to bypass new-origin confirmation on subsequent navigations.
+IN_PROC_BROWSER_TEST_F(ExecutionEngineOriginGatingBrowserTest,
+                       OpenerCrossOriginAboutBlankNavigationGated) {
+  const GURL evil_url =
+      embedded_https_test_server().GetURL("foo.com", "/actor/blank.html");
+  const GURL start_url =
+      embedded_https_test_server().GetURL("example.com", "/actor/link.html");
+
+  content::WebContents* acted_contents = web_contents();
+  ASSERT_TRUE(content::NavigateToURL(acted_contents, start_url));
+
+  // Open a popup on foo.com before starting the actor task so foo.com holds a
+  // `window.opener` reference to `acted_contents`.
+  content::TestNavigationObserver popup_observer(evil_url);
+  popup_observer.StartWatchingNewWebContents();
+  ASSERT_TRUE(content::ExecJs(
+      acted_contents, content::JsReplace("window.open($1);", evil_url)));
+  popup_observer.Wait();
+  content::WebContents* popup_contents = web_contents();
+  ASSERT_NE(popup_contents, acted_contents);
+
+  // Reactivate tab 0 (`acted_contents`) before opening Glic and starting the
+  // actor task on it.
+  browser()->tab_strip_model()->ActivateTabAt(0);
+  ASSERT_EQ(web_contents(), acted_contents);
+
+  OpenGlicAndCreateTask();
+  actor_task().AddTab(active_tab()->GetHandle(), /*stop_task_on_detach=*/true,
+                      base::DoNothing());
+  ASSERT_TRUE(actor_task().HasTab(active_tab()->GetHandle()));
+
+  // Deny any navigation confirmation requests.
+  RunTestSequence(CreateMockWebClientRequest(
+      content::JsReplace(kHandleNavigationConfirmationTempl, false)));
+
+  // Step 1: Cross-origin popup attempts to navigate the acted-on tab to
+  // about:blank via window.opener. This must be gated against foo.com and
+  // denied.
+  content::TestNavigationObserver about_blank_observer(
+      acted_contents, content::MessageLoopRunner::QuitMode::IMMEDIATE,
+      /*ignore_uncommitted_navigations=*/false);
+  EXPECT_TRUE(content::ExecJs(popup_contents,
+                              "window.opener.location = 'about:blank';"));
+  about_blank_observer.Wait();
+
+  EXPECT_FALSE(about_blank_observer.last_navigation_succeeded());
+  EXPECT_EQ(about_blank_observer.last_initiator_origin(),
+            url::Origin::Create(evil_url));
+  EXPECT_EQ(acted_contents->GetLastCommittedURL(), start_url);
+  EXPECT_EQ(acted_contents->GetPrimaryMainFrame()->GetLastCommittedOrigin(),
+            url::Origin::Create(start_url));
+
+  RunTestSequence(VerifyNavigationConfirmationRequest(
+      base::test::ParseJsonDict(content::JsReplace(
+          R"({"navigationOrigin": $1, "taskId": $2})",
+          url::Origin::Create(evil_url), actor_task().id().value()))));
+
+  // Step 2: Same-origin navigation to about:blank initiated by the acted-on tab
+  // itself must still be allowed via kAllowSameOrigin without prompting.
+  content::TestNavigationObserver same_origin_blank_observer(acted_contents);
+  EXPECT_TRUE(
+      content::ExecJs(acted_contents, "window.location = 'about:blank';"));
+  same_origin_blank_observer.Wait();
+  EXPECT_TRUE(same_origin_blank_observer.last_navigation_succeeded());
+  EXPECT_EQ(same_origin_blank_observer.last_initiator_origin(),
+            url::Origin::Create(start_url));
+  EXPECT_EQ(acted_contents->GetLastCommittedURL(), GURL(url::kAboutBlankURL));
+  EXPECT_EQ(acted_contents->GetPrimaryMainFrame()->GetLastCommittedOrigin(),
+            url::Origin::Create(start_url));
+}
+
+// Regression test for crbug.com/563004214: if an acted-on tab is at about:blank
+// with an inherited sensitive origin, page action safety checks must evaluate
+// the effective origin rather than auto-allowing via kAllowAboutBlank.
+IN_PROC_BROWSER_TEST_F(ExecutionEngineHardcodedSensitiveSiteBrowserTest,
+                       PageActionOnInheritedAboutBlankPromptsUser) {
+  const GURL sensitive_url =
+      embedded_https_test_server().GetURL("bar.com", "/actor/blank.html");
+  ASSERT_TRUE(content::NavigateToURL(web_contents(), sensitive_url));
+
+  // Navigate from bar.com to about:blank so GetLastCommittedURL() is
+  // about:blank while GetLastCommittedOrigin() is https://bar.com.
+  content::TestNavigationObserver blank_observer(web_contents());
+  ASSERT_TRUE(
+      content::ExecJs(web_contents(), "window.location = 'about:blank';"));
+  blank_observer.Wait();
+  ASSERT_EQ(web_contents()->GetLastCommittedURL(), GURL(url::kAboutBlankURL));
+  ASSERT_EQ(web_contents()->GetPrimaryMainFrame()->GetLastCommittedOrigin(),
+            url::Origin::Create(sensitive_url));
+  EXPECT_EQ(blank_observer.last_initiator_origin(),
+            url::Origin::Create(sensitive_url));
+
+  OpenGlicAndCreateTask();
+
+  RunTestSequence(CreateMockWebClientRequest(
+      content::JsReplace(kHandleUserConfirmationDialogTempl, false)));
+
+  WaitTool::SetNoDelayForTesting();
+  std::unique_ptr<ToolRequest> tool_request = MakeWaitRequest(active_tab());
+  ASSERT_TRUE(tool_request->RequiresUrlCheckInCurrentTab());
+
+  ActResultFuture result;
+  actor_task().Act(ToRequestList(tool_request), result.GetCallback());
+  ExpectErrorResult(result, mojom::ActionResultCode::kUrlBlocked);
+
+  RunTestSequence(VerifyUserConfirmationDialogRequest(
+      base::test::ParseJsonDict(content::JsReplace(
+          R"({"navigationOrigin": $1, "forBlocklistedOrigin": true})",
+          url::Origin::Create(sensitive_url)))));
+}
+
 }  // namespace actor