[PROTOTYPE] web interference detection #2048
Conversation
Temporary Branch UpdateThe temporary branch has been updated with the latest changes. Below are the details:
Please use the above install command to update to the latest version. |
❌ Deploy Preview for content-scope-scripts failed.
|
[Beta] Generated file diffTime updated: Thu, 06 Nov 2025 16:43:18 GMT Android
File has changed AppleFile has changed IntegrationFile has changed WindowsFile has changed |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| if (!this.root) { | ||
| setTimeout(() => this.start(), 500); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Bug: Memory Leak: Unstored Timeout Causes Repeated Retries
Memory leak: When findRoot() returns null, setTimeout schedules a retry but the timeout ID is not stored. If stop() is called before the root is found, the timeout will still fire and call start() again, potentially creating new intervals that can never be cleaned up. The timeout should be stored in a class property and cleared in the stop() method.
| const { status, selectors, textPatterns, textSources } = statusConfig; | ||
| const hasMatch = matchesSelectors(selectors) || matchesTextPatterns(document.body, textPatterns, textSources); | ||
| return hasMatch ? status : null; | ||
| })?.status || null |
There was a problem hiding this comment.
Bug: Fragile find() callback returning non-boolean value
Incorrect find() callback logic: The callback returns hasMatch ? status : null where it should return a boolean. While this works by accident (returning a truthy string causes find() to stop), the logic is confusing and fragile. The callback should return hasMatch directly, making the code: statusSelectors.find((statusConfig) => { ... return hasMatch; })?.status || null.
|
|
||
| cleanup() { | ||
| this.activeDetections.forEach((detection) => detection.stop()); | ||
| } |
There was a problem hiding this comment.
Bug: Memory leak from uncleared detections array
The cleanup() method calls stop() on all activeDetections but doesn't clear the array afterward. This means the array retains references to stopped detections, preventing garbage collection and causing the array to grow unbounded across multiple detect() calls. Should add this.activeDetections = []; after the forEach.
| */ | ||
|
|
||
| /** | ||
| * @typedef {'cloudflare' | 'hcaptcha'} VendorName |
There was a problem hiding this comment.
Can all of these just be defined with the code that makes it rather than a 'types' file? Often these can just be exported js constants with lighterweight annotations or none.
There was a problem hiding this comment.
can be any of that, not holding any strong preference; jsdoc is pretty hard to read also 😅
| * @param {string[]} [selectors] | ||
| * @returns {boolean} | ||
| */ | ||
| export function checkSelectors(selectors) { |
There was a problem hiding this comment.
I suspect a bunch of these already exist (either as part of features or already in utils). Ask AI if it can find any :).
querySelectors are usually faster just running once and concatting to find any of them.
| * @returns {TypeDetectionResult} | ||
| */ | ||
| detect() { | ||
| const root = this.findRoot(); |
There was a problem hiding this comment.
It seems like this whole class is generic enough to work for any ad detection, is that the case?
There was a problem hiding this comment.
It did cross my mind but then I'd have to "make it work" from the beginning for PIR as well, where we do not actually need this level of functionality, hence the 3rd goal in the description "The service architecture intentionally avoids over-constraining detector implementations. We will revisit this based on what patterns emerge."
Now that @jdorweiler is taking over and he will need the more complex detections, it may be wise to try make it generic enough and have a single detector or a single base detector that can be extended with minor custom stuff.
| @@ -0,0 +1,165 @@ | |||
| # Web Interference Detection Service | |||
|
|
|||
There was a problem hiding this comment.
I'm wondering instead of minting a whole new services/ directory just having these as injected/src/features/web-interference-detection/ that broker-protection imports is good enough?
There was a problem hiding this comment.
We'd be creating an artificial dependency between 2 features. TBH I don't know if we already have stuff like this in the code already, but theoretically, I don't think features should depend on each other. Happy to defer to you since you know this arch better.
2 participants
Asana Task/Github Issue: https://app.asana.com/1/137249556945/project/1206054067224240/task/1211804981097440
Description
This PR builds a prototype for the
WebInterferenceDetectionsystem:WebInterferenceDetectioncontent feature prototype + example usagesBrokerProtectioncontent featureGoals
services/web-interference-detection/) to avoid duplication across features while maintaining flexibility for future detector types.detection-utils.js(selector checking, visibility detection, pattern matching) to maximise code reuse and consistency.observeDOMChangesflag controlling whether continuous monitoring is enabled per-detector.Caveats
Current implementation creates new observers for each
detect()call whenobserveDOMChanges: true. Multiple detections may create redundant observers. Future iterations should consider observer pooling/reuse, debouncing detection calls, or centralised DOM observation.Testing Steps
n/a
Checklist
Please tick all that apply:
Note
Introduces a config-driven Web Interference Detection service (bot/fraud/YouTube ads) with messaging APIs, a new feature entry point, and BrokerProtection integration.
injected/src/services/web-interference-detection/: New detection service with detectorsbot-detection,fraud-detection,youtube-ads-detection, shared utils (detection-utils,result-factory), default config, and type definitions.detector-service.js: Orchestrates detections per request and optional continuous monitoring via callbacks.features/web-interference-detection.js: New content feature enabling on-demand detection (detectInterference) and continuous monitoring (startInterferenceMonitoring), emittinginterferenceDetected/interferenceChanged.features/broker-protection.js: Integrates detection viacreateWebInterferenceService, addsdetectInterferencehandler and emitsinterferenceDetected/interferenceDetectionError.error-utils.js: AddssafeCallhelper.default-config.js: Default interference configuration (bot, fraud, YouTube ads).types/*.js: API and detection type definitions.Written by Cursor Bugbot for commit 5e9d02f. This will update automatically on new commits. Configure here.