Added Custom Scriptlet section in the settings page. #25999

boocmp · 2024-10-15T14:42:29Z

Resolves brave/brave-browser#25586

Empty list

Add new scriplet

Add new scriplet with code

Scriptlet in the list and custom filter

Execution on the page

Dev mode toggle

dev_mode.mp4

Submitter Checklist:

I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on
All relevant documentation has been updated, for instance:

Test Plan:

brave-builds · 2024-10-15T15:42:07Z

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

app/brave_settings_strings.grdp

mkarolin

strings++ (with 1 change)

components/brave_shields/core/browser/ad_block_custom_resource_provider.cc

components/brave_shields/content/browser/ad_block_service.h

app/brave_settings_strings.grdp

ShivanKaul · 2024-10-30T23:21:47Z

@boocmp how do we handle naming collisions? For instance, if a user tries to create a new scriptlet called brave-fix which conflicts with an existing scriptlet, how do we handle that?

boocmp · 2024-10-31T04:17:11Z

We don't check. Default resources have priority over customs so if it's called 'brave-fix` it will be ignored.

how do we handle naming collisions? For instance, if a user tries to create a new scriptlet called brave-fix which conflicts with an existing scriptlet, how do we handle that?

browser/ui/webui/settings/brave_adblock_handler.cc

diracdeltas

see https://github.com/brave/reviews/issues/1783#issuecomment-2449041760

components/brave_shields/core/common/features.cc

antonok-edm · 2024-12-11T18:39:13Z

components/brave_shields/core/browser/ad_block_custom_resource_provider.cc

+
+  if (*mime == kAppJs) {
+    // Resource is a scriptlet:
+    if (!name->starts_with("brave-") || !name->ends_with(".js")) {


If we're going to enforce certain prefixes on user scriptlets, we should reserve brave- for Brave's CRX-provided scriptlets, and give user-provided scriptlets a user- prefix instead.

makes sense to me too

…td::string::starts_with. Added test.

…gs method. Added test.

…lets in readonly mode.

github-actions · 2024-12-12T11:19:39Z

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

Description

This PR adds support for custom scriptlets in the AdBlock feature of the Brave browser. It introduces a developer mode that allows users to add, edit, and remove custom scriptlets, which are small JavaScript snippets used for ad blocking and content modification.

Possible Issues

Enabling custom scriptlets could potentially introduce security risks if users add malicious scripts.
The feature is behind a feature flag, which might limit its availability to users.

Security Hotspots

Custom scriptlet execution: The ability for users to add and execute custom JavaScript could be a potential security risk if not properly sanitized or restricted.
Developer mode toggle: Ensure that enabling developer mode doesn't inadvertently expose sensitive functionality to regular users.

Changes

Changes

app/brave_settings_strings.grdp:
- Added new string resources for custom scriptlet UI elements and messages.
browser/about_flags.cc:
- Added a new feature flag for Brave AdBlock Custom Scriptlets.
browser/brave_profile_prefs.cc:
- Registered a new boolean preference for AdBlock developer mode.
browser/brave_shields/ad_block_custom_resources_browsertest.cc:
- Added new browser tests for custom scriptlet functionality.
browser/brave_shields/ad_block_pref_service_factory.cc:
- Updated to include the new developer mode preference.
browser/resources/settings/default_brave_shields_page/:
- Added new UI components for managing custom scriptlets.
browser/ui/webui/brave_adblock_ui.cc:
- Updated to handle custom scriptlet-related UI interactions.
browser/ui/webui/settings/brave_adblock_handler.cc:
- Added methods to handle custom scriptlet operations (add, update, remove).
components/brave_shields/content/browser/ad_block_service.cc:
- Modified to support custom scriptlets and developer mode.
components/brave_shields/core/browser/ad_block_custom_resource_provider.cc:
- Implemented the core functionality for managing custom scriptlets.
components/brave_shields/core/common/features.cc:
- Added a new feature flag for custom scriptlets.
components/brave_shields/core/common/pref_names.h:
- Added a new preference name for AdBlock developer mode.

sequenceDiagram
    User->>BraveSettings: Enable AdBlock Developer Mode
    BraveSettings->>AdBlockService: EnableDeveloperMode(true)
    User->>BraveSettings: Add Custom Scriptlet
    BraveSettings->>AdBlockCustomResourceProvider: AddResource(scriptlet)
    AdBlockCustomResourceProvider->>Storage: Save Custom Scriptlet
    AdBlockCustomResourceProvider->>AdBlockService: ReloadResourcesAndNotify()
    AdBlockService->>AdBlockEngine: Update with new scriptlet
    User->>Webpage: Visit
    Webpage->>AdBlockEngine: Check for blocking
    AdBlockEngine->>Webpage: Apply Custom Scriptlet

github-actions · 2024-12-12T11:23:42Z

components/brave_shields/content/browser/ad_block_pref_service.cc

+  pref_change_registrar_->Add(
+      prefs::kAdBlockDeveloperMode,
+      base::BindRepeating(&AdBlockPrefService::OnDeveloperModeChanged,
+                          base::Unretained(this)));


_{reported by reviewdog 🐶}
[semgrep] base::Unretained is most of the time unrequited, and a weak reference is better suited for secure coding.
Consider swapping Unretained for a weak reference.
base::Unretained usage may be acceptable when a callback owner is guaranteed
to be destroyed with the object base::Unretained is pointing to, for example:

- PrefChangeRegistrar
- base::*Timer
- mojo::Receiver
- any other class member destroyed when the class is deallocated

Source: https://github.com/brave/security-action/blob/main/assets/semgrep_rules/client/chromium-uaf.yaml

Cc @thypon @goodov @iefremov

boocmp self-assigned this Oct 15, 2024

antonok-edm reviewed Oct 18, 2024

View reviewed changes

app/brave_settings_strings.grdp Show resolved Hide resolved

github-actions bot added the chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch label Oct 20, 2024

boocmp force-pushed the issues/25586 branch from 03c6570 to 8cf76d6 Compare October 21, 2024 11:09

github-actions bot removed the chromium-version-mismatch The Chromium version on the PR branch does not match the version on the target branch label Oct 21, 2024

boocmp marked this pull request as ready for review October 21, 2024 11:10

boocmp requested review from a team as code owners October 21, 2024 11:10

mkarolin reviewed Oct 21, 2024

View reviewed changes

app/brave_settings_strings.grdp Outdated Show resolved Hide resolved

mkarolin approved these changes Oct 21, 2024

View reviewed changes

goodov reviewed Oct 22, 2024

View reviewed changes

components/brave_shields/core/browser/ad_block_custom_resource_provider.cc Outdated Show resolved Hide resolved

components/brave_shields/content/browser/ad_block_service.h Outdated Show resolved Hide resolved

boocmp requested review from ShivanKaul and a team October 24, 2024 01:10

github-actions bot added the puLL-Merge label Oct 24, 2024

goodov approved these changes Oct 24, 2024

View reviewed changes

app/brave_settings_strings.grdp Outdated Show resolved Hide resolved

diracdeltas reviewed Oct 31, 2024

View reviewed changes

browser/ui/webui/settings/brave_adblock_handler.cc Outdated Show resolved Hide resolved

boocmp force-pushed the issues/25586 branch from 496f8ce to 215f8f5 Compare October 31, 2024 05:29

diracdeltas requested changes Nov 4, 2024

View reviewed changes

boocmp force-pushed the issues/25586 branch from 215f8f5 to 0146bf7 Compare November 7, 2024 15:32

diracdeltas reviewed Nov 12, 2024

View reviewed changes

components/brave_shields/core/common/features.cc Outdated Show resolved Hide resolved

diracdeltas self-requested a review November 13, 2024 17:39

boocmp force-pushed the issues/25586 branch from 0146bf7 to 52e78c5 Compare November 14, 2024 15:19

boocmp requested review from a team as code owners November 14, 2024 15:19

boocmp removed the request for review from a team December 3, 2024 11:22

antonok-edm reviewed Dec 11, 2024

View reviewed changes

ShivanKaul approved these changes Dec 11, 2024

View reviewed changes

boocmp added 21 commits December 12, 2024 18:18

Added Custom Scriptlet section in the settings page.

b94d4b0

Added tests.

7108965

Added security warning message.

8d9f0c1

Review issues.

760530b

Removed FixName function.

b140a2e

Removed '\n' from warnign message.

1057f08

Changed regex for name checking.

774ff6a

Highlight placeholder with red color.

cf2967f

Removed unnecessary AllowJavascript. base::StartsWith replaced with s…

626c6fc

…td::string::starts_with. Added test.

Using leveldb instead of local_state to store the custom scriplets.

1aa3bea

Added adblock Developer mode protected pref.

46eaedf

Unit tests issue.

7992dae

Typo.

fe63c16

Moved 'developer mode' check to the new UpdateCustomFiltersFromSettin…

d291ee0

…gs method. Added test.

Android compilation.

c31708b

Changed the devmode toggle's desc. Exposed feature to the flags UI.

fe5a7d9

Make custom filters readonly if dev-mode is disabled.

825fd93

Do not enable Dev mode if there is existing rules. Show custom script…

7d53aaa

…lets in readonly mode.

Unnecessery change removed.

880c076

Settings ui fixes.

d080cf6

'user-' prefix for custom scriptlets instead of 'brave-'

7c6ccb0

boocmp force-pushed the issues/25586 branch from 18c1384 to 7c6ccb0 Compare December 12, 2024 11:18

github-actions bot reviewed Dec 12, 2024

View reviewed changes

boocmp merged commit 1f0e2fc into master Dec 13, 2024
19 of 20 checks passed

boocmp deleted the issues/25586 branch December 13, 2024 05:22

github-actions bot added this to the 1.75.x - Nightly milestone Dec 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Custom Scriptlet section in the settings page. #25999

Added Custom Scriptlet section in the settings page. #25999

boocmp commented Oct 15, 2024 •

edited

Loading

brave-builds commented Oct 15, 2024

mkarolin left a comment

ShivanKaul commented Oct 30, 2024

boocmp commented Oct 31, 2024

diracdeltas left a comment

antonok-edm Dec 11, 2024

ShivanKaul Dec 11, 2024

github-actions bot commented Dec 12, 2024

Changes

github-actions bot Dec 12, 2024

Added Custom Scriptlet section in the settings page. #25999

Added Custom Scriptlet section in the settings page. #25999

Conversation

boocmp commented Oct 15, 2024 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

brave-builds commented Oct 15, 2024

mkarolin left a comment

Choose a reason for hiding this comment

ShivanKaul commented Oct 30, 2024

boocmp commented Oct 31, 2024

diracdeltas left a comment

Choose a reason for hiding this comment

antonok-edm Dec 11, 2024

Choose a reason for hiding this comment

ShivanKaul Dec 11, 2024

Choose a reason for hiding this comment

github-actions bot commented Dec 12, 2024

Description

Possible Issues

Security Hotspots

Changes

github-actions bot Dec 12, 2024

Choose a reason for hiding this comment

boocmp commented Oct 15, 2024 •

edited

Loading