From 1b717d86dbb44d6227a6f693931c91157624460e Mon Sep 17 00:00:00 2001
From: Chris Fredrickson <[email protected]>
Date: Wed, 01 Jul 2026 09:20:01 -0700
Subject: [PATCH] [SAA] Fix embargoed permissions exposure

This prevents exposure of an automatic denial via permissions embargo,
per the SAA specification. (Note: this does not disable the embargo,
meaning the user will not be prompted >3 times if they continually
dismiss the prompt.)

Fixed: 519229463
Change-Id: I60985c89171297770a169041c7eea60258986b31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7909441
Reviewed-by: Antonio Sartori <[email protected]>
Auto-Submit: Chris Fredrickson <[email protected]>
Reviewed-by: Chris Fredrickson <[email protected]>
Commit-Queue: Chris Fredrickson <[email protected]>
Reviewed-by: Johann Hofmann <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1655543}
---

diff --git a/chrome/browser/storage_access_api/api_browsertest.cc b/chrome/browser/storage_access_api/api_browsertest.cc
index 6f3da6a..9ed824a 100644
--- a/chrome/browser/storage_access_api/api_browsertest.cc
+++ b/chrome/browser/storage_access_api/api_browsertest.cc
@@ -918,6 +918,37 @@
       UnorderedElementsAre(Pair(net::SchemefulSite(GURL(kOriginB)), false)));
 }
 
+// Test that permissions.query inside a credentialless iframe does not expose
+// embargoed status when a storage access request has been repeatedly dismissed.
+IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest,
+                       PermissionQueryEmbargoed_CredentiallessFrame) {
+  SetBlockThirdPartyCookies(true);
+  EnsureUserInteractionOn(kHostB);
+
+  NavigateToPageWithFrame(kHostA);
+  NavigateFrameTo(kHostB, "/echoheader?cookie");
+
+  prompt_factory()->set_response_type(
+      permissions::PermissionRequestManager::DISMISS);
+
+  // Dismissing the prompt 3 times places the origin under embargo.
+  for (int i = 0; i < 3; ++i) {
+    EXPECT_FALSE(
+        content::ExecJs(GetFrame(), "document.requestStorageAccess()"));
+  }
+  ASSERT_EQ(prompt_factory()->TotalRequestCount(), 3);
+  ASSERT_EQ(QueryPermission(GetFrame()), "prompt");
+
+  NavigateToPageWithFrame(kHostA, /*browser_ptr=*/nullptr,
+                          /*credentialless=*/true);
+  NavigateFrameTo(kHostB, "/echoheader?cookie");
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+
+  // Even when under embargo, permissions.query inside a credentialless iframe
+  // should return "prompt" rather than "denied".
+  EXPECT_EQ(QueryPermission(GetFrame()), "prompt");
+}
+
 IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest, PermissionQueryCrossSite) {
   SetBlockThirdPartyCookies(true);
 
diff --git a/chrome/browser/storage_access_api/storage_access_grant_permission_context.cc b/chrome/browser/storage_access_api/storage_access_grant_permission_context.cc
index 881a364..18b4fdce 100644
--- a/chrome/browser/storage_access_api/storage_access_grant_permission_context.cc
+++ b/chrome/browser/storage_access_api/storage_access_grant_permission_context.cc
@@ -407,29 +407,6 @@
     return;
   }
 
