Skip to content

Implement scriptlet download and storage#8557

Open
CrisBarreiro wants to merge 20 commits into
developfrom
feature/cris/ad-blocking/download-store-scriptlets
Open

Implement scriptlet download and storage#8557
CrisBarreiro wants to merge 20 commits into
developfrom
feature/cris/ad-blocking/download-store-scriptlets

Conversation

@CrisBarreiro
Copy link
Copy Markdown
Collaborator

@CrisBarreiro CrisBarreiro commented May 14, 2026

Task/Issue URL: https://app.asana.com/1/137249556945/project/488551667048375/task/1214486375443736?focus=true

Description

Steps to test this PR

Tip

Set logcat filter package:mine tag~:"RealScriptletUpdater|ScriptletDownloadWorker|AdBlockingExtensionProvider|RealPrivacyConfigDownloader|ScriptletSignatureValidator|RealAdBlockingExtensionConfigProvider"

Feature 1 - RC flag disabled

  • Install the app
  • Check logs for onPrivacyConfigDownloaded
  • Check no logs for files actually downloaded
  • Open database inspector and check ad_blocking_extension.db doesn't exist

Feature 2 - RC flag disabled with settings

  • Apply Patch 1 - update privacy-config
  • Fresh install the app
  • Check logs for onPrivacyConfigDownloaded
  • Check no logs for files actually downloaded
  • Open database inspector and check ad_blocking_extension.db doesn't exist

Feature 3 - Kill-switch on, operational flag off (feature is discoverable but dormant)

  • From the previous scenario, enable adBlockingExtension -> isDiscoverable from the feature flags inventory
  • Kill and reopen the app
  • Check logs for onPrivacyConfigDownloaded
  • Check no logs for files actually downloaded
  • Open database inspector and check ad_blocking_extension.db doesn't exist

Feature 4 - Both flags enabled (full operational path)

  • From the previous scenario, enable adBlockingExtension from the feature flags inventory
  • Check logs for onPrivacyConfigDownloaded, Validating scriptlet succeeded (x4), and Storing scriptlets
  • Open database inspector and check ad_blocking_extension.db exists, and has the following contents:
    • name: scriptlets/isolated/ublock-filters.js, version: 2026.5.12, content: {some bytes}
    • name: scriptlets/main/ublock-filters.js, version: 2026.5.12, content: {some bytes}
  • Kill and reopen the app
  • Check logs for Version matches stored. Skipping

Feature 5 - RC flag disabled with settings

  • Apply Patch 1 - update privacy-config
  • Fresh install the app
  • Enable adBlockingExtension from the feature flags inventory
  • Kill and restart the app
  • Check logs for onPrivacyConfigDownloaded
  • Check no logs for files actually downloaded
  • Open database inspector and check ad_blocking_extension.db doesn't exist
  • Enable adBlockingExtension -> isDiscoverable from the feature flags inventory
  • Open database inspector and check ad_blocking_extension.db exists, and has the following contents (you might need to stop inspectors and start inspector again):
    • name: scriptlets/isolated/ublock-filters.js, version: 2026.5.12, content: {some bytes}
    • name: scriptlets/main/ublock-filters.js, version: 2026.5.12, content: {some bytes}

Patches

Patch 1 - update privacy-config

Subject: [PATCH] Update privacy-config URL
---
Index: privacy-config/privacy-config-api/src/main/java/com/duckduckgo/privacy/config/api/PrivacyFeatureName.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/privacy-config/privacy-config-api/src/main/java/com/duckduckgo/privacy/config/api/PrivacyFeatureName.kt b/privacy-config/privacy-config-api/src/main/java/com/duckduckgo/privacy/config/api/PrivacyFeatureName.kt
--- a/privacy-config/privacy-config-api/src/main/java/com/duckduckgo/privacy/config/api/PrivacyFeatureName.kt	(revision 476d1a965596de1bab6c642a9b023d708bd86f04)
+++ b/privacy-config/privacy-config-api/src/main/java/com/duckduckgo/privacy/config/api/PrivacyFeatureName.kt	(date 1778775474685)
@@ -29,4 +29,4 @@
     TrackingParametersFeatureName("trackingParameters"),
 }
 
-const val PRIVACY_REMOTE_CONFIG_URL = "https://staticcdn.duckduckgo.com/trackerblocking/config/v5/android-config.json"
+const val PRIVACY_REMOTE_CONFIG_URL = "https://duckduckgo.github.io/privacy-configuration/pr-5175/v4/android-config.json"

UI changes

n/a


Note

Medium Risk
Adds new background download + signature verification and persists content to a new Room database, so failures could impact update reliability and introduces new network/storage paths. Feature is gated by remote toggles (kill-switch/contingency) which reduces blast radius but the crypto/WorkManager logic is still moderately risk-prone.

Overview
Implements end-to-end remote scriptlet update for the ad-blocking extension: parses settings from remote config, schedules a WorkManager job when the feature is discoverable (and not in contingency mode), downloads scriptlets, validates them via ECDSA signature + UTF-8 checks, and stores the resulting versioned scriptlets.

Adds a new Room-backed store (ad_blocking_extension.db) with DAO/repository plumbing and DI wiring, plus new module dependencies and comprehensive unit tests for config parsing, worker scheduling/execution, updating logic, and signature validation. Also wires :ad-blocking-impl into the main app module build.

Reviewed by Cursor Bugbot for commit 87c9397. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown
Collaborator Author

CrisBarreiro commented May 14, 2026

@aibrahim- aibrahim- force-pushed the develop branch 2 times, most recently from 231d816 to d08fbc3 Compare May 14, 2026 14:06
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/ad-blocking/download-store-scriptlets branch from 5e042be to 476d1a9 Compare May 14, 2026 16:13
@CrisBarreiro CrisBarreiro marked this pull request as ready for review May 14, 2026 17:03
Comment thread ad-blocking/ad-blocking-impl/build.gradle Outdated
@CrisBarreiro CrisBarreiro marked this pull request as draft May 14, 2026 17:15
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/ad-blocking/download-store-scriptlets branch from 476d1a9 to f113371 Compare May 18, 2026 10:28
@CrisBarreiro CrisBarreiro marked this pull request as ready for review May 18, 2026 13:54
@CrisBarreiro CrisBarreiro requested a review from cmonfortep May 18, 2026 16:44
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/ad-blocking/download-store-scriptlets branch from 7213cde to 3b6160c Compare May 19, 2026 15:07
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/ad-blocking/download-store-scriptlets branch from 3b6160c to f23bf2f Compare May 21, 2026 10:41
Copy link
Copy Markdown
Contributor

@cmonfortep cmonfortep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @CrisBarreiro
Just some comments, but overall this is super well done and solid!

@Toggle.DefaultValue(DefaultFeatureValue.FALSE)
/**
* @return `true` when the feature is operational.
* When false, feature is still accessible, but not working as expected.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we clarify what means "not working as expected"?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, will do

For reference, this is the contingency path, so we'd want to still show the settings entry, but show some contingency messaging to the user


@Toggle.DefaultValue(DefaultFeatureValue.FALSE)
/**
* Kill-switch. When false, the feature is completely inaccessible.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it a kill switch? I assume this is the one we will use for phase rollout? It seems the real kill switch is self(), but this one serve us more for FF and rollout. Just to confirm this.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a kill-switch. There are 3 different relevant flags here:

  1. self -> If enabled, we can inject the scripts (provided the user settings allow us to). If disabled, we won't inject the scripts, and we'll show contingency messaging instead
  2. isDiscoverable -> This is mainly meant to be used during development, so we can hide the entry from settings. I decided to use a sub-feature because then it's easier to remove it later
  3. enabledByDefault -> This is the one we'll use for the rollout

Phase 1, we'll enable both self and isDiscoverable. We can also perform a gradual rollout on isDiscoverable for extra safety
Then, if all looks good, we'll enable enabledByDefault as a gradual rollout


@ContributesBinding(AppScope::class)
class RealScriptletDownloader @Inject constructor(
@Named("nonCaching") private val okHttpClient: Lazy<OkHttpClient>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we double check this is actually expected? Mostly to be sure the BE can hold the load on requests here. Do they offer cache policy in their response?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll only try to refresh the scripts when privacy config has changed, the feature status allows for it, and there's a new version, so we won't be calling the endpoint that frequently. I disabled caching here because we're already effectively caching the response in the database, so this would be redundant.

The only edge case I can think of is #8557 (comment), but other than that, I don't see any other issues

@CrisBarreiro CrisBarreiro force-pushed the feature/cris/ad-blocking/download-store-scriptlets branch from f23bf2f to 5142d91 Compare May 22, 2026 07:41
@CrisBarreiro CrisBarreiro force-pushed the feature/cris/ad-blocking/download-store-scriptlets branch from 5142d91 to 8a3a2a0 Compare May 22, 2026 08:12
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 87c9397. Configure here.

private fun buildJsonAdapter(): JsonAdapter<AdBlockingExtensionSettings> {
val moshi = Moshi.Builder().add(KotlinJsonAdapterFactory()).build()
return moshi.adapter(AdBlockingExtensionSettings::class.java)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Identical buildJsonAdapter() method duplicated three times

Low Severity

The buildJsonAdapter() method — creating a Moshi instance with KotlinJsonAdapterFactory() and building an AdBlockingExtensionSettings adapter — is identically duplicated in ScriptletDownloadWorker, ScriptletDownloadWorkerScheduler, and RealAdBlockingExtensionConfigProvider. Each copy allocates its own Moshi instance. Extracting this into a shared provider or companion function would reduce maintenance burden and risk of inconsistent future changes.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 87c9397. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants