Skip to content

Fire information detection events in youtube detector#2439

Merged
jdorweiler merged 14 commits intomainfrom
jd/yt-events
Mar 13, 2026
Merged

Fire information detection events in youtube detector#2439
jdorweiler merged 14 commits intomainfrom
jd/yt-events

Conversation

@jdorweiler
Copy link
Contributor

@jdorweiler jdorweiler commented Mar 11, 2026

Asana Task/Github Issue:

Description

Testing Steps

Checklist

Please tick all that apply:

  • I have tested this change locally
  • 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

Medium Risk
Adds cross-feature event firing from the YouTube detector into webEvents and introduces new config-driven behavior (including domain gating), which could affect when/where detection runs and what telemetry is emitted. Risk is mitigated by new unit and Playwright integration coverage plus defensive error handling around event callbacks.

Overview
YouTube interference detections can now optionally emit webEvents notifications. The YouTube ad detector accepts an onEvent callback and a new per-detection fireDetectionEvents config gate; when enabled, it fires youtube_<type> events on new detections while swallowing sync/async callback failures.

Detector startup is tightened and made more testable. runYoutubeAdDetection is now hostname-gated (YouTube + test domains only), supports updating the event callback on an existing singleton, and adds resetYoutubeAdDetection(); stop() also clears pending start-retry timeouts.

New automated coverage. Adds unit tests for event gating/callback behavior and hostname gating, plus a Playwright integration test (new test page + config) verifying webEvent messages are emitted without page errors; the config is temporarily allowlisted from schema validation.

Written by Cursor Bugbot for commit dbf6b3c. This will update automatically on new commits. Configure here.

@jdorweiler jdorweiler changed the title Jd/yt events Fire information detection events in youtube detector Mar 11, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 11, 2026

[Beta] Generated file diff

Time updated: Fri, 13 Mar 2026 17:57:09 GMT

Android
    - android/adsjsContentScope.js
  • android/contentScope.js

File has changed

Apple
    - apple/contentScopeIsolated.js

File has changed

Chrome-mv3
    - chrome-mv3/inject.js

File has changed

Firefox
    - firefox/inject.js

File has changed

Integration
    - integration/contentScope.js

File has changed

Windows
    - windows/contentScope.js

File has changed

@github-actions github-actions bot added the semver-patch Bug fix / internal — no release needed label Mar 11, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 11, 2026

Build Branch

Branch pr-releases/jd/yt-events
Commit 0c6889ca33
Updated March 13, 2026 at 5:56:38 PM UTC

Static preview entry points

QR codes (mobile preview)
Entry point QR code
Docs QR for docs preview
Static pages QR for static pages preview
Integration pages QR for integration pages preview

Integration commands

npm (Android / Extension):

npm i github:duckduckgo/content-scope-scripts#pr-releases/jd/yt-events

Swift Package Manager (Apple):

.package(url: "https://github.com/duckduckgo/content-scope-scripts.git", branch: "pr-releases/jd/yt-events")

git submodule (Windows):

git -C submodules/content-scope-scripts fetch origin pr-releases/jd/yt-events
git -C submodules/content-scope-scripts checkout origin/pr-releases/jd/yt-events
Pin to exact commit

npm (Android / Extension):

npm i github:duckduckgo/content-scope-scripts#0c6889ca330efe37224eeea74c0dfbd626f6ff98

Swift Package Manager (Apple):

.package(url: "https://github.com/duckduckgo/content-scope-scripts.git", revision: "0c6889ca330efe37224eeea74c0dfbd626f6ff98")

git submodule (Windows):

git -C submodules/content-scope-scripts fetch origin pr-releases/jd/yt-events
git -C submodules/content-scope-scripts checkout 0c6889ca330efe37224eeea74c0dfbd626f6ff98

Copy link
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.

Stale comment

Web Compatibility Assessment

  1. injected/src/features/web-interference-detection.js (23-25) — warning
    fireEvent calls async callFeatureMethod() without await/.catch(). In YouTubeAdDetector.reportDetection(), this callback is wrapped in a synchronous try/catch, which does not catch promise rejections. If webEvents.fireEvent() throws (e.g., messaging transport/runtime failure), this can surface as unhandledrejection in page context and cause site-level handlers to behave differently.