-  {
-    // Normally a previous prompt rejection would already be filtered before
-    // reaching `StorageAccessGrantPermissionContext::DecidePermission`, but the
-    // requirement not to surface the user's denial back to the caller means
-    // this code is reachable even after permission has been blocked.
-    // Accordingly, check the default implementation, and if a denial has been
-    // persisted, respect that decision.
-    ContentSetting existing_setting = permissions::
-        ContentSettingPermissionContextBase::GetContentSettingStatusInternal(
-            rfh, request_data->requesting_origin,
-            request_data->embedding_origin);
-    // ALLOW grants are handled by ContentSettingPermissionContextBase so they
-    // never reach this point.
-    CHECK_NE(existing_setting, CONTENT_SETTING_ALLOW);
-    if (existing_setting == CONTENT_SETTING_BLOCK) {
-      NotifyPermissionSetInternal(*request_data, std::move(callback),
-                                  /*persist=*/false, PermissionDecision::kDeny,
-                                  RequestOutcome::kReusedPreviousDecision);
-      return;
-    }
-    CHECK_EQ(existing_setting, CONTENT_SETTING_ASK);
-  }
-
   // FedCM grants (and the appropriate permissions policy) may allow the call to
   // auto-resolve (without granting a new permission).
   if (FederatedIdentityPermissionContext* fedcm_context =
@@ -582,7 +559,7 @@
     const GURL& embedding_origin) const {
   if (render_frame_host) {
     if (IsAccessRestrictedInFrame(render_frame_host)) {
-      return CONTENT_SETTING_ASK;
+      return CONTENT_SETTING_BLOCK;
     }
 
     // Permission query from top-level frame should be "granted" by default.
@@ -591,16 +568,9 @@
     }
   }
 
-  ContentSetting setting = permissions::ContentSettingPermissionContextBase::
+  return permissions::ContentSettingPermissionContextBase::
       GetContentSettingStatusInternal(render_frame_host, requesting_origin,
                                       embedding_origin);
-
-  // The spec calls for avoiding exposure of rejections to prevent any attempt
-  // at retaliating against users who would reject a prompt.
-  if (setting == CONTENT_SETTING_BLOCK) {
-    return CONTENT_SETTING_ASK;
-  }
-  return setting;
 }
 
 void StorageAccessGrantPermissionContext::NotifyPermissionSet(
@@ -641,6 +611,15 @@
                               decision.overall_decision, outcome);
 }
 
+void StorageAccessGrantPermissionContext::MaybeOverridePermissionResultToReturn(
+    content::PermissionResult& result) const {
+  // The spec calls for avoiding exposure of rejections to prevent any attempt
+  // at retaliating against users who would reject a prompt.
+  if (result.status == content::PermissionStatus::DENIED) {
+    result.status = content::PermissionStatus::ASK;
+  }
+}
+
 void StorageAccessGrantPermissionContext::ReportRelatedWebsiteSetsDeprecation(
     content::RenderFrameHost* rfh) {
   auto deprecation_details = blink::mojom::DeprecationIssueDetails::New();
diff --git a/chrome/browser/storage_access_api/storage_access_grant_permission_context.h b/chrome/browser/storage_access_api/storage_access_grant_permission_context.h
index 1d2e4ed9..19254db 100644
--- a/chrome/browser/storage_access_api/storage_access_grant_permission_context.h
+++ b/chrome/browser/storage_access_api/storage_access_grant_permission_context.h
@@ -91,6 +91,10 @@
       std::unique_ptr<permissions::PermissionRequestData> request_data,
       permissions::BrowserPermissionCallback callback);
 
+  // PermissionContextBase:
+  void MaybeOverridePermissionResultToReturn(
+      content::PermissionResult& result) const override;
+
   static int GetImplicitGrantLimitForTesting();
   static void SetImplicitGrantLimitForTesting(int limit);
 
diff --git a/chrome/browser/storage_access_api/storage_access_grant_permission_context_unittest.cc b/chrome/browser/storage_access_api/storage_access_grant_permission_context_unittest.cc
index ff068b66..6c3e083a 100644
--- a/chrome/browser/storage_access_api/storage_access_grant_permission_context_unittest.cc
+++ b/chrome/browser/storage_access_api/storage_access_grant_permission_context_unittest.cc
@@ -43,6 +43,7 @@
 #include "components/prefs/pref_service.h"
 #include "components/privacy_sandbox/privacy_sandbox_prefs.h"
 #include "content/public/browser/btm_service.h"
+#include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/permission_descriptor_util.h"
 #include "content/public/browser/permission_result.h"
 #include "content/public/browser/web_contents.h"
@@ -85,6 +86,7 @@
 constexpr char kPromptResultHistogram[] = "Permissions.Action.StorageAccess";
 constexpr char kRequestOutcomeHistogram[] = "API.StorageAccess.RequestOutcome";
 constexpr int kImplicitGrantLimit = 5;  // Implicit grant limit for testing.
+constexpr int kDefaultDismissalsBeforeEmbargo = 3;
 
 const uint64_t kDwaEventNameHash =
     base::HashMetricName("StorageAccess.RequestOutcome");
@@ -196,12 +198,20 @@
     ChromeRenderViewHostTestHarness::TearDown();
   }
 
+  content::RenderFrameHost* RenderFrameHostFromID(
+      content::GlobalRenderFrameHostId id) {
+    content::RenderFrameHost* target_rfh = content::RenderFrameHost::FromID(id);
+    return target_rfh ? target_rfh : main_rfh();
+  }
+
   base::test::TestFuture<content::PermissionResult> DecidePermission(
       std::unique_ptr<permissions::PermissionRequestData> request_data,
       bool simulate_user_gesture = true) {
     DCHECK(request_data);
     if (request_data->user_gesture && simulate_user_gesture) {
-      content::RenderFrameHostTester::For(main_rfh())->SimulateUserActivation();
+      content::RenderFrameHostTester::For(
+          RenderFrameHostFromID(request_data->id.global_render_frame_host_id()))
+          ->SimulateUserActivation();
     }
     base::test::TestFuture<content::PermissionResult> future;
     permission_context_->DecidePermissionForTesting(std::move(request_data),
@@ -213,7 +223,9 @@
       std::unique_ptr<permissions::PermissionRequestData> request_data) {
     DCHECK(request_data);

diff --git a/chrome/browser/storage_access_api/api_browsertest.cc b/chrome/browser/storage_access_api/api_browsertest.cc
index 6f3da6a..9ed824a 100644
--- a/chrome/browser/storage_access_api/api_browsertest.cc
+++ b/chrome/browser/storage_access_api/api_browsertest.cc
@@ -918,6 +918,37 @@
       UnorderedElementsAre(Pair(net::SchemefulSite(GURL(kOriginB)), false)));
 }
 
+// Test that permissions.query inside a credentialless iframe does not expose
+// embargoed status when a storage access request has been repeatedly dismissed.
+IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest,
+                       PermissionQueryEmbargoed_CredentiallessFrame) {
+  SetBlockThirdPartyCookies(true);
+  EnsureUserInteractionOn(kHostB);
+
+  NavigateToPageWithFrame(kHostA);
+  NavigateFrameTo(kHostB, "/echoheader?cookie");
+
+  prompt_factory()->set_response_type(
+      permissions::PermissionRequestManager::DISMISS);
+
+  // Dismissing the prompt 3 times places the origin under embargo.
+  for (int i = 0; i < 3; ++i) {
+    EXPECT_FALSE(
+        content::ExecJs(GetFrame(), "document.requestStorageAccess()"));
+  }
+  ASSERT_EQ(prompt_factory()->TotalRequestCount(), 3);
+  ASSERT_EQ(QueryPermission(GetFrame()), "prompt");
+
+  NavigateToPageWithFrame(kHostA, /*browser_ptr=*/nullptr,
+                          /*credentialless=*/true);
+  NavigateFrameTo(kHostB, "/echoheader?cookie");
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+
+  // Even when under embargo, permissions.query inside a credentialless iframe
+  // should return "prompt" rather than "denied".
+  EXPECT_EQ(QueryPermission(GetFrame()), "prompt");
+}
+
 IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest, PermissionQueryCrossSite) {
   SetBlockThirdPartyCookies(true);
diff --git a/chrome/browser/storage_access_api/storage_access_grant_permission_context_unittest.cc b/chrome/browser/storage_access_api/storage_access_grant_permission_context_unittest.cc
index ff068b66..6c3e083a 100644
--- a/chrome/browser/storage_access_api/storage_access_grant_permission_context_unittest.cc
+++ b/chrome/browser/storage_access_api/storage_access_grant_permission_context_unittest.cc
@@ -43,6 +43,7 @@
 #include "components/prefs/pref_service.h"
 #include "components/privacy_sandbox/privacy_sandbox_prefs.h"
 #include "content/public/browser/btm_service.h"
+#include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/permission_descriptor_util.h"
 #include "content/public/browser/permission_result.h"
 #include "content/public/browser/web_contents.h"
@@ -85,6 +86,7 @@
 constexpr char kPromptResultHistogram[] = "Permissions.Action.StorageAccess";
 constexpr char kRequestOutcomeHistogram[] = "API.StorageAccess.RequestOutcome";
 constexpr int kImplicitGrantLimit = 5;  // Implicit grant limit for testing.
+constexpr int kDefaultDismissalsBeforeEmbargo = 3;
 
 const uint64_t kDwaEventNameHash =
     base::HashMetricName("StorageAccess.RequestOutcome");
@@ -196,12 +198,20 @@
     ChromeRenderViewHostTestHarness::TearDown();
   }
 
+  content::RenderFrameHost* RenderFrameHostFromID(
+      content::GlobalRenderFrameHostId id) {
+    content::RenderFrameHost* target_rfh = content::RenderFrameHost::FromID(id);
+    return target_rfh ? target_rfh : main_rfh();
+  }
+
   base::test::TestFuture<content::PermissionResult> DecidePermission(
       std::unique_ptr<permissions::PermissionRequestData> request_data,
       bool simulate_user_gesture = true) {
     DCHECK(request_data);
     if (request_data->user_gesture && simulate_user_gesture) {
-      content::RenderFrameHostTester::For(main_rfh())->SimulateUserActivation();
+      content::RenderFrameHostTester::For(
+          RenderFrameHostFromID(request_data->id.global_render_frame_host_id()))
+          ->SimulateUserActivation();
     }
     base::test::TestFuture<content::PermissionResult> future;
     permission_context_->DecidePermissionForTesting(std::move(request_data),
@@ -213,7 +223,9 @@
       std::unique_ptr<permissions::PermissionRequestData> request_data) {
     DCHECK(request_data);
     if (request_data->user_gesture) {
-      content::RenderFrameHostTester::For(main_rfh())->SimulateUserActivation();
+      content::RenderFrameHostTester::For(
+          RenderFrameHostFromID(request_data->id.global_render_frame_host_id()))
+          ->SimulateUserActivation();
     }
     base::test::TestFuture<content::PermissionResult> future;
     permission_context_->RequestPermissionForTesting(std::move(request_data),
@@ -222,14 +234,15 @@
   }
 
   std::unique_ptr<permissions::PermissionRequestData> MakePermissionRequestData(
-      bool user_gesture) {
+      bool user_gesture,
+      content::RenderFrameHost* rfh = nullptr) {
     return std::make_unique<permissions::PermissionRequestData>(
         content::PermissionDescriptorUtil::
             CreatePermissionDescriptorForPermissionType(
                 permissions::PermissionUtil::
                     ContentSettingsTypeToPermissionType(
                         ContentSettingsType::STORAGE_ACCESS)),
-        CreateFakeID(), user_gesture, GetRequesterURL(), GetTopLevelURL());
+        CreateFakeID(rfh), user_gesture, GetRequesterURL(), GetTopLevelURL());
   }
 
   // Helper to ensure that a given content setting is consistently applied on a
@@ -417,17 +430,14 @@
 
   // Querying permission from a credentialless frame should return ASK (prompt)
   // even if there is a grant.
-  EXPECT_EQ(
-      PermissionStatus::ASK,
-      permission_context()
-          ->GetPermissionStatus(
-              content::PermissionDescriptorUtil::
-                  CreatePermissionDescriptorForPermissionType(
-                      permissions::PermissionUtil::
-                          ContentSettingsTypeToPermissionType(
-                              permission_context()->content_settings_type())),
-              child_rfh, GetRequesterURL(), GetTopLevelURL())
-          .status);
+  content::PermissionResult result = permission_context()->GetPermissionStatus(
+      content::PermissionDescriptorUtil::
+          CreatePermissionDescriptorForPermissionType(
+              permissions::PermissionUtil::ContentSettingsTypeToPermissionType(
+                  permission_context()->content_settings_type())),
+      child_rfh, GetRequesterURL(), GetTopLevelURL());
+  permission_context()->MaybeOverridePermissionResultToReturn(result);
+  EXPECT_EQ(PermissionStatus::ASK, result.status);
 }
 
 // Test that after a successful explicit storage access grant, there's a content
@@ -897,6 +907,14 @@
 
 TEST_F(StorageAccessGrantPermissionContextTest,
        ExplicitGrantDenialNotExposedViaQuery) {
+  content::RenderFrameHost* child_rfh =
+      content::RenderFrameHostTester::For(main_rfh())->AppendChild("child");
+  std::unique_ptr<content::NavigationSimulator> navigation =
+      content::NavigationSimulator::CreateRendererInitiated(GetRequesterURL(),
+                                                            child_rfh);
+  navigation->Commit();
+  child_rfh = navigation->GetFinalRenderFrameHost();
+
   // Set the content setting to blocked, mimicking a prompt rejection by the
   // user.
   HostContentSettingsMap* settings_map =
@@ -908,26 +926,22 @@
   prompt_factory().set_response_type(
       permissions::PermissionRequestManager::AutoResponseType::NONE);
 
-  auto future =
-      DecidePermission(MakePermissionRequestData(/*user_gesture=*/true));
+  auto future = RequestPermission(
+      MakePermissionRequestData(/*user_gesture=*/true, child_rfh));
   // Ensure the prompt is not shown.
   ASSERT_FALSE(request_manager()->IsRequestInProgress());
   EXPECT_EQ(PermissionStatus::DENIED, future.Get().status);
 
   // However, ensure that the user's denial is not exposed when querying the
   // permission, per the spec.
-  EXPECT_EQ(
-      PermissionStatus::ASK,
-      permission_context()
-          ->GetPermissionStatus(
-              content::PermissionDescriptorUtil::
-                  CreatePermissionDescriptorForPermissionType(
-                      permissions::PermissionUtil::
-                          ContentSettingsTypeToPermissionType(
-                              permission_context()->content_settings_type())),
-              /*render_frame_host=*/nullptr, GetRequesterURL(),
-              GetTopLevelURL())
-          .status);
+  content::PermissionResult result = permission_context()->GetPermissionStatus(
+      content::PermissionDescriptorUtil::
+          CreatePermissionDescriptorForPermissionType(
+              permissions::PermissionUtil::ContentSettingsTypeToPermissionType(
+                  permission_context()->content_settings_type())),
+      child_rfh, GetRequesterURL(), GetTopLevelURL());
+  permission_context()->MaybeOverridePermissionResultToReturn(result);
+  EXPECT_EQ(PermissionStatus::ASK, result.status);
 
   EXPECT_THAT(page_specific_content_settings()->GetTwoSiteRequests(
                   ContentSettingsType::STORAGE_ACCESS),
@@ -1045,6 +1059,67 @@
               IsEmpty());
 }
 
+TEST_F(StorageAccessGrantPermissionContextTest, RepeatedDismissalsNotExposed) {
+  content::RenderFrameHost* child_rfh =
+      content::RenderFrameHostTester::For(main_rfh())->AppendChild("child");
+  std::unique_ptr<content::NavigationSimulator> navigation =
+      content::NavigationSimulator::CreateRendererInitiated(GetRequesterURL(),
+                                                            child_rfh);
+  navigation->Commit();
+  child_rfh = navigation->GetFinalRenderFrameHost();
+
+  prompt_factory().set_response_type(
+      permissions::PermissionRequestManager::AutoResponseType::DISMISS);
+
+  for (int i = 0; i < kDefaultDismissalsBeforeEmbargo; ++i) {
+    EXPECT_EQ(PermissionStatus::ASK,
+              RequestPermission(
+                  MakePermissionRequestData(/*user_gesture=*/true, child_rfh))
+                  .Get()
+                  .status);
+  }
+
+  content::PermissionResult result = permission_context()->GetPermissionStatus(
+      content::PermissionDescriptorUtil::
+          CreatePermissionDescriptorForPermissionType(
+              permissions::PermissionUtil::ContentSettingsTypeToPermissionType(
+                  permission_context()->content_settings_type())),
+      child_rfh, GetRequesterURL(), GetTopLevelURL());
+  permission_context()->MaybeOverridePermissionResultToReturn(result);
+  EXPECT_EQ(PermissionStatus::ASK, result.status);
+}
+
+TEST_F(StorageAccessGrantPermissionContextTest,
+       EmbargoActivatesAfterRepeatedDismissals) {
+  content::RenderFrameHost* child_rfh =
+      content::RenderFrameHostTester::For(main_rfh())->AppendChild("child");
+  std::unique_ptr<content::NavigationSimulator> navigation =
+      content::NavigationSimulator::CreateRendererInitiated(GetRequesterURL(),
+                                                            child_rfh);
+  navigation->Commit();
+  child_rfh = navigation->GetFinalRenderFrameHost();
+
+  prompt_factory().set_response_type(
+      permissions::PermissionRequestManager::AutoResponseType::DISMISS);
+
+  for (int i = 0; i < kDefaultDismissalsBeforeEmbargo; ++i) {
+    EXPECT_EQ(PermissionStatus::ASK,
+              RequestPermission(
+                  MakePermissionRequestData(/*user_gesture=*/true, child_rfh))
+                  .Get()
+                  .status);
+  }
+
+  prompt_factory().set_response_type(
+      permissions::PermissionRequestManager::AutoResponseType::NONE);
+
+  EXPECT_EQ(PermissionStatus::DENIED,
+            RequestPermission(
+                MakePermissionRequestData(/*user_gesture=*/true, child_rfh))
+                .Get()
+                .status);
+}
+
 class StorageAccessGrantPermissionContextAPIWithFedCMConnectionTest
     : public StorageAccessGrantPermissionContextTest,
       public testing::WithParamInterface<bool> {
diff --git a/components/permissions/permission_manager_unittest.cc b/components/permissions/permission_manager_unittest.cc
index 96c3f65..b215d147 100644
--- a/components/permissions/permission_manager_unittest.cc
+++ b/components/permissions/permission_manager_unittest.cc
@@ -938,6 +938,34 @@
   EXPECT_TRUE(!context);
 }
 
+TEST_F(PermissionManagerTest, StorageAccessPermissionStatusMasksDenied) {
+  SetPermission(PermissionType::STORAGE_ACCESS_GRANT,
+                PermissionStatus::GRANTED);
+  CheckPermissionStatus(PermissionType::STORAGE_ACCESS_GRANT,
+                        PermissionStatus::GRANTED);
+
+  SetPermission(PermissionType::STORAGE_ACCESS_GRANT, PermissionStatus::ASK);
+  CheckPermissionStatus(PermissionType::STORAGE_ACCESS_GRANT,
+                        PermissionStatus::ASK);
+
+  SetPermission(PermissionType::STORAGE_ACCESS_GRANT, PermissionStatus::DENIED);
+  CheckPermissionStatus(PermissionType::STORAGE_ACCESS_GRANT,
+                        PermissionStatus::ASK);
+  CheckPermissionResult(PermissionType::STORAGE_ACCESS_GRANT,
+                        PermissionStatus::ASK,
+                        content::PermissionStatusSource::UNSPECIFIED);
+}
+
+TEST_F(PermissionManagerTest, StorageAccessPermissionRequestMasksDenied) {
+  NavigateAndCommit(url());
+  SetPermission(PermissionType::STORAGE_ACCESS_GRANT, PermissionStatus::DENIED);
+
+  RequestPermissionFromCurrentDocument(PermissionType::STORAGE_ACCESS_GRANT,
+                                       main_rfh());
+  EXPECT_TRUE(callback_called());
+  EXPECT_EQ(PermissionStatus::ASK, callback_result());
+}
+
 class PermissionManagerWithGeolocationTest : public PermissionManagerTest {
  public:
   PermissionManagerWithGeolocationTest() {
diff --git a/components/permissions/test/permission_test_util.cc b/components/permissions/test/permission_test_util.cc
index 3818979..4592373a 100644
--- a/components/permissions/test/permission_test_util.cc
... (truncated)