[PSST] Show infobar when the feature availability is detected for the website

[vadimstruts]

Resolves brave/brave-browser#53452

Added display of the infobar when PSST feature availability is detected for the website.
When the user clicks the Review button on the infobar, we plan to show a WebUI modal dialog, which will be added in follow up.

Here is screenshot of the infobar:
1

[github-actions]

📋 Code Owners Summary

14 file(s) changed, 2 with assigned owners

2 team(s) affected: @brave/chromium-src-reviewers, @brave/string-reviewers-team

---

Owners and Their Files

@brave/string-reviewers-team — 1 file(s)

• app/brave_generated_resources.grd

@brave/chromium-src-reviewers — 1 file(s)

• chromium_src/components/infobars/core/infobar_delegate.h

[brave-builds]

A Storybook has been deployed to preview UI for the latest push

[github-actions]

Chromium major version is behind target branch (145.0.7632.120 vs 146.0.7680.32). Please rebase.

[fallaciousreasoning]

going to defer to @boocmp on the review here as I'm not too familiar with this feature

[github-actions]

_{reported by reviewdog 🐶}
[opengrep] RequestAsyncExecuteScript usages should be vet by the security-team.

References:
- https://github.com/brave/brave-browser/wiki/Security-reviews (point 12)


Source: https://github.com/brave/security-action/blob/main/assets/opengrep_rules/client/brave-execute-script.yaml


Cc @stoletheminerals @diracdeltas @bridiver

[diracdeltas]

how are you planning to distribute these scripts (component updater)? will the initial set of scripts and updates be security audited?

note ISOLATED_WORLD_ID_BRAVE_INTERNAL is reused for a bunch of stuff like brave ads, publishers, speedreader, etc; so if one of these scripts is compromised, all the others can be prototype-polluted. therefore if these scripts deal with untrusted input, it might make sense to use a separate world id for them. see https://bravesoftware.slack.com/archives/C7VLGSR55/p1673046335116019 for info.

[vadimstruts]

how are you planning to distribute these scripts

It will be distributed like other CRX components, so the component updater
will receive new versions of the scripts in the standard way.

will the initial set of scripts and updates be security audited?

I think it will be required once we are ready to release the initial version,
at least for the first website.

it might make sense to use a separate world id for them

It make sense

[ShivanKaul]

@diracdeltas we'd discussed having a separate world ID for PSST in this security review: https://github.com/brave/reviews/issues/2020#issuecomment-3210147843

[diracdeltas]

k no concerns from me then

[fallaciousreasoning]

lgtm % nits

[mkarolin]

strings, chromium_src ++

[brave-builds]

Warning

You have got a presubmit warning. Please address it if possible.

browser/psst/psst_ui_delegate_impl.h: WebContents forward declaration is no longer needed

[SergeyZhukovsky]

++

[github-actions]

[puLL-Merge] - brave/brave-core@34174

Description

This PR evolves the PSST (Privacy Settings Suggestions Tool) feature by adding a UI layer consisting of an infobar notification and a presenter pattern. The main changes include:

1. New InfoBar delegate for showing PSST suggestions to users
2. New UI presenter abstraction (PsstUiPresenter interface with UiDesktopPresenter implementation) to decouple UI presentation from business logic
3. Refactored PsstUiDelegateImpl to support the new UI flow with observer pattern, infobar integration, and consent management
4. Changed navigation observation from DidFinishNavigation to NavigationEntryCommitted
5. Added async script injection support for policy scripts
6. Richer script result parsing with new property names for tasks, site names, and navigation flow
7. Removed browser tests in favor of expanded unit tests with mocks
8. Added navigation after policy script execution — the observer can now navigate to URLs returned by policy scripts

Possible Issues

1. base::ListValue is deprecated: The code uses base::ListValue in multiple places (e.g., PsstUiDelegateImpl::Show, ListValueToStringVector). base::ListValue is a deprecated alias and should be replaced with base::Value::List.

2. ListValueToStringVector silently drops non-string values: If the list contains non-string values, they are silently skipped. This may hide data issues.

3. OnUserAcceptedInfobar is incomplete: The method has empty if/else branches with comments like "save info in preferences that infobar is accepted" and "User declined the infobar" — no actual logic is implemented. When the user accepts the infobar, nothing happens to actually trigger the consent flow (no call to OnUserAcceptedPsstSettings), and when they decline, nothing is persisted.