Security Assessment

  1. No direct security vulnerabilities found in this diff.
    The new payload sent to webEvents is explicitly shaped ({ type }) and does not spread unknown objects, so it does not introduce nativeData forwarding risk.

Risk Level

Medium Risk — the PR introduces new cross-feature event emission on detection paths; behavior is config-gated and domain-scoped, but there is one unhandled async failure path that can create compatibility regressions.

Recommendations

  1. Handle the returned promise in fireEvent to prevent unhandled rejections:
const fireEvent = (type) => {
    void this.callFeatureMethod('webEvents', 'fireEvent', { type }).catch(() => {});
};
  1. Add a unit test around the callback path verifying no unhandledrejection occurs when callFeatureMethod rejects.
  2. Keep the domain guard test-covered (non-YouTube host should return immediately and not initialize detector state/intervals).

Validation run:

  • npm run test-unit --workspace=injected -- unit-test/youtube-ad-detection.js (passes: 11 specs)
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

@github-actions
Copy link
Contributor

⚠️ Cursor review was not successful.

This PR requires a manual review and approval from a member of one of the following teams:

  • @duckduckgo/content-scope-scripts-owners
  • @duckduckgo/apple-devs
  • @duckduckgo/android-devs
  • @duckduckgo/team-windows-development
  • @duckduckgo/extension-owners
  • @duckduckgo/config-aor
  • @duckduckgo/breakage-aor
  • @duckduckgo/breakage

Copy link
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.

Stale comment

Web Compatibility Assessment

  • No blocking web-compat regressions found in changed lines.
  • injected/src/features/web-interference-detection.js (init path), severity: info.
    The detector invocation is now centralized (runYoutubeAdDetection(...)) and host-gated internally; this preserves previous YouTube-only runtime behavior while avoiding feature-level hostname branching.
  • injected/src/detectors/detections/youtube-ad-detection.js (reportDetection + runYoutubeAdDetection changes), severity: info.
    Event firing is correctly de-duplicated by existing detection-state logic (showing/lastMessage) and remains config-gated via fireDetectionEvents, which limits event churn.

Security Assessment

  • No exploitable security vulnerabilities identified in this diff.
  • injected/src/features/web-interference-detection.js (fireEvent callback), severity: info.
    Outgoing payload is explicitly shaped as { type } (no spread of untrusted objects), so this change does not introduce nativeData leakage risk.
  • injected/src/detectors/detections/youtube-ad-detection.js (runYoutubeAdDetection hostname gate), severity: info.
    Detector start remains restricted to youtube.com and the test domain gate, reducing attack surface on unrelated sites.

Risk Level

Medium Risk — runtime behavior now includes a new event path from detector results to webEvents messaging, but it is config-gated, host-gated, and covered by focused unit tests.

Recommendations

  1. Add an integration test in web-interference-detection/web-events flow to assert that webEvent fires only when fireDetectionEvents[type] === true and only on allowed hostnames.
  2. Add a regression test for runYoutubeAdDetection host gating to prevent accidental expansion beyond youtube.com and test domains.
  3. Consider explicit rejection handling on void this.callFeatureMethod('webEvents', 'fireEvent', { type }) for future-proofing against async failures (.catch(() => {}) parity with existing defensive patterns).
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

Copy link
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.

Stale comment

Web Compatibility Assessment

  1. injected/src/features/web-interference-detection.js (lines 23-25) - warning: fireEvent calls void this.callFeatureMethod(...) without handling promise rejection. If webEvents.fireEvent() or underlying messaging.notify() throws, this becomes an unhandled rejection path. In injected hostile-page runtime, that can create noisy console/runtime errors and brittle behavior under transport/init edge cases. This conflicts with the project’s async error-handling pattern used in web-detection (await + try/catch).

Security Assessment

  1. injected/src/detectors/detections/youtube-ad-detection.js (lines 134-140) - info: event emission is correctly gated by fireDetectionEvents and only forwards a constrained { type } payload via webEvents, so no nativeData leakage pattern was introduced.
  2. injected/src/detectors/detections/youtube-ad-detection.js (lines 740-745) - info: explicit hostname gating (youtube.com + privacy-test-pages.site) reduces detector execution scope and does not relax message-bridge or origin/security boundaries.

Risk Level

Medium Risk - the PR introduces cross-feature event firing from detector code and adds a new async messaging call path; the unhandled-rejection edge case is the main compatibility concern.

Recommendations

  1. Wrap callFeatureMethod('webEvents', 'fireEvent', ...) in explicit rejection handling (e.g. .catch(() => {}) or try/catch in an async helper) to prevent unhandled promise rejections.
  2. Add a unit test for web-interference-detection that simulates callFeatureMethod rejecting and asserts no uncaught rejection escapes.
  3. Keep the hostname allowlist explicit (as done here) and avoid broadening to wildcard domains for detector auto-start.
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

Copy link
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.

Stale comment

Web Compatibility Assessment

No compatibility regressions found in the changed lines.

Findings:

  • injected/src/features/web-interference-detection.js (23-29) — info: New fireEvent wiring is non-blocking and intentionally best-effort, but there is no feature-level test proving end-to-end dispatch from detector transition -> webEvents.fireEvent. Current unit coverage is detector-local only, so regressions in cross-feature wiring could slip.

Security Assessment

No exploitable security vulnerabilities found in this diff.

Findings:

  • injected/src/features/web-interference-detection.js (23-25) — info: Outgoing payload is explicitly { type } (good: no nativeData spread), but failures from callFeatureMethod are fully suppressed. This is acceptable for optional telemetry, though it reduces observability if event delivery is expected in some platform configs.

Risk Level

Medium Risk — runtime behavior changes on YouTube pages now include cross-feature event dispatch, but scope is config-gated, payload is minimal, and there are no changes to wrapper utilities, captured globals, bridge/origin validation, or messaging transports.

Recommendations

  1. Add a unit/integration test around WebInterferenceDetection to verify detector transitions trigger webEvents.fireEvent with type values only from the expected youtube_* set.
  2. Add an assertion test that disabled/absent fireDetectionEvents never emits events through the feature wiring path (not only via direct detector unit tests).
  3. Consider debug-only logging when callFeatureMethod('webEvents', 'fireEvent', ...) returns CallFeatureMethodError, to keep silent-fail behavior in prod while improving diagnosability.
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

Copy link
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.

Stale comment

Web Compatibility Assessment

No web compatibility regressions identified in the changed hunks.

  • injected/src/detectors/detections/youtube-ad-detection.js (constructor, reportDetection, runYoutubeAdDetection) — severity: info

    • Event callback wiring is additive and does not alter browser API shims/prototypes, descriptor contracts, or wrapper fidelity surfaces.
    • Hostname gating is still explicit and now centralized in runYoutubeAdDetection; detector startup remains restricted to YouTube (plus explicit test domain), which avoids broad-site DOM polling.
  • injected/src/features/web-interference-detection.js (init) — severity: info

    • Moving YouTube detection invocation to unconditional call-site is safe because startup is gated inside runYoutubeAdDetection.
    • No new DOM mutation patterns, timers, or custom element registration were introduced in this file.

Security Assessment

No security vulnerabilities identified in the changed hunks.

  • injected/src/features/web-interference-detection.js (fireEvent callback) — severity: info

    • Outgoing payload is explicitly { type } (no object spread), so this does not introduce nativeData propagation risk.
    • Errors from callFeatureMethod('webEvents', 'fireEvent', ...) are contained locally, avoiding feature crash cascades.
  • injected/src/detectors/detections/youtube-ad-detection.js (reportDetection) — severity: info

    • Event type emitted is derived from internal detector enums (youtube_<type>), not page-controlled input.
    • No changes to message bridge trust boundaries, origin checks, captured-globals, or stack-trace exemption logic.

Risk Level

Medium Risk — behavior changed in runtime detector flow (event emission + initialization path adjustment), but no wrapper/messaging boundary hardening was removed and no new high-risk primitives were introduced.

Recommendations

  1. Add a focused unit test for runYoutubeAdDetection hostname gating to lock in expected behavior for youtube.com, subdomains, and privacy-test-pages.site.
  2. Add one integration test asserting webEvents.fireEvent is triggered only for detection types enabled in fireDetectionEvents and not emitted for duplicate detections.
  3. Tighten callback contract in detector docs/types to explicitly allow async callbacks (or enforce sync), so future refactors don’t accidentally introduce unhandled Promise rejections outside current guarded implementation.
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

Tests lock in expected behavior: allows youtube.com and subdomains,
privacy-test-pages.site and subdomains, rejects other domains.
Also adds resetYoutubeAdDetection() for test cleanup and stores
the start retry timeout so stop() can clear it properly.

Made-with: Cursor
Copy link
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.

Stale comment

Web Compatibility Assessment

  • No web compatibility regressions found in the changed lines.
  • Coverage added in injected/unit-test/youtube-ad-detection.js materially improves confidence for hostname gating, duplicate detection suppression, and event-gating behavior.

Security Assessment

  • No security vulnerabilities identified in the changed lines.
  • Event forwarding path uses explicit payload construction ({ type } into fireEvent, then { type, data } in webEvents), with no object spreading of untrusted inputs, so this change does not introduce nativeData leakage risk.

Risk Level

Medium Risk — the PR changes detector lifecycle behavior and introduces new cross-feature event emission, but does not modify wrapper utilities, captured globals, message bridge security checks, or transport/origin validation paths.

Recommendations

  1. Add one integration test that verifies a YouTube detection triggers a webEvent message end-to-end (not only unit-level callback assertions), so platform wiring is covered.
  2. Add one unit/integration assertion that event emission is non-fatal when webEvents is unavailable or skipped, to guard the silent-failure path intentionally introduced in web-interference-detection.
  3. Optionally add debug logging (guarded by debug mode) when callFeatureMethod('webEvents', 'fireEvent', ...) returns a CallFeatureMethodError, to improve diagnosis without changing runtime behavior.
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

- Integration test: verifies YouTube detection triggers webEvent E2E
  (ad-blocker modal DOM → detector sweep → webEvent notification)
- Integration test: verifies webEvent message structure (context, featureName, no nativeData)
- Integration test: verifies no page errors during detection and event firing
- Unit test: callback invocation when async
- Debug logging: log CallFeatureMethodError in debug mode when webEvents is unavailable
- Allow localhost in hostname check for integration test compatibility
- Add youtube-detection-events config to schema validation allowlist
  (fireDetectionEvents not yet in published schema)

Made-with: Cursor
Copy link
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.

Stale comment

Web Compatibility Assessment

  • injected/src/detectors/detections/youtube-ad-detection.js (runYoutubeAdDetection, around window.location.hostname gating): warning

    • The detector now runs on localhost and *.privacy-test-pages.site in addition to YouTube. This broadens runtime behavior beyond real YouTube surfaces and can trigger retries/sweeps on non-YouTube pages when youtubeAds is enabled, which is a compatibility/perf regression vector for local/dev environments.
  • injected/src/detectors/detections/youtube-ad-detection.js (singleton path around if (detectorInstance) return detectorInstance.getResults();): warning

    • Event callback attachment is one-shot at first construction. If the singleton is created via a caller that does not pass fireEvent, later calls that do pass it cannot attach the callback, so webEvent emissions are silently skipped for that page lifetime. This is a timing/ordering regression risk.

Security Assessment

  • injected/src/detectors/detections/youtube-ad-detection.js (hostname allowlist expansion): warning

    • Under the configuration-trust model, hardcoded test-domain enablement in production logic increases attack surface for telemetry/event generation outside intended production domains. Even with current payload constraints, this should be explicitly config/debug gated rather than unconditional in code.
  • No direct messaging-boundary violations found in changed code:

    • webEvents.fireEvent call uses explicit { type } payload (no object spread), so no nativeData leakage path introduced.
    • No origin-validation or message-bridge security checks were modified.

Risk Level

Medium Risk — behavioral changes are localized, but they alter detector activation scope and introduce a singleton callback-order dependency that can cause silent functional regressions.

Recommendations

  1. Gate non-YouTube test domains (localhost, privacy-test-pages.site) behind explicit debug/test config and default it to off in production paths.
  2. Make callback binding idempotent for existing singleton instances (e.g., update detectorInstance.onEvent when a new callback is supplied) or force re-init when transitioning from no-callback to callback.
  3. Add a unit/integration test for initialization order: first call without callback, second call with callback, and assert webEvent is emitted.

