You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have tested this change locally in all supported browsers
This change will be visible to users
I have added automated tests that cover this change
I have ensured the change is gated by config
This change was covered by a ship review
This change was covered by a tech design
Any dependent config has been merged
Note
High Risk
High risk because it reworks trackerProtection’s core reporting/notification contract (replacing trackerDetected with resourceObserved) and changes how blocking/CTL/allowlist/exception decisions are derived, which can impact privacy protections and surrogate behavior across Apple platforms.
Overview trackerProtection is rescoping from doing in-script tracker classification/reporting to acting as a raw resource observer: it now emits resourceObserved events (with {url, resourceType, potentiallyBlocked, pageUrl}) and leaves final classification to native, while keeping surrogate injection as an optional, config-gated behavior.
The PR removes the trackerDetected message/schema/types, simplifies surrogateInjected payload to {url, pageUrl, surrogateName}, and updates integration/unit tests plus test configs to the new messaging and config model (splitting contentBlocking, clickToLoad, and trackerAllowlist, and moving unprotected handling to trackerProtection.exceptions). It also introduces selfGatingFeatures so trackerProtection bypasses exception-based disabling in computeEnabledFeatures, and updates unprotected-domain matching to handle single-label hostnames.
Reviewed by Cursor Bugbot for commit 7764101. Bugbot is set up for automated code reviews on this repo. Configure here.
Replace rich trackerDetected with raw resourceObserved signal:
- Emit {url, resourceType, potentiallyBlocked, pageUrl} only
- No tracker/entity/reason/prevalence classification from JS
- Native TrackerResolver is sole classification authority
surrogateInjectionEnabled gate:
- When false (shadow phase): no surrogate injection, no surrogateInjected
- _loadSurrogate throws if called with gate off (fail-fast)
- When true (post-cutover): surrogate injection active
surrogateInjected schema minimized:
- {url, pageUrl, surrogateName} — no rich classification fields
- Native maps to full DetectedRequest via TrackerResolver
tracker-resolver.js retained for surrogate matching against filtered TDS.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
injected/src/features/tracker-protection.js (303-308, 358-362, 375-380), injected/src/messages/tracker-protection/resourceObserved.notify.json (1-26), injected/src/messages/tracker-protection/surrogateInjected.notify.json (5-20), injected/src/types/tracker-protection.ts (13-58) — warning: Message contract changed from trackerDetected-centric payloads to resourceObserved + minimized surrogateInjected. This is a hard interface change with native. If script/native versions are not strictly capability-gated together, older native parsers can silently drop or reject notifications, causing tracker/surrogate behavior divergence and dashboard regressions.
injected/src/features/tracker-protection.js (64, 349-365, 395-397) — warning: surrogateInjectionEnabled now defaults to false unless explicitly set to true. This is correct for shadowing, but it introduces a compat dependency on native-side surrogate execution for all affected script breakage paths. Any config miss or partial rollout can regress sites that currently rely on JS surrogate injection.
Security Assessment
injected/src/features/tracker-protection.js (358-362, 375-380, 303-308) — info: Outgoing notifications use explicit fields only; no object spread from page-controlled inputs and no nativeData forwarding path introduced.
injected/src/features/tracker-protection.js (395-397) — info: New throw new Error(...) fail-fast path is intentional invariant protection. In hostile-page threat modeling, this is acceptable but should remain unreachable under config invariants to avoid surfacing injected-script exceptions.
Risk Level
High Risk — this PR changes tracker/surrogate messaging contracts and surrogate injection control flow in an Apple-only but web-critical feature, with strong dependency on native-side capability/version alignment.
Recommendations
Add explicit capability/version gating for resourceObserved + reduced surrogateInjected payloads (config keyed to native support), rather than relying on rollout coordination alone.
Add integration coverage that validates message decode/handling against both migration phases: surrogateInjectionEnabled=false (native surrogate path) and surrogateInjectionEnabled=true (JS surrogate path).
Add a regression test asserting surrogate-dependent pages still function when surrogateInjectionEnabled=false, to catch rollout/config mismatches before release.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
File: injected/src/features/tracker-protection.js
Line range: 344-360, 390-392; related config/schema/type changes in injected/src/messages/tracker-protection/surrogateInjected.notify.json, injected/src/messages/tracker-protection/resourceObserved.notify.json, injected/src/types/tracker-protection.ts
Severity: error surrogateInjectionEnabled now hard-gates JS surrogate injection (=== true), with no C-S-S fallback path when false. Any rollout/config/native capability mismatch can regress surrogate-dependent sites (legacy surrogate behavior is now assumed to exist outside this feature).
File: injected/src/messages/tracker-protection/resourceObserved.notify.json
Line range: 1-26; related emit sites in injected/src/features/tracker-protection.js
Severity: warning
Notification contract shifts from classified trackerDetected payloads to raw resourceObserved observations. This is a hard interface transition with native-side parser/logic coupling; mixed app/script versions can silently degrade behavior (missed classification, dropped fields, dashboard drift).
File: injected/src/features/tracker-protection.js
Line range: 321, 370-375
Severity: warning
Scripts are now always reported via resourceObserved, including non-tracker and first-party URLs. Combined with no seen-URL guard in _checkAndBlock, repeated dynamic <script> insertions can significantly increase IPC volume and native processing on script-heavy pages.
Security Assessment
File: injected/src/features/tracker-protection.js
Line range: 370-375
Severity: warning
Hostile pages can force high-volume resourceObserved emissions by repeatedly adding script elements (including repeated same-URL insertions, since _checkAndBlock does not dedupe by _seenUrls before notify). This creates an IPC amplification/DoS surface against native classification.
File: injected/src/features/tracker-protection.js
Line range: 166, 222, 276
Severity: warning potentiallyBlocked is hardcoded true for XHR/image error paths. If native logic ever treats this hint as authoritative (instead of advisory), a page can synthesize errors to bias block decisions/metrics.
File: injected/src/features/tracker-protection.js
Line range: 298-303, 353-357, 370-375
Severity: info
Outgoing messages use explicit fields (no spread of page-controlled objects), so this change does not introduce a nativeData forwarding path.
Risk Level
High Risk — this PR changes tracker/surrogate messaging semantics and surrogate execution ownership in a web-critical interception path, with strong native-version/capability coupling and new high-volume observation behavior.
Recommendations
Add explicit capability negotiation/version gating for resourceObserved and the reduced surrogateInjected payload before enabling this path broadly.
Keep a controlled fallback path for surrogate execution when native capability is absent or disabled (or force-disable the feature via config in that state).
Add throttling/dedup strategy for script-origin resourceObserved messages (per URL + time window or per-node lifecycle) to bound hostile-page amplification.
Add integration coverage for mixed-version migration states (surrogateInjectionEnabled true/false + native capability present/absent) and for adversarial script/XHR/image error spam scenarios.
…tected
The tracker-protection feature now sends 'resourceObserved' (raw observation
data: url, resourceType, potentiallyBlocked, pageUrl) instead of the classified
'trackerDetected' message. The 'surrogateInjected' schema was also minimized
to {url, pageUrl, surrogateName}.
Update all integration test assertions to match the new message contract.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
injected/src/features/tracker-protection.js (init, _checkAndBlock, _loadSurrogate), severity: error
The new gate this.getFeatureSetting('surrogateInjectionEnabled') === true makes surrogate injection opt-in with no backward-compatible default. If the key is absent, surrogates are fully disabled. This is a high breakage risk because blocked scripts that previously relied on surrogate replacement will stop being shimmed, matching the "config-gated rollback" and "third-party script compatibility" concerns.
injected/integration-test/tracker-protection.spec.js (surrogate assertions), severity: warning
Tests still assert surrogate behavior but fixture configs do not declare surrogateInjectionEnabled. That makes test intent ambiguous and can mask rollout-state regressions (shadow vs active injection) depending on external config assumptions.
Security Assessment
injected/src/features/tracker-protection.js (resourceObserved payload path), severity: warning potentiallyBlocked is now emitted as a JS-side hint, and classification moved native-side (good). To preserve trust boundaries, native must treat this field as advisory only and recompute policy from URL/resourceType/top-level context. If native ever treats it as authoritative, a page can influence telemetry/classification inputs by causing controlled request failures.
No direct vulnerabilities found in this diff related to message-secret/origin validation, nativeData leakage, dynamic code execution, or new cross-origin messaging paths.
Risk Level
High Risk — this PR changes tracker/surrogate runtime behavior and message contracts in a security-sensitive injected feature; the new surrogate gate default can cause real-site compatibility regressions at scale if not explicitly configured.
Recommendations
Preserve legacy behavior by defaulting surrogate injection to enabled unless explicitly disabled, or explicitly set surrogateInjectionEnabled: true in all active configs before merge.
Add split integration coverage for both gate states:
surrogateInjectionEnabled: false => no surrogate execution + no surrogateInjected.
Add/confirm native-side tests asserting resourceObserved.potentiallyBlocked is non-authoritative and recomputed server/native-side.
Alternative rollout option: keep injection active but add a separate shadow-only telemetry mode flag for resourceObserved, instead of coupling telemetry migration to injection gating.
- Add surrogateInjectionEnabled: true to configs that back surrogate
tests (tracker-protection.json, ctl-enabled.json, real-surrogates.json).
Without this, the feature gates off surrogate injection and
surrogateInjected is never emitted, causing 5s timeouts.
- Fix fetch interception tests to expect potentiallyBlocked: false.
The fetch wrapper always reports false since it uses _reportResource
(observation-only), not _checkAndBlock. Native does classification.
MutationObserver img path uses _reportResource with potentiallyBlocked=false
(observation-only, no classification). Test incorrectly expected true.
All 24 tracker-protection integration tests now pass locally.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
injected/src/features/tracker-protection.js (59, 344-364) — warning surrogateInjectionEnabled is strict opt-in (=== true) and suppresses surrogate injection when absent. This PR already had to patch test configs to add the key, which indicates real breakage potential if any production/experiment config path omits it. In that state, matching surrogate candidates are silently skipped, which can regress sites that depend on surrogate behavior.
injected/src/features/tracker-protection.js (329-375) — info potentiallyBlocked semantics are path-dependent (fetch always false, XHR error/image error often true, DOM-added image often false). If native treats this as anything stronger than a hint, this can cause inconsistent behavior across resource types. Current code comments say non-authoritative, but contract tests should enforce that assumption.
Security Assessment
injected/src/features/tracker-protection.js (390-392) — warning throw new Error(...) uses uncaptured globalThis.Error in a hostile page context. Per injected threat model, new security-relevant global usage should come from captured-globals.js to avoid page tampering of constructors/prototypes.
Risk Level
High Risk — this changes core resource interception behavior and native-facing notification contracts in tracker-protection, with direct impact on surrogate execution and classification telemetry.
Recommendations
Import and use captured Error from captured-globals.js for the gate-violation throw path.
Add a rollout guard test for missing surrogateInjectionEnabled in real config fixtures (explicitly verifying expected behavior and no silent surrogate regression).
Tighten resourceObserved schema by constraining resourceType to an enum and add a contract test that native treats potentiallyBlocked as advisory only.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
injected/integration-test/tracker-protection.spec.js (387-400) — error: The updated assertion expects potentiallyBlocked === true for DOM-appended <img> resources, but the new observer path in tracker-protection reports DOM-added images via _reportResource(..., 'image', false). This is a deterministic mismatch and currently fails the suite (tracker-protection: detects tracker from DOM-appended img element).
injected/src/features/tracker-protection.js (329-337, 369-375) — warning: The new raw-observer flow now emits resourceObserved for script URLs even when they are first-party/non-tracker (previous code path filtered many of these via _reportThirdPartyRequest host checks). On script-heavy pages this can materially increase native-message volume and processing overhead, which is a compatibility/perf risk.
Security Assessment
injected/src/features/tracker-protection.js (292-304, 369-375) — warning: resourceObserved now forwards raw URLs for a much broader set of resources to native. A hostile page can generate high-cardinality resource URLs (query-string churn) to spam the native messaging channel. This is a practical message-flood vector at the page→native trust boundary; no local throttling/rate-limit is present in this layer.
No direct regressions found in message-secret/origin validation paths (message bridge/transports were not changed in this PR).
Risk Level
High Risk — this PR changes core interception/reporting behavior and message contracts in tracker-protection, and currently contains at least one deterministic integration-test failure plus a broadened page→native reporting surface.
Recommendations
Fix the failing DOM-image expectation in tracker-protection.spec.js (or adjust runtime behavior if true is intended for that path), then rerun tracker-protection.spec.js end-to-end.
Add an explicit contract test that validates per-resource potentiallyBlocked semantics across all ingestion paths (MutationObserver img, Image.src error, fetch, XHR).
Add guardrails for raw observation volume before native handoff (e.g., third-party prefilter, normalization-based dedupe, or bounded rate per document).
Add an integration assertion that resourceObserved payload never includes extra fields (especially nativeData) to protect message schema boundaries during future refactors.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
injected/src/features/tracker-protection.js68-70, 85-90, 99-105 — error
The feature now reads raw values from this.bundledConfig.features.* instead of resolved feature settings (getFeatureSetting*). This bypasses conditional patching/domain overrides, so runtime behavior can diverge from remote-config intent (for example allowlist/CTL/content-blocking decisions no longer reflecting resolved per-site config).
injected/src/utils.js876-879, 932, 940 — warning trackerProtection is now both platform-specific and self-gating, so exception-based disabling is bypassed in computeEnabledFeatures. This keeps interception active on exception/allowlisted/broken sites, which increases risk of site breakage on domains that were explicitly exempted to avoid compatibility issues.
Security Assessment
injected/src/features/tracker-protection.js85-90 — warning Object.entries/Object.fromEntries are used directly in hostile page context when deriving allowlist data. This violates captured-global hygiene; a page that tampers with Object methods can alter/poison this transformation and influence blocking/surrogate behavior.
injected/src/utils.js876-879, 932, 940 and injected/src/features/tracker-protection.js68-70, 99-100 — error
Config trust boundary is weakened: exception-based disable no longer removes the feature from active interception, and blocking state is taken from raw config paths. This reduces ability to remotely and safely disable behavior on problematic domains (a key rollback/safety control).
Risk Level
High Risk — this PR changes feature enablement semantics and runtime interception behavior for tracker protection, affecting whether prototype/network interception runs on sites that were previously exempted.
Recommendations
Restore resolved-config reads for tracker-protection runtime decisions (getFeatureSetting* / resolved featureSettings) or add equivalent conditional-change application before use.
Keep self-gating only if needed for telemetry, but add a strict mode split: no prototype interception on exception/allowlisted/broken domains unless an explicit reporting-only flag is enabled.
Replace direct Object.entries/Object.fromEntries with captured equivalents (and add objectFromEntries capture) to prevent page tampering influence.
Add regression tests for: conditionalChanges on trackerAllowlist, exception/allowlisted domains, and verification that interception is truly disabled where rollback expects it.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
File: injected/src/utils.js + injected/src/features/tracker-protection.js; line range: ~919-940 and ~98-100/~355-393; severity: error; trackerProtection is now forced active via platformSpecificFeatures + selfGatingFeatures, but _isUnprotectedDomain only checks trackerProtection.exceptions and site.allowlisted (not site.isBroken / unprotectedTemporary). On globally unprotected sites, _checkAndBlock() can still classify as blocked and inject surrogates, which is a direct regression risk for sites that were globally exempted to avoid breakage.
File: injected/src/features/tracker-protection.js; line range: ~161-243; severity: warning; the feature still monkey-patches XMLHttpRequest, fetch, and Image.prototype.src with direct replacements (no DDGProxy / wrapToString masking). This was pre-existing, but scope is now broader due self-gating, so anti-tamper scripts are more likely to detect and branch into incompatible paths even on domains intended to be “unprotected”.
File: injected/src/features/tracker-protection.js; line range: ~318-329 and call sites ~192-307; severity: warning; resourceObserved is now emitted for broad resource classes without first/third-party filtering. This increases telemetry volume significantly and risks page-performance regressions on asset-heavy sites.
Security Assessment
File: injected/src/features/tracker-protection.js; line range: ~318-329 and ~395-401; severity: error; hostile pages can generate unbounded unique URLs (fetch/XHR/img) to force high-rate resourceObserved emissions. Exact-URL dedupe is bypassable with cache-busting params, creating a practical message-channel DoS vector against native processing.
File: injected/src/features/tracker-protection.js; line range: ~84-90; severity: warning; newly added Object.entries/Object.fromEntries usage is not from captured globals. In this threat model, page-tampered globals can alter behavior or throw during init(), producing bypass/DoS conditions in feature setup.
Risk Level
High Risk — this PR changes feature enablement semantics and tracker-protection interception/reporting behavior in ways that can affect globally exempted sites and introduces a high-volume message path exploitable by hostile pages.
Recommendations
Add global-unprotected gating in TrackerProtection.init() (site.isBroken in addition to site.allowlisted/exceptions) before any blocking/surrogate path, and add an integration test covering unprotectedTemporary + surrogate rule.
Add backpressure to resourceObserved (per-page cap, rate limiter, and/or first/third-party prefilter) to prevent message-channel DoS.
Move newly added global operations to captured references (at minimum entries/fromEntries path) to avoid page-tampering effects during init.
Alternative approach: if reporting on exempted sites is required, keep passive reporting but disable prototype interception paths on exempted domains; this preserves observability while minimizing compatibility and detectability risk.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
injected/src/utils.jslines 876-879 and lines 932-940 — severity: error trackerProtection is now both platformSpecific and selfGating, so computeEnabledFeatures() no longer applies exception-based disabling for this feature. In practice this means tracker-protection still initializes on globally exempt pages (site.allowlisted / site.isBroken) and still patches runtime APIs (fetch, XMLHttpRequest, Image.src). That weakens the existing breakage escape hatch and can reintroduce compatibility issues on domains that were intentionally exempted.
injected/src/features/tracker-protection.jslines 98-100 — severity: error
Internal exception handling now uses only trackerProtection.exceptions plus args.site.allowlisted, but does not include args.site.isBroken (unprotectedTemporary). Domains temporarily unprotected via global breakage controls can be treated as protected for tracker-protection behavior (including surrogate path decisions), which is a behavior regression versus previous global exemption semantics.
Security Assessment
injected/src/features/tracker-protection.jslines 84-90 — severity: error
New allowlist normalization uses uncaptured globals: Object.entries and Object.fromEntries. In this hostile page environment, page scripts can tamper with these before init(), influencing allowlist materialization and potentially altering potentiallyBlocked / surrogate decisions. This violates the repo’s captured-globals hardening model.
injected/src/utils.jslines 876-879 and lines 932-940 — severity: warning
The self-gating bypass reduces configuration trust boundaries: a global exemption is no longer sufficient to stop tracker-protection runtime hooks. For emergency rollback scenarios, this increases blast radius when a site-specific compatibility issue is discovered.
Risk Level
High Risk — this PR changes tracker-protection runtime activation semantics in computeEnabledFeatures() and exception handling in a way that can re-enable injected API interception on pages previously protected by global breakage controls.
Recommendations
Restore global breakage semantics for tracker-protection interception: either remove tracker-protection from selfGatingFeatures, or keep self-gating but hard-stop _setupInterception() when args.site.isBroken || args.site.allowlisted.
Include args.site.isBroken in _isUnprotectedDomain computation (or explicitly map unprotectedTemporary into tracker-protection exceptions) to preserve existing unprotected-domain guarantees.
Replace Object.entries/Object.fromEntries usage with captured equivalents (objectEntries) and a local reducer; if fromEntries is needed, add a captured reference in captured-globals.js.
Add regression tests for:
globally exempt (unprotectedTemporary) domains not activating surrogate/blocking logic, and
hostile page tampering of Object.entries before init() not changing allowlist behavior.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
injected/src/utils.js (computeEnabledFeatures and self-gating additions around isSelfGatingFeature, platformSpecificFeatures, selfGatingFeatures) + injected/src/features/tracker-protection.js (init() + interception setup): error trackerProtection now bypasses exception-based disabling and still installs invasive prototype hooks (XMLHttpRequest.prototype.open/send, window.fetch, Image.prototype.src) on exception/allowlisted/unprotected domains. This removes the previous compatibility escape hatch for domains explicitly excepted due breakage and can reintroduce site regressions even when blocking is suppressed.
injected/src/features/tracker-protection.js (_reportResource() and _checkAndBlock() notify paths): warning
The new resourceObserved flow reports all intercepted HTTP(S) resources (including first-party resources) rather than only tracker/third-party classifications. On heavy pages this can substantially increase observer + messaging volume and cause perf-sensitive compatibility regressions (main-thread + native bridge pressure).
Security Assessment
injected/src/features/tracker-protection.js (allowlist normalization using Object.entries/Object.fromEntries in init()): error
These are uncaptured globals in a hostile page context. A page can monkeypatch them before init() to throw or return attacker-controlled structures, causing fail-open behavior (feature init failure) or altered allowlist semantics. This is a direct bypass surface per captured-global hygiene requirements.
Risk Level
High Risk — this PR changes feature enablement semantics and runtime interception scope on previously excepted domains, while also introducing new uncaptured-global dependency in security-sensitive initialization.
Recommendations
Gate interception installation on exception/allowlist state, not only blocking state: either fully skip prototype patching on excepted domains, or split into a passive/non-invasive reporting mode that avoids monkeypatching (fetch/XHR/Image descriptor) there.
Replace new allowlist normalization calls with captured-safe references (or a plain for...in + hasOwnProperty path using captured primitives) to remove tamperable Object.entries/Object.fromEntries dependency.
Add integration coverage for exception/allowlisted domains asserting the intended contract explicitly: whether API prototypes should remain untouched, plus perf/message-volume guardrails for high-resource pages.
Alternative if always-on reporting is required: move raw resource observation to a native/network-layer signal path and keep C-S-S script-side hooks limited to surrogate injection paths only.
Delete trackerDetected.notify.json (no callers remain after refactor)
and regenerate tracker-protection.ts to drop the dead
TrackerDetectedNotification type from the notifications union.
Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
injected/src/features/tracker-protection.js (84-90) — error allowlist is now read directly from this.bundledConfig.features.trackerAllowlist.settings.allowlistedTrackers and normalized via Object.fromEntries(...), instead of going through getFeatureSetting(...). That bypasses ConfigFeature conditional patching (conditionalChanges/domain-context matching), so domain- or context-specific allowlist overrides may not apply. Result: trackers expected to be allowlisted can become potentiallyBlocked=true and trigger surrogate behavior unexpectedly (site-breakage risk).
injected/src/features/tracker-protection.js (161-243, 188-189, 214, 222) — warning
The feature still monkey-patches XMLHttpRequest.prototype, window.fetch, and Image.prototype.src, but teardown was removed (previous destroy() unpatch logic is gone). If the feature is ever re-initialized in the same document lifecycle (re-injection/hot-update/test harness), wrappers can stack and duplicate listeners/notifications, which can alter behavior and increase breakage/perf risk.
Security Assessment
injected/src/features/tracker-protection.js (84-90) — warning
New allowlist normalization uses uncaptured globals (Object.entries, Object.fromEntries) in init(). In the hostile-page model, relying on live globals after page script execution can be subverted by prototype/global tampering, affecting classification inputs. This is not a bridge/origin bypass, but it weakens robustness of a security-sensitive decision path.
injected/src/features/tracker-protection.js (161-243) — info
No direct new postMessage/origin-validation/message-secret regression found in this PR. Main security concern remains lifecycle hardening of prototype patches (see warning above), not trust-boundary checks.
Risk Level
High Risk — this PR changes core tracker-protection interception/reporting flow and feature-enablement semantics, and introduces a likely allowlist-application regression that can directly impact blocking/surrogate decisions.
Recommendations
Replace direct config access with getFeatureSetting('allowlistedTrackers', 'trackerAllowlist') (or equivalent helper) before normalization so conditional/domain-scoped allowlist patches are honored.
Add regression tests for conditional allowlist application (trackerAllowlist.settings.conditionalChanges) proving different domains receive different allowlist outcomes.
Reintroduce explicit unpatch/cleanup (destroy) or make interception setup idempotent with guards to prevent wrapper stacking on re-init.
For hostile-context hardening, avoid newly introduced live-global dependence in this path (use captured references or a safer normalization path).
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
File: injected/src/utils.js
Line range: 876-878, 919-940
Severity: error trackerProtection is now both platform-specific and self-gating, so it is still loaded when site.isBroken / user allowlist disables normal protections. This defeats the existing breakage rollback path and keeps API interception active (fetch, XMLHttpRequest, Image.src) on pages that were expected to run with protections off.
File: injected/src/features/tracker-protection.js
Line range: 68-107, 339-399
Severity: error
The new gating only checks contentBlocking.state + trackerProtection.exceptions/site.allowlisted; it does not include site.isBroken (unprotectedTemporary). On broken domains, the feature can still classify as potentiallyBlocked and perform surrogate-injection decisions, which is a direct web-compat regression for emergency unprotect flows.
Security Assessment
File: injected/src/utils.js, injected/src/features/tracker-protection.js
Line range: 876-878, 919-940, 324-329, 392-397
Severity: error
Because tracker protection now remains active on globally disabled sites, resourceObserved telemetry (url, pageUrl) continues to be emitted on allowlisted/broken domains. This weakens the trust boundary of the "disabled/unprotected" mode and expands collection scope on pages users/operators explicitly exempted.
File: injected/src/features/tracker-protection.js
Line range: 85-90
Severity: warning
New allowlist normalization uses uncaptured globals (Object.entries, Object.fromEntries) in hostile page context. A page can tamper these before init() to alter allowlist parsing or throw during setup, creating a bypass/fail-open surface. This should use captured references from captured-globals.js.
Risk Level
High Risk — this PR changes feature enablement semantics so tracker-protection can continue patching and reporting on domains intended to be globally unprotected/allowlisted, which is both a compatibility rollback risk and a security/privacy-boundary regression.
Recommendations
Reintroduce global kill-switch behavior for tracker protection on site.isBroken and user allowlist, or explicitly split into a separate report-only path that does not patch browser APIs.
If the intent is report-only on exempted sites, keep resourceObserved but skip _setupXHRInterception, _setupFetchInterception, _setupImageSrcInterception, and surrogate logic entirely.
Replace new Object.entries/Object.fromEntries usage with captured equivalents (or add captured fromEntries) to avoid page-tampering bypass.
Add explicit tests for site.isBroken and user-allowlisted cases validating no blocking/surrogate behavior and validating intended reporting contract.
…rules
When CTL is disabled and a matched rule action starts with `block-ctl-`,
do not report potentiallyBlocked: true. This prevents the native mapper
from misclassifying CTL-only requests as blocked when CTL is inactive.
Updated integration tests to expect potentiallyBlocked: false for
CTL-disabled scenarios and added a control test confirming non-CTL rules
remain potentiallyBlocked: true.
Made-with: Cursor
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
injected/src/utils.js (around platformSpecificFeatures, includes trackerProtection) — error trackerProtection is now treated as platform-specific, so it is loaded even when isGloballyDisabled(args) is true (site.allowlisted or site.isBroken). In this PR, TrackerProtection only maps site.allowlisted into _isUnprotectedDomain, not site.isBroken, so globally-disabled/broken sites can still run tracker-protection interception and potentially block/inject surrogates. This is a global-disable behavior regression.
injected/src/features/tracker-protection.js (_ctlEnabled assignment + block-ctl-* handling in _checkAndBlock) — warning
CTL enablement now derives from clickToLoad.state only. Per-domain CTL exceptions are not considered, so domains excluded via clickToLoad.exceptions can still be treated as CTL-enabled and execute CTL surrogate paths. That can alter site behavior on domains intentionally excepted from CTL.
Security Assessment
injected/src/features/tracker-protection.js (allowlist extraction using Object.entries/Object.fromEntries) — warning
This new path uses uncaptured globals in hostile page context. A page can monkeypatch Object.entries/Object.fromEntries before init() and influence allowlist materialization, changing downstream blocking/surrogate decisions. Per capture-hygiene requirements, this should use captured references.
Risk Level
High Risk — this PR changes feature-gating semantics and tracker-protection execution scope; it can affect blocking behavior on globally-disabled/broken or CTL-exception domains, and introduces a new uncaptured-global path in a security-sensitive decision flow.
Recommendations
Gate trackerProtection correctly under global-disable semantics: either do not load it in platformSpecificFeatures for globally-disabled pages, or force non-blocking mode when site.isBroken || site.allowlisted.
Compute _ctlEnabled with exception awareness (state + hostname exception check), not state alone.
Replace uncaptured Object.entries/Object.fromEntries usage with captured-safe equivalents in allowlist extraction.
Add regression tests for:
site.isBroken/unprotectedTemporary domains (assert no blocking/surrogate behavior).
clickToLoad.exceptions domains (assert CTL surrogate paths are suppressed).
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
injected/src/utils.js (lines 876-879, 919-933, 940-955) — error: trackerProtection now bypasses exception-based disabling (isSelfGatingFeature) and is also added to platformSpecificFeatures (always loaded when isGloballyDisabled). This means domains intentionally exempted for breakage (exceptions, allowlisted, broken) still execute tracker-protection interception paths, expanding prototype patching/observer behavior to sites that previously had a hard disable path.
injected/src/features/tracker-protection.js (lines 98-107, 114-244, 339-399) — warning: self-gating keeps the feature active on unprotected/allowlisted domains and still installs interception hooks (MutationObserver, XMLHttpRequest, fetch, Image.src), only changing potentiallyBlocked semantics. This can reintroduce compatibility failures on domains where remote exceptions were expected to fully suppress feature-side behavior.
Security Assessment
injected/src/utils.js (lines 876-879, 919-933) + injected/src/features/tracker-protection.js (lines 318-330, 393-399) — warning: configuration trust/rollback scope is reduced. Per-domain exceptions no longer stop resource interception or resourceObserved URL/pageUrl reporting, so rollback now effectively requires broader toggles (e.g., contentBlocking) rather than targeted per-domain disablement.
No direct regressions found in message-bridge secret/origin checks, nativeData handling, or new dynamic-code/network-exfil patterns in this diff.
Risk Level
High Risk — this change alters global feature gating and interception behavior in a high-sensitivity area (resource/API interception) and reduces the precision of remote breakage rollback controls.
Recommendations
Reintroduce a hard-disable path before _setupInterception() for exception/allowlisted domains, or gate this new behavior behind a dedicated remote flag (e.g., observeOnExceptions).
Avoid loading trackerProtection from platformSpecificFeatures by default; alternatively require explicit remote opt-in for globally-disabled sites.
Add integration tests that assert exception/allowlisted domains can fully avoid interception hooks (fetch, XMLHttpRequest, Image.src) when rollback is enabled.
Add a rollback-contract test proving trackerProtection per-domain exceptions (or equivalent new setting) can stop resourceObserved emission without disabling contentBlocking globally.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
injected/src/utils.js (computeEnabledFeatures around L876-L879, platformSpecificFeatures around L919-L933) + injected/src/features/tracker-protection.js (L98-L101) — error trackerProtection is now forced into enabledFeatures even on globally unprotected/broken sites, but _isUnprotectedDomain only checks trackerProtection.exceptions and site.allowlisted. It does not include site.isBroken (unprotectedTemporary). Result: on a globally unprotected site, tracker-protection can still mark requests as blocked and inject surrogates, which is a regression from the previous global-disable behavior and can reintroduce breakage on emergency-disabled domains.
injected/src/features/tracker-protection.js (L324-L329, L392-L398) — warning resourceObserved now emits for every seen HTTP(S) resource (including same-party/non-tracker paths), whereas previous reporting path filtered non-tracker/same-host requests. This materially increases message volume from page -> native and creates a performance compatibility risk on resource-heavy sites.
Security Assessment
injected/src/utils.js (L876-L879) + injected/src/features/tracker-protection.js (L98-L101) — error
This weakens config rollback guarantees: global unprotected state (site.isBroken) no longer reliably suppresses tracker-protection blocking behavior. In practice this means remote emergency-disable (unprotectedTemporary) may not fully neutralize this protection path, which is a configuration trust/safety issue.
injected/src/features/tracker-protection.js (L84-L90) — warning
New allowlist normalization uses uncaptured globals (Object.entries, Object.fromEntries) in hostile page context. Per injected threat model, this should use captured references (or equivalent safe construction) to avoid prototype/global tampering affecting classification inputs.
Risk Level
High Risk — this PR changes tracker-protection enablement semantics and runtime reporting/blocking behavior across all Apple page loads, with a potential emergency-disable bypass and increased runtime message pressure.
Recommendations
Treat site.isBroken as unprotected inside trackerProtection self-gating (or explicitly force observation-only mode with potentiallyBlocked=false + no surrogate injection when site.isBroken || site.allowlisted).
Add an integration test that places the domain in global unprotectedTemporary (not feature exceptions) and asserts no blocking/surrogate injection occurs.
Rework allowlist normalization to use captured globals from captured-globals.js (add captured helper for fromEntries if needed) instead of live Object.* reads.
Add a perf-focused integration test (or assertion bound) for high-resource pages to validate resourceObserved throughput does not regress page responsiveness/native queue handling.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
injected/src/utils.js (computeEnabledFeatures self-gating branch), injected/src/features/tracker-protection.js (_isUnprotectedDomain + _setupInterception path)
Severity: error trackerProtection now bypasses exception-based disabling and still installs invasive API hooks (XMLHttpRequest.prototype.open/send, window.fetch, Image.prototype.src) on exception/allowlisted domains. This removes the per-domain “hard off” behavior those exceptions previously provided and can re-introduce breakage on sites that were explicitly exempted.
injected/src/features/tracker-protection.js (_blockingEnabled from contentBlocking.state, allowlist read from trackerAllowlist.settings.allowlistedTrackers, _ctlEnabled from clickToLoad.state)
Severity: warning
The feature now hard-depends on a new config shape with no compatibility guard. During staggered app/config rollout, missing keys silently change behavior (for example, disable blocking or drop allowlist handling), which can cause real-site behavior drift.
Security Assessment
injected/src/utils.js (isSelfGatingFeature path), injected/src/features/tracker-protection.js (_setupInterception still runs when domain is excepted/allowlisted)
Severity: warning
This is a configuration-trust regression: exception lists no longer fully disable tracker-protection hook installation. In practice, remote config loses per-domain ability to remove this injected attack surface; only global state toggles remain.
injected/src/features/tracker-protection.js (state/allowlist source migration lines)
Severity: warning
If contentBlocking/trackerAllowlist/clickToLoad keys are absent or malformed, protection behavior downgrades silently. In a hostile environment, silent fail-open/fail-closed transitions without explicit validation are risky because they can reduce privacy coverage unexpectedly.
Risk Level
High Risk — this PR changes runtime interception behavior, feature enablement semantics, and messaging contracts for a platform-wide injected feature that monkey-patches core web APIs.
Recommendations
Reintroduce a true per-domain kill switch for invasive hooks.
Option A: skip _setupInterception() entirely when domain is excepted/allowlisted.
Option B: keep reporting enabled but split “report-only observer” from prototype patching so exceptions can disable monkey-patching specifically.
Add explicit config-contract validation for the new key paths before enabling behavior.
Option A: require all required keys (contentBlocking, trackerAllowlist, clickToLoad) and abort with a single deterministic mode if contract is incomplete.
Option B: keep a temporary migration shim with telemetry until all supported app versions ship the new schema.
The reason will be displayed to describe this comment to others. Learn more.
Web Compatibility Assessment
injected/src/features/tracker-protection.js (around 100-101), injected/src/utils.js (around 876-879, 919-940) — severity: error trackerProtection is now both selfGating and platformSpecific, so it continues to load on globally unprotected sites. However, internal unprotected logic only checks trackerProtection.exceptions and site.allowlisted, not site.isBroken (unprotectedTemporary). This can re-enable tracker blocking/surrogate behavior on sites that were globally unprotected for breakage mitigation, creating a high-likelihood web compatibility regression.
Security Assessment
injected/src/features/tracker-protection.js (around 100-101), injected/src/utils.js (around 876-879, 919-940) — severity: warning
This weakens configuration trust guarantees: global rollback/unprotection (unprotectedTemporary) no longer reliably suppresses enforcement side effects for this feature. From a threat-model perspective, that reduces the safety valve expected for rapid breakage/security rollback and may leave interception behavior active where policy intends full disable.
Risk Level
High Risk — this PR changes core enablement semantics and runtime interception behavior for a platform-level feature injected into every page, and currently appears to bypass a global unprotection control path.
Recommendations
Treat global unprotection as authoritative inside trackerProtection by including this.args?.site?.isBroken in the unprotected check before computing potentiallyBlocked and surrogate injection.
Add an integration test where unprotectedTemporary contains the top-level domain and assert no effective blocking/surrogate injection signals (potentiallyBlocked=false, no surrogateInjected) while still allowing passive observation if that is intended.
Add a unit test covering computeEnabledFeatures + isFeatureBroken interaction for selfGatingFeatures that verifies expected behavior on site.isBroken and allowlisted domains.
Sent by Cursor Automation: Web compat and sec
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
semver-majorBreaking change — triggers major version bump
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
laghee
added
semver-minor
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
github-actions
bot
added
semver-patch
Asana Task/Github Issue: https://app.asana.com/1/137249556945/project/1163321984198618/task/1213882745212509?focus=true
Description
Testing Steps
Paired with Apple PR
Checklist
Please tick all that apply:
Note
High Risk
High risk because it reworks
trackerProtection’s core reporting/notification contract (replacingtrackerDetectedwithresourceObserved) and changes how blocking/CTL/allowlist/exception decisions are derived, which can impact privacy protections and surrogate behavior across Apple platforms.Overview
trackerProtectionis rescoping from doing in-script tracker classification/reporting to acting as a raw resource observer: it now emitsresourceObservedevents (with{url, resourceType, potentiallyBlocked, pageUrl}) and leaves final classification to native, while keeping surrogate injection as an optional, config-gated behavior.The PR removes the
trackerDetectedmessage/schema/types, simplifiessurrogateInjectedpayload to{url, pageUrl, surrogateName}, and updates integration/unit tests plus test configs to the new messaging and config model (splittingcontentBlocking,clickToLoad, andtrackerAllowlist, and moving unprotected handling totrackerProtection.exceptions). It also introducesselfGatingFeaturessotrackerProtectionbypasses exception-based disabling incomputeEnabledFeatures, and updates unprotected-domain matching to handle single-label hostnames.Reviewed by Cursor Bugbot for commit 7764101. Bugbot is set up for automated code reviews on this repo. Configure here.