Avoid blocking ServiceWorker initiated requests for Chrome MV3 (#1544)

We already avoid blocking ServiceWorker initiated requests in the MV2
extension, since each time we enabled the blocking of those we saw
major website breakage. In the future, we'd like to start blocking
those requests again, but we'll have to do so carefully.

Anyway, it's important that request blocking is consistent between MV2
and MV3 builds of the extension. So for now, let's also avoid blocking
ServiceWorker initiated requests for Chrome MV3 builds too.

Note: The way we do this, is to ignore requests with a tabId of
      -1 (aka requests not associated with a tab). This could also
      lead to other requests being ignored, but since that's
      consistent with the MV2 code-path[1] it seems the right approach.

1 - https://github.com/duckduckgo/duckduckgo-privacy-extension/blob/92222c85b04894a7f411cd844e2612b7a7d7c923/shared/js/background/before-request.es6.js#L85

integration-test/background/request-blocking.js

@@ -31,9 +31,6 @@ describe('Test request blocking', () => {
             pageRequests,
             ({ url }) => url.hostname === 'bad.third-party.site'
         )
-        const manifestVersion = await bgPage.evaluate(() => {
-            return globalThis.dbg.browserWrapper.getManifestVersion()
-        })
 
         // Initiate the test requests and wait until they have all completed.
         // Note:
@@ -66,26 +63,29 @@ describe('Test request blocking', () => {
             () => results.results // eslint-disable-line no-undef
         )
         for (const { id, category, status } of pageResults) {
+            const description = `ID: ${id}, Category: ${category}`
+
             // ServiceWorker initiated request blocking is not yet supported.
+            // TODO: Remove this condition once they are blocked again.
             if (id === 'serviceworker-fetch') {
+                expect(status).withContext(description).toEqual('loaded')
                 continue
             }
 
-            const description = `ID: ${id}, Category: ${category}`
             expect(status).withContext(description).not.toEqual('loaded')
         }
 
-        // Test the tracker reporting matches the expected outcomes.
-        const trackers = await bgPage.evaluate(async () => {
+        // Test the extension's tracker reporting matches the expected outcomes.
+        const extensionTrackers = await bgPage.evaluate(async () => {
             const currentTab = await globalThis.dbg.utils.getCurrentTab()
             return globalThis.dbg.tabManager.get({ tabId: currentTab.id }).trackers
         })
-        // TODO fix manifest v3 blocking service workers (https://app.asana.com/0/1200940319964997/1202895557146471/f)
-        // The count is higher for MV2 as we permit a load that loads more requests.
-        const count = manifestVersion === 2 ? testCount + 1 : testCount
 
-        // Test the tabs tracker objects match the expected snapshot.
-        const trackerSnapshot = {
+        const extensionTrackersCount =
+              extensionTrackers['Test Site for Tracker Blocking'].count
+        expect(extensionTrackersCount).toBeGreaterThanOrEqual(testCount)
+
+        expect(extensionTrackers).toEqual({
             'Test Site for Tracker Blocking': {
                 displayName: 'Bad Third Party Site',
                 prevalence: 0.1,
@@ -101,10 +101,9 @@ describe('Test request blocking', () => {
                         }
                     }
                 },
-                count
+                count: extensionTrackersCount
             }
-        }
-        expect(trackers).toEqual(trackerSnapshot)
+        })
 
         await page.close()
     })

package.json

@@ -81,7 +81,7 @@
     "dependencies": {
         "@duckduckgo/autofill": "github:duckduckgo/duckduckgo-autofill#5.3.1",
         "@duckduckgo/content-scope-scripts": "github:duckduckgo/content-scope-scripts#3.3.1",
-        "@duckduckgo/ddg2dnr": "github:duckduckgo/ddg2dnr#0.2.4",
+        "@duckduckgo/ddg2dnr": "github:duckduckgo/ddg2dnr#0.2.5",
         "@duckduckgo/jsbloom": "^1.0.2",
         "@duckduckgo/privacy-grade": "github:duckduckgo/privacy-grade#2.1.1",
         "@duckduckgo/privacy-reference-tests": "github:duckduckgo/privacy-reference-tests#main",

shared/js/background/declarative-net-request.js

@@ -14,6 +14,7 @@ import {
     generateDNRRule
 } from '@duckduckgo/ddg2dnr/lib/utils'
 import {
+    SERVICE_WORKER_INITIATED_ALLOWING_PRIORITY,
     USER_ALLOWLISTED_PRIORITY
 } from '@duckduckgo/ddg2dnr/lib/rulePriorities'
 
@@ -27,9 +28,10 @@ const ruleIdRangeByConfigName = {
     config: [10001, 20000]
 }
 
-// User allowlisting only requires one declarativeNetRequest rule, so hardcode
-// the rule ID here.
+// User allowlisting and the ServicerWorker initiated request exception both
+// only require one declarativeNetRequest rule, so hardcode the rule IDs here.
 export const USER_ALLOWLIST_RULE_ID = 20001
+export const SERVICE_WORKER_INITIATED_ALLOWING_RULE_ID = 20002
 
 /**
  * A dummy etag rule is saved with the declarativeNetRequest rules generated for
@@ -333,6 +335,12 @@ export async function getMatchDetails (ruleId) {
         }
     }
 
+    if (ruleId === SERVICE_WORKER_INITIATED_ALLOWING_RULE_ID) {
+        return {
+            type: 'serviceWorkerInitiatedAllowing'
+        }
+    }
+
     for (const [configName, [ruleIdStart, ruleIdEnd]]
         of Object.entries(ruleIdRangeByConfigName)) {
         if (ruleId >= ruleIdStart && ruleId <= ruleIdEnd) {
@@ -429,7 +437,36 @@ export async function updateUserDenylist () {
     await updateExtensionConfigRules()
 }
 
+/**
+ * Ensure that the allowing rule for ServiceWorker initiated requests is
+ * enabled. Since the rule needs to be restricted to matching requests not
+ * associated with a tab (tabId of -1) and so must be a session rule. Session
+ * rules don't persist past a browsing session, so must be re-added.
+ * Note: Only exported for use by unit tests, do not call manually.
+ * @return {Promise}
+ */
+export async function ensureServiceWorkerInitiatedRequestException () {
+    const removeRuleIds = [SERVICE_WORKER_INITIATED_ALLOWING_RULE_ID]
+    const addRules = [generateDNRRule({
+        id: SERVICE_WORKER_INITIATED_ALLOWING_RULE_ID,
+        priority: SERVICE_WORKER_INITIATED_ALLOWING_PRIORITY,
+        actionType: 'allow',
+        tabIds: [-1]
+    })]
+
+    // Rather than check if the rule already exists before adding it, add it and
+    // just clear the existing rule if it exists.
+    // Note: This might need to be adjusted in the future if there is a
+    //       performance impact, on the other hand, checking for the rule first
+    //       might cause a race-condition, where ServiceWorker requests are
+    //       blocked before the rule is added.
+    await chrome.declarativeNetRequest.updateSessionRules({
+        removeRuleIds, addRules
+    })
+}
+
 if (browserWrapper.getManifestVersion() === 3) {
     tdsStorage.onUpdate('config', onConfigUpdate)
     tdsStorage.onUpdate('tds', onConfigUpdate)
+    ensureServiceWorkerInitiatedRequestException()
 }

unit-test/background/declarative-net-request.js

@@ -10,13 +10,16 @@ import tabManager from '../../shared/js/background/tab-manager.es6'
 import tdsStorage from '../../shared/js/background/storage/tds.es6'
 import trackers from '../../shared/js/background/trackers.es6'
 import {
+    ensureServiceWorkerInitiatedRequestException,
     getMatchDetails,
     onConfigUpdate,
     toggleUserAllowlistDomain,
     SETTING_PREFIX,
+    SERVICE_WORKER_INITIATED_ALLOWING_RULE_ID,
     USER_ALLOWLIST_RULE_ID
 } from '../../shared/js/background/declarative-net-request'
 import {
+    SERVICE_WORKER_INITIATED_ALLOWING_PRIORITY,
     USER_ALLOWLISTED_PRIORITY
 } from '@duckduckgo/ddg2dnr/lib/rulePriorities'
 
@@ -139,15 +142,18 @@ async function updateConfiguration (configName, etag) {
 describe('declarativeNetRequest', () => {
     let updateSettingObserver
     let updateDynamicRulesObserver
+    let updateSessionRulesObserver
 
     let extensionVersion
     let settingsStorage
     let dynamicRulesByRuleId
+    let sessionRulesByRuleId
 
     beforeAll(async () => {
         extensionVersion = TEST_EXTENION_VERSIONS[0]
         settingsStorage = new Map()
         dynamicRulesByRuleId = new Map()
+        sessionRulesByRuleId = new Map()
 
         onUpdateListeners = tdsStorageStub.stub({ config }).onUpdateListeners
         onUpdateListeners.set('config', [onConfigUpdate])
@@ -167,6 +173,7 @@ describe('declarativeNetRequest', () => {
                     settingsStorage.set(name, value)
                 }
             )
+
         updateDynamicRulesObserver =
             spyOn(
                 chrome.declarativeNetRequest,
@@ -193,6 +200,32 @@ describe('declarativeNetRequest', () => {
             () => Array.from(dynamicRulesByRuleId.values())
         )
 
+        updateSessionRulesObserver =
+            spyOn(
+                chrome.declarativeNetRequest,
+                'updateSessionRules'
+            ).and.callFake(
+                ({ removeRuleIds, addRules }) => {
+                    if (removeRuleIds) {
+                        for (const id of removeRuleIds) {
+                            sessionRulesByRuleId.delete(id)
+                        }
+                    }
+                    if (addRules) {
+                        for (const rule of addRules) {
+                            if (sessionRulesByRuleId.has(rule.id)) {
+                                throw new Error('Duplicate rule ID: ' + rule.id)
+                            }
+                            sessionRulesByRuleId.set(rule.id, rule)
+                        }
+                    }
+                    return Promise.resolve()
+                }
+            )
+        spyOn(chrome.declarativeNetRequest, 'getSessionRules').and.callFake(
+            () => Array.from(sessionRulesByRuleId.values())
+        )
+
         spyOn(browserWrapper, 'getExtensionVersion').and.callFake(
             () => extensionVersion
         )
@@ -204,8 +237,10 @@ describe('declarativeNetRequest', () => {
     beforeEach(() => {
         updateSettingObserver.calls.reset()
         updateDynamicRulesObserver.calls.reset()
+        updateSessionRulesObserver.calls.reset()
         settingsStorage.clear()
         dynamicRulesByRuleId.clear()
+        sessionRulesByRuleId.clear()
     })
 
     it('Config ruleset updates', async () => {
@@ -628,6 +663,35 @@ describe('declarativeNetRequest', () => {
         })
     })
 
+    it('ServiceWorker initiated request allowing', async () => {
+        const ruleId = SERVICE_WORKER_INITIATED_ALLOWING_RULE_ID
+        const rulePriority = SERVICE_WORKER_INITIATED_ALLOWING_PRIORITY
+        const rule = {
+            id: ruleId,
+            priority: rulePriority,
+            action: { type: 'allow' },
+            condition: { tabIds: [-1] }
+        }
+
+        // The rule won't exist initially.
+        expect(updateSessionRulesObserver.calls.count()).toEqual(0)
+        expect(sessionRulesByRuleId.has(ruleId)).toBeFalse()
+
+        // Once ensureServiceWorkerInitiatedRequestException is called it should
+        // be though.
+        ensureServiceWorkerInitiatedRequestException()
+        expect(updateSessionRulesObserver.calls.count()).toEqual(1)
+        expect(sessionRulesByRuleId.has(ruleId)).toBeTrue()
+        expect(sessionRulesByRuleId.get(ruleId)).toEqual(rule)
+
+        // If ensureServiceWorkerInitiatedRequestException is called again, the
+        // rule should just be recreated.
+        ensureServiceWorkerInitiatedRequestException()
+        expect(updateSessionRulesObserver.calls.count()).toEqual(2)
+        expect(sessionRulesByRuleId.has(ruleId)).toBeTrue()
+        expect(sessionRulesByRuleId.get(ruleId)).toEqual(rule)
+    })
+
     it('getMatchDetails', async () => {
         // No rules, so no match details.
         // - Tracker blocking:
@@ -686,5 +750,10 @@ describe('declarativeNetRequest', () => {
         expect(await getMatchDetails(USER_ALLOWLIST_RULE_ID)).toEqual({
             type: 'userAllowlist'
         })
+
+        // Likewise for ServiceWorker initiated requests.
+        expect(
+            await getMatchDetails(SERVICE_WORKER_INITIATED_ALLOWING_RULE_ID)
+        ).toEqual({ type: 'serviceWorkerInitiatedAllowing' })
     })
 })

unit-test/helpers/mock-browser-api.js

@@ -29,7 +29,9 @@ globalThis.browser = {
     declarativeNetRequest: {
         isRegexSupported () { return { isSupported: true } },
         getDynamicRules () { },
-        updateDynamicRules () { }
+        getSessionRules () { },
+        updateDynamicRules () { },
+        updateSessionRules () { }
     }
 }
 globalThis.chrome = globalThis.browser