Validation performed:

  • npm run test-unit -- --filter=YouTubeAdDetector (pass)
  • npx playwright test integration-test/web-interference-detection-events.spec.js --project windows --reporter list (pass)
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

If the detector singleton already exists and a new fireEvent callback
is provided, update the callback. This handles the case where the
detector is created without a callback and later called with one.

Made-with: Cursor
Copy link
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.

Stale comment

Web Compatibility Assessment

  • injected/src/detectors/detections/youtube-ad-detection.js (line range: 745-751) warning: Hostname gating now explicitly allows localhost and *.privacy-test-pages.site, so the detector can run outside YouTube in production bundles. If youtubeAds is enabled remotely, this introduces periodic DOM sweeps/timers on local development apps and non-YouTube test hosts, which is a broader execution surface than before and can create avoidable perf/breakage risk.

Security Assessment

  • injected/src/detectors/detections/youtube-ad-detection.js (line range: 745-751) warning: Expanding execution to localhost and *.privacy-test-pages.site weakens the trust boundary for youtube_* event signals. Arbitrary localhost content can intentionally shape DOM state to trigger youtube_adBlocker/related events via webEvents, creating spoofable telemetry/event noise from attacker-controlled pages.
  • No direct nativeData leakage paths were introduced in this diff; event payload construction remains explicit ({ type }) before calling webEvents.fireEvent.

Risk Level

Medium Risk: The PR adds cross-feature event emission in runtime detector code and broadens host eligibility for detector startup; this is not a wrapper/bridge critical-path change, but it does expand behavioral scope in page contexts.

Recommendations

  1. Restrict production hostname allowlist back to YouTube-only, and gate test domains behind an explicit test/debug flag or test-only config path.
  2. Add a negative integration test that verifies no webEvent is emitted on a non-YouTube/non-test host even when youtubeAds.state is enabled.
  3. Add a unit test for an async-rejecting onEvent callback path to ensure detection flow cannot produce unhandled promise rejections if callback behavior changes later.
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

Copy link
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.

Stale comment

Web Compatibility Assessment

  1. injected/src/detectors/detections/youtube-ad-detection.js (745-751) — warning
    The detector host gate was expanded to run on localhost and *.privacy-test-pages.site in production code paths, not just YouTube hosts. This increases DOM polling (setInterval sweeps + selector queries) on non-YouTube pages and can produce false-positive youtube_* detections on those domains if config is enabled, which is a compatibility/behavior drift from prior YouTube-only runtime scope.

  2. injected/src/detectors/detections/youtube-ad-detection.js (135-140) — info
    onEvent is invoked in a sync try/catch; rejected async callbacks are not caught here. Current caller catches internally, so this PR is likely safe today, but the detector contract is now easier to misuse by future callers and could surface unhandled promise rejections in page context.

Security Assessment

  1. injected/src/detectors/detections/youtube-ad-detection.js (745-751) + injected/src/features/web-interference-detection.js (23-35) — warning
    Expanding detector execution beyond YouTube broadens the page-controlled surface that can trigger webEvents.fireEvent notifications. This is not a direct trust-boundary break (message API usage and nativeData handling look correct), but it increases telemetry/event-spam exposure on non-YouTube hosts if settings are active.

  2. injected/src/features/web-interference-detection.js (23-31) — info
    Error handling is intentionally permissive (catch {}) when webEvents is unavailable. That avoids hard failures, but it also suppresses signal for unexpected failures on supported platforms unless debug is enabled.

Risk Level

Medium Risk — runtime detector behavior changed in core injected code and now conditionally emits cross-feature messaging events; no wrapper/message-bridge primitive changes, but host-scope expansion and event firing warrant careful rollout.

Recommendations

  1. Restrict non-YouTube host exceptions (localhost, privacy-test-pages.site) to explicit test/debug mode so production runtime scope remains YouTube-only.
  2. Harden the event callback contract by normalizing async handling in reportDetection (e.g., void Promise.resolve(this.onEvent(...)).catch(() => {})) to prevent future unhandled rejections.
  3. Add a negative integration test asserting no webEvent emission on non-YouTube hosts when not in test/debug mode.
  4. Keep the existing nativeData assertions (good) and add one rate/duplication assertion to prevent event-spam regressions.

Validation run:

  • npm run test-unit -- unit-test/youtube-ad-detection.js
  • npx playwright test integration-test/web-interference-detection-events.spec.js --project windows --grep-invert '@screenshots' --reporter list
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

@jdorweiler jdorweiler marked this pull request as ready for review March 12, 2026 14:59
@jdorweiler jdorweiler requested review from a team and GuiltyDolphin as code owners March 12, 2026 14:59
Copy link
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.

Stale comment

Web Compatibility Assessment

  1. injected/src/detectors/detections/youtube-ad-detection.js 135-140warning
    reportDetection() invokes this.onEvent(...) inside a synchronous try/catch, but does not handle async rejections from promise-returning callbacks. The new caller (fireEvent) is async today, so any future rejected promise path would become an unhandled rejection (window.onunhandledrejection), which can cause observable page-side behavior/regressions in hostile pages.

  2. injected/src/detectors/detections/youtube-ad-detection.js 744-751info
    Hostname gating was moved into runYoutubeAdDetection() and now includes test domains. This is compatible with the feature-level call-site change (detector is now called from all pages), and prevents detector startup off-target; no immediate breakage seen.

Security Assessment

  1. injected/src/features/web-interference-detection.js 23-31 and 25info
    Event forwarding uses explicit params ({ type }) via callFeatureMethod('webEvents', 'fireEvent', ...) and does not spread arbitrary objects, so there is no nativeData leakage vector introduced here.

  2. injected/src/detectors/detections/youtube-ad-detection.js 744-773info
    No new privileged bridge/origin surfaces were added; changes are contained to detector lifecycle and inter-feature call usage. I did not find a new message-bridge trust boundary bypass.

Risk Level

Medium Risk — runtime behavior changed in a cross-feature detector path (new event callback + detector init path), but no wrapper-utils/captured-globals/transport security primitives were modified.

Recommendations

  1. Handle promise-returning callbacks in YouTubeAdDetector.reportDetection() (e.g., normalize return to a promise and attach .catch) so async callback failures cannot surface as unhandled rejections.
  2. Add a unit test that passes an async callback which explicitly rejects, and assert no unhandled rejection is emitted while detection state still updates.
  3. Once available, add fireDetectionEvents to the published config schema to remove the current schema allowlist exception and keep config contract validation strict.

Validation run:

  • npm run test-unit -- --filter='YouTubeAdDetector'
  • npm run test-int -- --project windows --reporter list integration-test/web-interference-detection-events.spec.js
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

Copy link
Member

@GuiltyDolphin GuiltyDolphin left a comment

Choose a reason for hiding this comment

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

Tested it out on extension and Android. Looks good.

  • tried out events with the test page
  • enabled/disabled feature
  • enabled/disabled fireDetectionEvents entries
  • removed fireDetectionEvents

Detections seemed a bit slow on extension, but I tested on Android too and they were much more responsive there, so I'm guessing something to do with bundled config in test extension.

…se rejections

The synchronous try/catch in reportDetection does not catch rejected
promises from async onEvent callbacks. Although the current fireEvent
caller has its own internal try/catch, the detector's contract should
be safe for any async callback.

- Check if onEvent returns a thenable and attach .catch() to absorb rejections
- Widen JSDoc to note the callback may be async
- Add unit test with a rejecting async callback to verify no unhandled rejection

Co-authored-by: Ben Moon <GuiltyDolphin@users.noreply.github.com>
Copy link
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.

Stale comment

Web Compatibility Assessment

  1. injected/src/detectors/detections/youtube-ad-detection.js (L749-L754, L751-L753)warning
    runYoutubeAdDetection() now explicitly allows localhost and *.privacy-test-pages.site in production runtime, not just youtube.com. This expands where the detector’s polling/timers run (start() + sweep/reroot intervals), which increases compatibility/perf exposure on non-YouTube pages and can generate youtube_* detections outside intended scope.

  2. injected/src/features/web-interference-detection.js (L23-L31) + injected/src/detectors/detections/youtube-ad-detection.js (L135-L144)info
    Event delivery failures are fully swallowed (sync throw + async rejection). This is compatibility-safe (avoids page breakage/unhandled rejections), but it also makes regressions in cross-feature signaling silent and harder to detect in the field.

Security Assessment

  1. injected/src/detectors/detections/youtube-ad-detection.js (L749-L754, L751-L753)warning
    From a trust-boundary perspective, hardcoded host expansion increases event-emission surface area for webEvents.fireEvent (youtube_*), including localhost. A malicious page on an allowed non-YouTube host can intentionally trigger detector patterns to spam event telemetry. This is a Configuration Trust / Data Exfiltration surface concern (increased outbound signal surface without config-only scoping).

  2. injected/unit-test/features.js (L87-L91)warning
    The new test config is exempted from schema validation because fireDetectionEvents is unpublished. This weakens config-contract guarantees in CI for the new setting and increases risk of malformed config shape silently passing tests (Configuration Trust category).

Risk Level

Medium Risk — cross-feature event emission itself is well-contained, but scope expansion to non-YouTube hosts plus temporary schema-contract bypass introduces moderate compatibility and telemetry-surface risk.

Recommendations

  1. Restrict runtime host gating back to YouTube only in production code, and inject test-domain allowance via test-only mechanism.
    Alternative: add a dedicated config flag (e.g., allowNonYouTubeTestHosts) defaulting false, enabled only in test fixtures.

  2. Add an explicit negative test that verifies no detector initialization/event firing on non-YouTube production hosts (including localhost) unless a test-only override is enabled.

  3. Publish fireDetectionEvents in the config schema, then remove the schema-validation allowlist entry from unit-test/features.js.

  4. Keep the no-throw behavior for event callbacks, but add optional debug telemetry/counter for dropped event-delivery failures so silent breakage is observable.

Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

Copy link
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.

Web Compatibility Assessment

  • warning; file: injected/src/detectors/detections/youtube-ad-detection.js; line range: 749-754 (especially 751-753). The detector now starts on localhost and *.privacy-test-pages.site in addition to YouTube. That broadens periodic DOM polling (sweep) and detection/event side effects to non-YouTube pages if config scope is accidentally wide, increasing false-positive and breakage risk outside the intended surface.
  • info; file: injected/src/detectors/detections/youtube-ad-detection.js; line range: 659-662. Clearing startRetryTimeout in stop() is a good lifecycle fix that reduces stale timer activity and cross-navigation timing artifacts.

Security Assessment

  • warning; files: injected/src/detectors/detections/youtube-ad-detection.js, injected/src/features/web-interference-detection.js; line ranges: 751-753 and 23-35. Allowing non-YouTube hostnames plus immediate webEvents.fireEvent wiring creates a new signal path from localhost/test pages into native webEvent notifications. Payload is constrained ({ type }) and no nativeData leakage is introduced, but this still expands collection surface in a hostile-page model.
  • info; files: injected/src/features/web-interference-detection.js, injected/src/detectors/detections/youtube-ad-detection.js; line ranges: 23-31 and 135-145. Async callback failures are contained and do not break detector execution, which is compatibility-positive; however, broad catch-and-ignore behavior can mask runtime integration faults unless debug logging is enabled.

Risk Level

Medium Risk — this PR introduces cross-feature event emission and async messaging from detector paths, with domain-scope expansion beyond YouTube that can change runtime behavior outside the primary target site.

Recommendations

  1. Restrict runtime hostname gating in production to YouTube-only, and move localhost/privacy-test-pages.site allowances behind an explicit test/internal flag so integration tests keep coverage without broadening shipped behavior.
  2. Add a negative integration assertion that pages on non-YouTube hostnames do not emit webEvent notifications even when webInterferenceDetection is enabled.
  3. Keep explicit payload shaping ({ type }) and add/retain tests that assert absence of nativeData in both top-level message and params (current new spec is good and should remain mandatory).

Validation run:

  • npm run test-unit -- --filter='YouTubeAdDetector' (pass)
  • npx playwright test integration-test/web-interference-detection-events.spec.js --project windows --reporter list --grep-invert '@screenshots' (pass)
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

@jdorweiler jdorweiler added this pull request to the merge queue Mar 13, 2026
Merged via the queue into main with commit 4b0f889 Mar 13, 2026
42 checks passed
@jdorweiler jdorweiler deleted the jd/yt-events branch March 13, 2026 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-patch Bug fix / internal — no release needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants