From 49e0c203cdaaaf4565fe7f323a66e3469d856017 Mon Sep 17 00:00:00 2001
From: Owen Min <[email protected]>
Date: Fri, 05 Jun 2026 09:48:34 -0700
Subject: [PATCH] Downgrade URLAllowlist wildcard to neutral state.

Introduces a feature flag `kDowngradeURLAllowlistWildcardToNeutral`
(enabled by default) to modify the behavior of URLBlocklistManager. When
enabled, if the highest priority match is an allow rule using the
wildcard "*", the URLBlocklistState will be `URL_NEUTRAL_STATE` instead
of `URL_IN_ALLOWLIST`.

URLAllowlist is designed to be exception of URLBlocklist. Setting it to
* means all URLs will be allowed which is meaningless as it's the
default behavior without policy.

However, it will create unintentional side effects for features that
check if a URL is specifically allowed. Those checks may skip some
protection mechanism for those URLs. Through blindly do so for all URLs
introduce unnecessary risk.

Downgrade to neutral state will still allow all URLs but keep protection
mechanism.

Note that a switch is provided allow caller (e.g. OnTaskBlocklist) use
old behavior when override blocklist source.


Bug: 513611659
Change-Id: Ic308f77c6541bd2e825efb5746498642e18c0555
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7886057
Reviewed-by: Igor <[email protected]>
Reviewed-by: April Zhou <[email protected]>
Reviewed-by: Michał Kaczmarczyk <[email protected]>
Commit-Queue: Owen Min <[email protected]>
Reviewed-by: Mikołaj Wałachowski <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1642399}
---

diff --git a/chromeos/ash/components/boca/on_task/on_task_blocklist.cc b/chromeos/ash/components/boca/on_task/on_task_blocklist.cc
index 0e2972d1..cddd1f54 100644
--- a/chromeos/ash/components/boca/on_task/on_task_blocklist.cc
+++ b/chromeos/ash/components/boca/on_task/on_task_blocklist.cc
@@ -353,3 +353,8 @@
 OnTaskBlocklist::OnTaskBlocklistSource::GetAllowlistSpec() const {
   return &allowlist_;
 }
+
+bool OnTaskBlocklist::OnTaskBlocklistSource::
+    DowngradeAllowlistWildcardToNeutral() const {
+  return false;
+}
diff --git a/chromeos/ash/components/boca/on_task/on_task_blocklist.h b/chromeos/ash/components/boca/on_task/on_task_blocklist.h
index bba05911..6c72778 100644
--- a/chromeos/ash/components/boca/on_task/on_task_blocklist.h
+++ b/chromeos/ash/components/boca/on_task/on_task_blocklist.h
@@ -36,6 +36,7 @@
 
     const base::ListValue* GetBlocklistSpec() const override;
     const base::ListValue* GetAllowlistSpec() const override;
+    bool DowngradeAllowlistWildcardToNeutral() const override;
     void SetBlocklistObserver(base::RepeatingClosure observer) override {}
 
    private:
diff --git a/components/policy/core/browser/url_list/policy_blocklist_service.cc b/components/policy/core/browser/url_list/policy_blocklist_service.cc
index 2cde25f3..85f40f2 100644
--- a/components/policy/core/browser/url_list/policy_blocklist_service.cc
+++ b/components/policy/core/browser/url_list/policy_blocklist_service.cc
@@ -56,6 +56,8 @@
         policy::policy_prefs::kAlwaysOnVpnPreConnectUrlAllowlist, observer);
   }
 
+  bool DowngradeAllowlistWildcardToNeutral() const override { return true; }
+
  private:
   const base::ListValue blocklist_;
   PrefChangeRegistrar pref_change_registrar_;
diff --git a/components/policy/core/browser/url_list/url_blocklist_manager.cc b/components/policy/core/browser/url_list/url_blocklist_manager.cc
index 6cc1e34..c5d9d34 100644
--- a/components/policy/core/browser/url_list/url_blocklist_manager.cc
+++ b/components/policy/core/browser/url_list/url_blocklist_manager.cc
@@ -13,6 +13,7 @@
 #include <utility>
 
 #include "base/check.h"
+#include "base/feature_list.h"
 #include "base/files/file_path.h"
 #include "base/functional/bind.h"
 #include "base/location.h"
@@ -79,8 +80,10 @@
 
 // Returns a blocklist based on the given |block| and |allow| pattern lists.
 std::unique_ptr<URLBlocklist> BuildBlocklist(const base::ListValue* block,
-                                             const base::ListValue* allow) {
+                                             const base::ListValue* allow,
+                                             bool downgrade = true) {
   auto blocklist = std::make_unique<URLBlocklist>();
+  blocklist->SetDowngradeAllowlistWildcardToNeutral(downgrade);
   if (block) {
     blocklist->Block(*block);
   }
@@ -140,6 +143,10 @@
   return !filter.allow && filter.IsWildcard();
 }
 
+bool IsWildcardAllowlist(const FilterComponents& filter) {
+  return filter.allow && filter.IsWildcard();
+}
+
 // Determines if the left-hand side `lhs` filter takes precedence over the
 // right-hand side `rhs` filter. Returns true if `lhs` takes precedence over
 // `rhs`, false otherwise.
@@ -227,12 +234,18 @@
     }
   }
 
+  bool DowngradeAllowlistWildcardToNeutral() const override { return true; }
+
  private:
   std::optional<std::string> blocklist_pref_path_;
   std::optional<std::string> allowlist_pref_path_;
   PrefChangeRegistrar pref_change_registrar_;
 };
 
+bool BlocklistSource::DowngradeAllowlistWildcardToNeutral() const {
+  return true;
+}
+
 URLBlocklist::URLBlocklist() : url_matcher_(new URLMatcher) {}
 
 URLBlocklist::~URLBlocklist() = default;
@@ -263,6 +276,13 @@
     return URLBlocklist::URLBlocklistState::URL_NEUTRAL_STATE;
   }
 
+  if (base::FeatureList::IsEnabled(
+          features::kDowngradeURLAllowlistWildcardToNeutral) &&
+      downgrade_allowlist_wildcard_to_neutral_ &&
+      IsWildcardAllowlist(*highest_priority_filter)) {
+    return URLBlocklist::URLBlocklistState::URL_NEUTRAL_STATE;
+  }
+
   // Some of the internal Chrome URLs are not affected by the "*" in the
   // blocklist. Note that the "*" is the lowest priority filter possible, so
   // any higher priority filter will be applied first.
@@ -276,6 +296,10 @@
              : URLBlocklist::URLBlocklistState::URL_IN_BLOCKLIST;
 }
 
+void URLBlocklist::SetDowngradeAllowlistWildcardToNeutral(bool downgrade) {
+  downgrade_allowlist_wildcard_to_neutral_ = downgrade;
+}
+
 const FilterComponents* URLBlocklist::GetHighestPriorityFilterFor(
     const GURL& url) const {
   const FilterComponents* highest_priority_filter = nullptr;
@@ -343,6 +367,8 @@
                                               ? override_blocklist_source_.get()
                                               : default_blocklist_source_.get();
 
+  bool downgrade = current_source->DowngradeAllowlistWildcardToNeutral();
+
   const base::ListValue* block = current_source->GetBlocklistSpec();
   const base::ListValue* allow = current_source->GetAllowlistSpec();
 
@@ -353,7 +379,8 @@
           base::Owned(block ? std::make_unique<base::ListValue>(block->Clone())
                             : nullptr),
           base::Owned(allow ? std::make_unique<base::ListValue>(allow->Clone())
-                            : nullptr)),
+                            : nullptr),
+          downgrade),
       base::BindOnce(&URLBlocklistManager::SetBlocklist,
                      ui_weak_ptr_factory_.GetWeakPtr()));
 }
diff --git a/components/policy/core/browser/url_list/url_blocklist_manager.h b/components/policy/core/browser/url_list/url_blocklist_manager.h
index 0bd2c0c..656da0f 100644
--- a/components/policy/core/browser/url_list/url_blocklist_manager.h
+++ b/components/policy/core/browser/url_list/url_blocklist_manager.h
@@ -67,12 +67,15 @@
 
   URLBlocklistState GetURLBlocklistState(const GURL& url) const;
 
+  void SetDowngradeAllowlistWildcardToNeutral(bool downgrade);
+
  private:
   // Returns the highest priority filter in `filters_` matching the given URL,
   // or nullptr if none found.
   const url_matcher::util::FilterComponents* GetHighestPriorityFilterFor(
       const GURL& url) const;
 
+  bool downgrade_allowlist_wildcard_to_neutral_ = true;
   base::MatcherStringPattern::ID id_ = 0;
   std::map<base::MatcherStringPattern::ID, url_matcher::util::FilterComponents>
       filters_;
@@ -92,6 +95,15 @@
   // Returns exceptions to the blocklist.
   virtual const base::ListValue* GetAllowlistSpec() const = 0;
 
+  // Returns true if matching level of allowlist wildcard '*' is downgraded to
+  // neutral.

diff --git a/components/policy/core/browser/url_list/url_blocklist_manager_unittest.cc b/components/policy/core/browser/url_list/url_blocklist_manager_unittest.cc
index 143a410..45797cf4 100644
--- a/components/policy/core/browser/url_list/url_blocklist_manager_unittest.cc
+++ b/components/policy/core/browser/url_list/url_blocklist_manager_unittest.cc
@@ -764,6 +764,68 @@
                                           "https://*", "http://example.com"));
 }
 
+TEST_F(URLBlocklistManagerTest, DowngradeURLAllowlistWildcardToNeutralEnabled) {
+  using State = URLBlocklist::URLBlocklistState;
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      features::kDowngradeURLAllowlistWildcardToNeutral);
+
+  // Wildcard allowlist alone returns neutral.
+  EXPECT_EQ(State::URL_NEUTRAL_STATE,
+            GetUrlBlocklistStateAfterAllowing("*", "http://example.com"));
+
+  // When both "*" and "example.com" are in the allowlist.
+  URLBlocklist blocklist;
+  base::ListValue allowed;
+  allowed.Append("*");
+  allowed.Append("example.com");
+  blocklist.Allow(allowed);
+
+  // "google.com" only matches "*" and is downgraded to neutral.
+  EXPECT_EQ(State::URL_NEUTRAL_STATE,
+            blocklist.GetURLBlocklistState(GURL("http://google.com")));
+
+  // "example.com" matches the more specific filter and returns allowed.
+  EXPECT_EQ(State::URL_IN_ALLOWLIST,
+            blocklist.GetURLBlocklistState(GURL("http://example.com")));
+
+  // Explicitly disable downgrading.
+  blocklist.SetDowngradeAllowlistWildcardToNeutral(false);
+  EXPECT_EQ(State::URL_IN_ALLOWLIST,
+            blocklist.GetURLBlocklistState(GURL("http://google.com")));
+  EXPECT_EQ(State::URL_IN_ALLOWLIST,
+            blocklist.GetURLBlocklistState(GURL("http://example.com")));
+}
+
+TEST_F(URLBlocklistManagerTest,
+       DowngradeURLAllowlistWildcardToNeutralDisabled) {
+  using State = URLBlocklist::URLBlocklistState;
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndDisableFeature(
+      features::kDowngradeURLAllowlistWildcardToNeutral);
+
+  // When both "*" and "example.com" are in the allowlist.
+  URLBlocklist blocklist;
+
+  base::ListValue allowed;
+  allowed.Append("*");
+  allowed.Append("example.com");
+  blocklist.Allow(allowed);
+
+  // When disabled, both match and return allowed.
+  EXPECT_EQ(State::URL_IN_ALLOWLIST,
+            blocklist.GetURLBlocklistState(GURL("http://google.com")));
+  EXPECT_EQ(State::URL_IN_ALLOWLIST,
+            blocklist.GetURLBlocklistState(GURL("http://example.com")));
+
+  // Explicitly disable downgrading.
+  blocklist.SetDowngradeAllowlistWildcardToNeutral(false);
+  EXPECT_EQ(State::URL_IN_ALLOWLIST,
+            blocklist.GetURLBlocklistState(GURL("http://google.com")));
+  EXPECT_EQ(State::URL_IN_ALLOWLIST,
+            blocklist.GetURLBlocklistState(GURL("http://example.com")));
+}
+
 #if BUILDFLAG(IS_CHROMEOS)
 // Custom BlocklistSource implementation.
 // Custom BlocklistSource implementation.
@@ -782,6 +844,10 @@
     return &allowlist_;
   }
 
+  bool DowngradeAllowlistWildcardToNeutral() const override {
+    return downgrade_allowlist_wildcard_to_neutral_;
+  }
+
   void SetBlocklistObserver(base::RepeatingClosure observer) override {
     blocklist_observer_ = std::move(observer);
   }
@@ -796,6 +862,11 @@
     TriggerObserver();
   }
 
+  void SetDowngradeAllowlistWildcardToNeutral(bool downgrade) {
+    downgrade_allowlist_wildcard_to_neutral_ = downgrade;
+    TriggerObserver();
+  }
+
  private:
   void TriggerObserver() {
     if (!blocklist_observer_) {
@@ -807,6 +878,7 @@
   base::ListValue blocklist_;
   base::ListValue allowlist_;
   base::RepeatingClosure blocklist_observer_;
+  bool downgrade_allowlist_wildcard_to_neutral_ = true;
 };
 
 TEST_F(URLBlocklistManagerTest, SetAndUnsetOverrideBlockListSource) {
@@ -880,5 +952,24 @@
       URLBlocklist::URL_NEUTRAL_STATE,
       blocklist_manager()->GetURLBlocklistState(GURL("http://preconnect.com")));
 }
+
+TEST_F(URLBlocklistManagerTest, SetDowngradeAllowlistWildcardToNeutral) {
+  using State = URLBlocklist::URLBlocklistState;
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      features::kDowngradeURLAllowlistWildcardToNeutral);
+
+  std::unique_ptr<CustomBlocklistSource> custom_blocklist =
+      std::make_unique<CustomBlocklistSource>();
+  custom_blocklist->SetAllowlistSpec(base::ListValue().Append("*"));
+  custom_blocklist->SetDowngradeAllowlistWildcardToNeutral(false);
+
+  blocklist_manager()->SetOverrideBlockListSource(std::move(custom_blocklist));
+  task_environment()->RunUntilIdle();
+
+  // Wildcard is not downgraded because BlocklistSource configured it to false.
+  EXPECT_EQ(State::URL_IN_ALLOWLIST, blocklist_manager()->GetURLBlocklistState(
+                                         GURL("http://example.com")));
+}
 #endif
 }  // namespace policy