From 0db5072577f34c19aedb711734737d8e6540899b Mon Sep 17 00:00:00 2001
From: Zainab Rizvi <rizvis@google.com>
Date: Thu, 06 Aug 2026 04:16:13 -0700
Subject: [PATCH] PlzWorker: Check that worker final response URL is committable

Verify that the worker's renderer process is permitted to commit the
main script's final response URL before adopting it in
DedicatedWorkerHost and SharedWorkerServiceImpl. If the URL cannot be
committed, reject the worker script load.

Also grant commit access when the URL is same-origin with the worker's
storage key origin (e.g. for isolated-app://).

Add browser tests verifying that non-committable response URLs are
rejected for dedicated and shared workers.

Bug: 497635917

TAG=agy
CONV=18d74d7e-1f1b-4b21-a38d-52218d65f900

Change-Id: I104e6d94f998c94de2c3ba5de568416ebd62098b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8207261
Commit-Queue: Zainab Rizvi <rizvis@google.com>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1674858}
---

diff --git a/content/browser/file_system/file_system_url_loader_factory.h b/content/browser/file_system/file_system_url_loader_factory.h
index a820d61..b28d4a7 100644
--- a/content/browser/file_system/file_system_url_loader_factory.h
+++ b/content/browser/file_system/file_system_url_loader_factory.h
@@ -36,9 +36,8 @@
 // - For a factory created to pass to the renderer for subresource requests from
 //   the frame: that renderer process's ID.
 // - For a factory created for a browser-initiated worker main script request:
-//   the ID of the process the worker will run in.
-//   TODO(crbug.com/41471904): We should specify kInvalidUniqueID for this
-//   worker main script case like the browser-initiated navigation case.
+//   ChildProcessHost::kInvalidUniqueID (permissions are checked later via
+//   CanCommitURL in PlzWorker).
 // - For a factory created to pass to the renderer for subresource requests from
 //   the worker: that renderer process's ID.
 //
diff --git a/content/browser/worker_host/dedicated_worker_host.cc b/content/browser/worker_host/dedicated_worker_host.cc
index 8e448c3..908964a 100644
--- a/content/browser/worker_host/dedicated_worker_host.cc
+++ b/content/browser/worker_host/dedicated_worker_host.cc
@@ -29,6 +29,7 @@
 #include "content/browser/renderer_host/frame_tree_node.h"
 #include "content/browser/renderer_host/local_network_access_util.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
+#include "content/browser/security/cpsp/child_process_security_policy_impl.h"
 #include "content/browser/service_worker/service_worker_client.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_main_resource_handle.h"
@@ -454,8 +455,25 @@
     return;
   }
 
-  // TODO(crbug.com/41471904): Check if the main script's final response
-  // URL is committable.
+  // The final response URL is derived from data that may have been supplied by
+  // a renderer (e.g., via the URL list of a service worker provided response),
+  // so make sure the worker process is allowed to commit it before adopting it
+  // as this worker's URL.
+  //
+  // Only grant commit permissions if the URL is same-origin with the worker's
+  // expected origin (e.g. for Isolated Web Apps or extensions).
+  if (url::Origin::Create(result->final_response_url)
+          .IsSameOriginWith(worker_storage_key_.origin())) {
+    ChildProcessSecurityPolicyImpl::GetInstance()->GrantCommitURL(
+        worker_process_host_->GetDeprecatedID(), result->final_response_url);
+  }
+  if (!ChildProcessSecurityPolicyImpl::GetInstance()->CanCommitURL(
+          worker_process_host_->GetDeprecatedID(),
+          result->final_response_url)) {
+    ScriptLoadStartFailed(network::URLLoaderCompletionStatus(net::ERR_ABORTED));
+    return;
+  }
+
   final_response_url_ = result->final_response_url;
   service_->NotifyWorkerFinalResponseURLDetermined(token_,
                                                    result->final_response_url);
diff --git a/content/browser/worker_host/shared_worker_service_impl.cc b/content/browser/worker_host/shared_worker_service_impl.cc
index 9973fa8..7b3ffff 100644
--- a/content/browser/worker_host/shared_worker_service_impl.cc
+++ b/content/browser/worker_host/shared_worker_service_impl.cc
@@ -21,6 +21,7 @@
 #include "base/timer/elapsed_timer.h"
 #include "content/browser/devtools/shared_worker_devtools_agent_host.h"
 #include "content/browser/loader/file_url_loader_factory.h"
+#include "content/browser/security/cpsp/child_process_security_policy_impl.h"
 #include "content/browser/service_worker/service_worker_client.h"
 #include "content/browser/service_worker/service_worker_main_resource_handle.h"
 #include "content/browser/storage_partition_impl.h"
@@ -591,8 +592,24 @@
     return;
   }
 
-  // TODO(crbug.com/41471904): Check if the main script's final response
-  // URL is committable.
+  // The final response URL is derived from data that may have been supplied by
+  // a renderer (e.g., via the URL list of a service worker provided response),
+  // so make sure the worker process is allowed to commit it before adopting it
+  // as this worker's URL.
+  //
+  // Only grant commit permissions if the URL is same-origin with the worker's
+  // expected origin (e.g. for Isolated Web Apps or extensions).
+  if (url::Origin::Create(result->final_response_url)
+          .IsSameOriginWith(host->instance().worker_storage_key().origin())) {
+    ChildProcessSecurityPolicyImpl::GetInstance()->GrantCommitURL(
+        host->GetProcessHost()->GetDeprecatedID(), result->final_response_url);
+  }
+  if (!ChildProcessSecurityPolicyImpl::GetInstance()->CanCommitURL(
+          host->GetProcessHost()->GetDeprecatedID(),
+          result->final_response_url)) {
+    DestroyHost(host.get());
+    return;
+  }
 
   // Get the factory used to instantiate the new shared worker instance in
   // the target process.
diff --git a/content/browser/worker_host/worker_browsertest.cc b/content/browser/worker_host/worker_browsertest.cc
index 9165709..69ab4b7 100644
--- a/content/browser/worker_host/worker_browsertest.cc
+++ b/content/browser/worker_host/worker_browsertest.cc
@@ -2,11 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/byte_size.h"
 #include "base/check.h"
 #include "base/feature_list.h"
 #include "base/functional/bind.h"
 #include "base/functional/callback.h"
 #include "base/path_service.h"
+#include "base/scoped_observation.h"
 #include "base/strings/escape.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_util.h"
@@ -20,12 +22,15 @@
 #include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
 #include "content/browser/process_lock.h"
+#include "content/browser/security/cpsp/child_process_security_policy_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/browser/worker_host/shared_worker_service_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/client_certificate_delegate.h"
+#include "content/public/browser/dedicated_worker_service.h"
+#include "content/public/browser/shared_worker_service.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_features.h"
@@ -46,6 +51,7 @@
 #include "net/cookies/canonical_cookie.h"
 #include "net/cookies/cookie_access_result.h"
 #include "net/dns/mock_host_resolver.h"
+#include "net/http/http_response_headers.h"
 #include "net/ssl/client_cert_identity.h"
 #include "net/ssl/ssl_server_config.h"
 #include "net/test/embedded_test_server/connection_tracker.h"
@@ -59,6 +65,8 @@
 #include "services/network/public/mojom/connection_change_observer_client.mojom.h"
 #include "services/network/public/mojom/cookie_manager.mojom.h"
 #include "services/network/public/mojom/network_context.mojom.h"
+#include "services/network/public/mojom/parsed_headers.mojom.h"
+#include "services/network/public/mojom/url_response_head.mojom.h"
 #include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/common/storage_key/storage_key.h"
 #include "url/gurl.h"
@@ -78,6 +86,90 @@
   return base::FeatureList::IsEnabled(blink::features::kSharedWorker);
 }
 
+// Writes a worker script response with the given `url_list` populated in the
+// service worker URL list of the response head.
+void WriteWorkerScriptResponseWithServiceWorkerUrlList(
+    network::mojom::URLLoaderClient* client,
+    const std::vector<GURL>& url_list) {
+  static constexpr char kBody[] = "postMessage('done');";
+  auto response = network::mojom::URLResponseHead::New();
+  response->headers = base::MakeRefCounted<net::HttpResponseHeaders>(
+      "HTTP/1.1 200 OK\nContent-Type: text/javascript\n\n");
+  response->mime_type = "text/javascript";
+  response->was_fetched_via_service_worker = true;
+  response->url_list_via_service_worker = url_list;
+  response->parsed_headers = network::mojom::ParsedHeaders::New();
+
+  mojo::ScopedDataPipeProducerHandle producer_handle;
+  mojo::ScopedDataPipeConsumerHandle consumer_handle;
+  EXPECT_EQ(mojo::CreateDataPipe(nullptr, producer_handle, consumer_handle),
+            MOJO_RESULT_OK);
+  if (producer_handle.is_valid()) {
+    EXPECT_EQ(
+        producer_handle->WriteAllData(base::byte_span_from_cstring(kBody)),
+        MOJO_RESULT_OK);

diff --git a/content/browser/worker_host/worker_browsertest.cc b/content/browser/worker_host/worker_browsertest.cc
index 9165709..69ab4b7 100644
--- a/content/browser/worker_host/worker_browsertest.cc
+++ b/content/browser/worker_host/worker_browsertest.cc
@@ -2,11 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/byte_size.h"
 #include "base/check.h"
 #include "base/feature_list.h"
 #include "base/functional/bind.h"
 #include "base/functional/callback.h"
 #include "base/path_service.h"
+#include "base/scoped_observation.h"
 #include "base/strings/escape.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_util.h"
@@ -20,12 +22,15 @@
 #include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
 #include "content/browser/process_lock.h"
+#include "content/browser/security/cpsp/child_process_security_policy_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/browser/worker_host/shared_worker_service_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/client_certificate_delegate.h"
+#include "content/public/browser/dedicated_worker_service.h"
+#include "content/public/browser/shared_worker_service.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_features.h"
@@ -46,6 +51,7 @@
 #include "net/cookies/canonical_cookie.h"
 #include "net/cookies/cookie_access_result.h"
 #include "net/dns/mock_host_resolver.h"
+#include "net/http/http_response_headers.h"
 #include "net/ssl/client_cert_identity.h"
 #include "net/ssl/ssl_server_config.h"
 #include "net/test/embedded_test_server/connection_tracker.h"
@@ -59,6 +65,8 @@
 #include "services/network/public/mojom/connection_change_observer_client.mojom.h"
 #include "services/network/public/mojom/cookie_manager.mojom.h"
 #include "services/network/public/mojom/network_context.mojom.h"
+#include "services/network/public/mojom/parsed_headers.mojom.h"
+#include "services/network/public/mojom/url_response_head.mojom.h"
 #include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/common/storage_key/storage_key.h"
 #include "url/gurl.h"
@@ -78,6 +86,90 @@
   return base::FeatureList::IsEnabled(blink::features::kSharedWorker);
 }
 
+// Writes a worker script response with the given `url_list` populated in the
+// service worker URL list of the response head.
+void WriteWorkerScriptResponseWithServiceWorkerUrlList(
+    network::mojom::URLLoaderClient* client,
+    const std::vector<GURL>& url_list) {
+  static constexpr char kBody[] = "postMessage('done');";
+  auto response = network::mojom::URLResponseHead::New();
+  response->headers = base::MakeRefCounted<net::HttpResponseHeaders>(
+      "HTTP/1.1 200 OK\nContent-Type: text/javascript\n\n");
+  response->mime_type = "text/javascript";
+  response->was_fetched_via_service_worker = true;
+  response->url_list_via_service_worker = url_list;
+  response->parsed_headers = network::mojom::ParsedHeaders::New();
+
+  mojo::ScopedDataPipeProducerHandle producer_handle;
+  mojo::ScopedDataPipeConsumerHandle consumer_handle;
+  EXPECT_EQ(mojo::CreateDataPipe(nullptr, producer_handle, consumer_handle),
+            MOJO_RESULT_OK);
+  if (producer_handle.is_valid()) {
+    EXPECT_EQ(
+        producer_handle->WriteAllData(base::byte_span_from_cstring(kBody)),
+        MOJO_RESULT_OK);
+  }
+  producer_handle.reset();
+
+  client->OnReceiveResponse(std::move(response), std::move(consumer_handle),
+                            std::nullopt);
+  network::URLLoaderCompletionStatus status;
+  status.error_code = net::OK;
+  status.decoded_body_length = base::ByteSize(sizeof(kBody) - 1);
+  client->OnComplete(status);
+}
+
+// Records final response URLs reported for newly created workers.
+class WorkerFinalResponseURLObserver : public DedicatedWorkerService::Observer,
+                                       public SharedWorkerService::Observer {
+ public:
+  explicit WorkerFinalResponseURLObserver(StoragePartition* storage_partition) {
+    dedicated_worker_observation_.Observe(
+        storage_partition->GetDedicatedWorkerService());
+    shared_worker_observation_.Observe(
+        storage_partition->GetSharedWorkerService());
+  }
+
+  const std::vector<GURL>& final_response_urls() const {
+    return final_response_urls_;
+  }
+
+  // DedicatedWorkerService::Observer:
+  void OnWorkerCreated(const blink::DedicatedWorkerToken&,
+                       ChildProcessId,
+                       const url::Origin&,
+                       DedicatedWorkerCreator) override {}
+  void OnBeforeWorkerDestroyed(const blink::DedicatedWorkerToken&,
+                               DedicatedWorkerCreator) override {}
+  void OnFinalResponseURLDetermined(const blink::DedicatedWorkerToken&,
+                                    const GURL& url) override {
+    final_response_urls_.push_back(url);
+  }
+
+  // SharedWorkerService::Observer:
+  void OnWorkerCreated(const blink::SharedWorkerToken&,
+                       ChildProcessId,
+                       const url::Origin&,
+                       const base::UnguessableToken&) override {}
+  void OnBeforeWorkerDestroyed(const blink::SharedWorkerToken&) override {}
+  void OnFinalResponseURLDetermined(const blink::SharedWorkerToken&,
+                                    const GURL& url) override {
+    final_response_urls_.push_back(url);
+  }
+  void OnClientAdded(const blink::SharedWorkerToken&,
+                     GlobalRenderFrameHostId) override {}
+  void OnClientRemoved(const blink::SharedWorkerToken&,
+                       GlobalRenderFrameHostId) override {}
+
+ private:
+  std::vector<GURL> final_response_urls_;
+  base::ScopedObservation<DedicatedWorkerService,
+                          DedicatedWorkerService::Observer>
+      dedicated_worker_observation_{this};
+  base::ScopedObservation<SharedWorkerService, SharedWorkerService::Observer>
+      shared_worker_observation_{this};
+};
+
 }  // namespace
 
 class WorkerTest : public ContentBrowserTest {
@@ -314,6 +406,100 @@
   RunTest(url, /*expect_failure=*/true);
 }
 
+// Tests that a dedicated worker main script response whose service worker
+// supplied URL list resolves to a URL that the worker process cannot commit
+// is rejected.
+IN_PROC_BROWSER_TEST_F(WorkerTest,
+                       DedicatedWorkerRejectsNonCommittableFinalResponseUrl) {
+  const GURL main_url = ssl_server()->GetURL("a.test", "/title1.html");
+  const GURL worker_url = ssl_server()->GetURL("a.test", "/workers/worker.js");
+  const GURL non_committable_url("file:///non_committable_path");
+
+  WorkerFinalResponseURLObserver observer(shell()
+                                              ->web_contents()
+                                              ->GetBrowserContext()
+                                              ->GetDefaultStoragePartition());
+
+  URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
+      [&](URLLoaderInterceptor::RequestParams* params) {
+        if (params->url_request.url != worker_url) {
+          return false;
+        }
+        WriteWorkerScriptResponseWithServiceWorkerUrlList(
+            params->client.get(), {non_committable_url});
+        return true;
+      }));
+
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+  EXPECT_FALSE(ChildProcessSecurityPolicyImpl::GetInstance()->CanCommitURL(
+      shell()
+          ->web_contents()
+          ->GetPrimaryMainFrame()
+          ->GetProcess()
+          ->GetDeprecatedID(),
+      non_committable_url));
+
+  EXPECT_EQ("error", EvalJs(shell(), R"(
+    new Promise(resolve => {
+      const worker = new Worker('/workers/worker.js');
+      worker.onerror = () => resolve('error');
+      worker.onmessage = e => resolve(e.data);
+    })
+  )"));
+
+  // The worker process is not allowed to commit `non_committable_url`, so it
+  // must not have been adopted as the worker's final response URL.
+  EXPECT_TRUE(observer.final_response_urls().empty())
+      << "unexpected final response URL: "
+      << observer.final_response_urls().front();
+}
+
+// Same as DedicatedWorkerRejectsNonCommittableFinalResponseUrl, but for shared
+// workers.
+IN_PROC_BROWSER_TEST_F(WorkerTest,
+                       SharedWorkerRejectsNonCommittableFinalResponseUrl) {
+  if (!SupportsSharedWorker()) {
+    return;
+  }
+
+  const GURL main_url = ssl_server()->GetURL("a.test", "/title1.html");
+  const GURL worker_url = ssl_server()->GetURL("a.test", "/workers/worker.js");
+  const GURL non_committable_url("file:///non_committable_path");
+
+  WorkerFinalResponseURLObserver observer(shell()
+                                              ->web_contents()
+                                              ->GetBrowserContext()
+                                              ->GetDefaultStoragePartition());
+
+  URLLoaderInterceptor interceptor(base::BindLambdaForTesting(
+      [&](URLLoaderInterceptor::RequestParams* params) {
+        if (params->url_request.url != worker_url) {
+          return false;
+        }
+        WriteWorkerScriptResponseWithServiceWorkerUrlList(
+            params->client.get(), {non_committable_url});
+        return true;
+      }));
+
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+  EXPECT_EQ("error", EvalJs(shell(), R"(
+    new Promise(resolve => {
+      const worker = new SharedWorker('/workers/worker.js');
+      worker.onerror = () => resolve('error');
+      worker.port.onmessage = e => resolve(e.data);
+    })
+  )"));
+
+  // The worker process is not allowed to commit `non_committable_url`, so it
+  // must not have been adopted as the worker's final response URL.
+  EXPECT_TRUE(observer.final_response_urls().empty())
+      << "unexpected final response URL: "
+      << observer.final_response_urls().front();
+  EXPECT_FALSE(GetSharedWorkerHost(worker_url));
+}
+
 IN_PROC_BROWSER_TEST_F(WorkerTest, MultipleWorkers) {
   RunTest(GetTestURL("multi_worker.html", std::string()));
 }