4. Cancel() in PsstInfoBarDelegate fires callback with false: But the delegate only declares BUTTON_OK in GetButtons(). The Cancel method may never be called since there's no cancel button, meaning the callback might leak if the infobar is dismissed without accepting.

5. Automatic navigation is risky: In OnPolicyScriptResult, the observer navigates to URLs (next_url, start_url) returned by script execution via LoadURL. If the script result is compromised or malformed, this could navigate the user to unexpected URLs. The only validation is url_to_load.is_valid().

6. raw_ptr<content::WebContents> in UiDesktopPresenter: The web_contents_ pointer has no guarantee of outliving the presenter. If the WebContents is destroyed while the presenter is alive, this becomes a dangling pointer.

7. Mock in content namespace: The unit test adds MockNavigationEntry directly inside the content namespace, which is fragile and could conflict with upstream changes.

8. std::find used instead of std::ranges::find: In PrepareParametersForPolicyExecution, std::find is used while std::ranges is used elsewhere in the same PR, creating inconsistency.

Security Hotspots

1. Unvalidated navigation from script results (psst_tab_web_contents_observer.cc, OnPolicyScriptResult): URLs from next_url and start_url are loaded directly. A compromised component update or injected script could redirect users to phishing or malicious sites. There should be origin/scheme validation beyond just is_valid().

2. Script injection with UserActivationOption::kActivate (psst_ui_presenter.cc via the async script callback): The async script injection grants user activation, which could allow the injected script to perform actions that normally require user gestures (popups, downloads, etc.).

Changes

Changes

• browser/psst/BUILD.gn: Removed browser_tests source set; added new source files and dependencies for infobar/presenter.
• browser/psst/psst_infobar_delegate.{cc,h}: New ConfirmInfoBarDelegate subclass for PSST infobar with accept/cancel callbacks.
• browser/psst/psst_ui_presenter.{cc,h}: New PsstUiPresenter interface and UiDesktopPresenter implementation for showing infobars and icons.
• browser/psst/psst_ui_delegate_impl.{cc,h}: Refactored to use presenter, observer pattern, and new Show signature with site name and tasks.
• browser/psst/psst_tab_web_contents_observer_browsertest.cc: Deleted.
• components/psst/browser/content/psst_tab_web_contents_observer.{cc,h}: Switched to NavigationEntryCommitted, added async script injection, richer result parsing, and post-policy navigation.
• components/psst/browser/content/psst_tab_web_contents_observer_unittest.cc: Expanded unit tests with mocks for new flow.
• app/brave_generated_resources.grd: Added PSST infobar strings.
• chromium_src/components/infobars/core/infobar_delegate.h: Added BRAVE_PSST_INFOBAR_DELEGATE identifier.
• browser/ui/tabs/brave_tab_features.cc: Updated to pass UiDesktopPresenter to PsstUiDelegateImpl.

sequenceDiagram
    participant User
    participant WebContents
    participant Observer as PsstTabWebContentsObserver
    participant Registry as PsstRuleRegistry
    participant UiDelegate as PsstUiDelegateImpl
    participant Presenter as UiDesktopPresenter
    participant InfoBar as PsstInfoBarDelegate

    WebContents->>Observer: NavigationEntryCommitted()
    Observer->>Observer: SetUserData(kShouldProcessKey)
    WebContents->>Observer: DocumentOnLoadCompleted()
    Observer->>Registry: CheckIfMatch(url)
    Registry-->>Observer: MatchedRule
    Observer->>WebContents: inject_script_callback_(user_script)
    WebContents-->>Observer: OnUserScriptResult(script_result)
    Observer->>UiDelegate: GetPsstWebsiteSettings(origin, user_id)
    UiDelegate-->>Observer: std::nullopt (first visit)
    Observer->>UiDelegate: Show(origin, settings, site_name, tasks, callback)
    UiDelegate->>UiDelegate: Check consent_status
    UiDelegate->>Presenter: ShowIcon()
    UiDelegate->>Presenter: ShowInfoBar(callback)
    Presenter->>InfoBar: Create(infobar_manager, callback)
    User->>InfoBar: Accept
    InfoBar->>Presenter: OnInfobarAccepted(true)
    Presenter->>UiDelegate: OnUserAcceptedInfobar(origin, true)
    UiDelegate->>Observer: ConsentCallback(disabled_checks)
    Observer->>Observer: PrepareParametersForPolicyExecution()
    Observer->>WebContents: inject_async_script_callback_(policy_script)
    WebContents-->>Observer: OnPolicyScriptResult(result)
    Observer->>UiDelegate: UpdateTasks(progress, tasks, status)
    Observer->>WebContents: LoadURL(next_url)

Loading

[bridiver]

I think this is generally supposed to use url_formatter::FormatOriginForSecurityDisplay, but also what happens if the webcontents has navigated by the time the callback runs? Does navigation clear it? Is there a test for that? This looks like a potential security spoofing issue to me.

[boocmp]

There is a check for ShouldInsertScriptForPage at func's beginning, it checks for the navigation entry id, this id is updated in the DocumentOnLoadCompletedInPrimaryMainFrame. There are several tests with navigations, @vadimstruts could you point to the one which checks the mentioned behavior?

[vadimstruts]

As for

what happens if the webcontents has navigated by the time the callback runs

When a user navigates to any website for the first time (in NavigationEntryCommitted), we check the following conditions:

• The main frame navigated to a different page
• The entry's URL has an http or https scheme
• The entry is not restored
• PSST is enabled in preferences

If all conditions are met, we save the current entry ID to user data and use ShouldInsertScriptForPage to validate it at the beginning of every relevant function.
Navigations Related Unittests:

brave-core/components/psst/browser/content/psst_tab_web_contents_observer_unittest.cc

Lines 395 to 438 in 2a117f6

	TEST_F(PsstTabWebContentsObserverUnitTest,
	ShouldNotProcessRestoredNavigationEntry) {
	auto url = GURL("https://example1.com");
	auto restored_entry = std::make_unique<content::MockNavigationEntry>();
	EXPECT_CALL(*restored_entry.get(), GetURL())
	.WillOnce(testing::ReturnRef(url));
	EXPECT_CALL(*restored_entry.get(), IsRestored())
	.WillOnce(testing::Return(true));
	content::LoadCommittedDetails details;
	details.entry = restored_entry.get();
	psst_web_contents_observer().NavigationEntryCommitted(details);
	EXPECT_EQ(nullptr, restored_entry->GetUserData("should_process_key"));
	}
	TEST_F(PsstTabWebContentsObserverUnitTest, ShouldOnlyProcessHttpOrHttps) {
	EXPECT_CALL(psst_rule_registry(), CheckIfMatch(_, _)).Times(0);
	{
	DocumentOnLoadObserver observer(web_contents());
	content::NavigationSimulator::NavigateAndCommitFromBrowser(
	web_contents(), GURL("chrome://flags"));
	observer.Wait();
	}
	{
	DocumentOnLoadObserver observer(web_contents());
	content::NavigationSimulator::NavigateAndCommitFromBrowser(
	web_contents(), GURL("about:blank"));
	observer.Wait();
	}
	{
	DocumentOnLoadObserver observer(web_contents());
	content::NavigationSimulator::NavigateAndCommitFromBrowser(
	web_contents(), GURL("file:///somepath"));
	observer.Wait();
	}
	{
	DocumentOnLoadObserver observer(web_contents());
	content::NavigationSimulator::NavigateAndCommitFromBrowser(
	web_contents(), GURL("ftp://example.com"));
	observer.Wait();
	}
	}

[vadimstruts]

As for

I think this is generally supposed to use url_formatter::FormatOriginForSecurityDisplay

In the current PR, we use the origin solely to save and retrieve website-related settings data, i.e. it is not visible to the user. Even in the follow-up PR (coming soon), when we add the consent dialog to display the feature status, we will only show the user information from the loaded CRX component.

[bridiver]

why are we getting the site_name from JS? Is there a description somewhere of what these properties are and how they are determined? We may not want to trust this value if it can be potentially spoofed by the page. Since we're not running this before any page code, the page can spoof just about anything by messing with prototypes

[boocmp]

PSST executes scripts in the ISOLATED_WORLD_ID_BRAVE_INTERNAL, page can't spoof this

[vadimstruts]

The site name is part of the information retrieved from user.js, which is managed as a CRX component.
Here is example of user.js code:
https://github.com/brave-experiments/psst-component/blob/main/scripts/twitter/user.js#L38
This means that we will prepare the content of the CRX component, which contains the user.js, policy.js and the config: https://github.com/brave-experiments/psst-component/blob/main/psst.json#L3 for each website we want to support.
Both scripts, as @boocmp said, will be executed in the ISOLATED_WORLD_ID_BRAVE_INTERNAL

[bridiver]

ah right, forgot it was an isolated world, but still curious what we're returning here for site_name that we need js for?

[bridiver]

it would really be helpful is there were more comments here or documentation about what is actually happening in these scripts and what all these values are