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
Medium Risk
Medium risk due to non-trivial changes to injected content-scope shimming logic (apiManipulation descriptor overriding and webCompat synthetic InputDeviceInfo.getCapabilities), which can affect site compatibility and fingerprinting surfaces; the TypeScript/tooling bumps are comparatively low risk.
Overview Tooling: Switches tsconfig.json to moduleResolution: "bundler", bumps TypeScript/typedoc/types tooling deps, and updates typedoc.js typing; .gitignore rules are also tightened.
Injected runtime shims: Extends apiManipulation to support value-descriptor (method) replacement and setter overrides, including masking replacements to look native and safely handling inherited properties via descriptor merging (mergePropertyDescriptors). Enhances webCompat device enumeration by adding a masked InputDeviceInfo.getCapabilities shim for synthetic devices (configurable shimMode), and expands Playwright/unit tests to validate these behaviors.
Special pages: Adds enableAskAiSuggestion config to the New Tab omnibar to optionally hide the inline “Ask Duck.ai” suggestion (with integration tests and docs/schema updates), adds new RMF big-single-action examples/icons (Preview, YoutubeNew), and updates onboarding locale strings for the YouTube row copy.
Reviewed by Cursor Bugbot for commit 89da4c1. Bugbot is set up for automated code reviews on this repo. Configure here.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
No findings. The diff does not change injected runtime code, API shims/wrappers, message bridge behavior, DOM manipulation, or platform entry points. The special-pages/pages/special-error/app/hooks/ErrorStrings.jsx changes are JSDoc-only type import updates; rendered output and URL handling are unchanged.
Security Assessment
No findings. I did not find new uncaptured global usage in injected code, messaging trust-boundary changes, origin/postMessage changes, config-gating changes, or iframe/network/CSS injection surfaces.
Risk Level
Low Risk for web compatibility and security: this PR is limited to TypeScript/Typedoc typing configuration, ignore-rule cleanup, and JSDoc-only type references, with no shipped injected behavior changes.
Recommendations
No blocking recommendations. Targeted verification passed:
* Fix synthetic input device capabilities shim
* Add illegal invocation characterization test
* Clarify illegal invocation regression case
* Tighten synthetic capabilities regression test
* Simplify synthetic capabilities proof tests
* web-compat: mask synthetic getCapabilities toString via wrapToString
Use the existing wrapToString helper so the synthetic InputDeviceInfo.getCapabilities shim returns a native-looking toString() (and toString.toString()), and align descriptor flags (writable/configurable/enumerable: true) with native prototype methods. Extends the device-enumeration integration test to assert the masked toString output.
Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
* web-compat: harden synthetic input device prototype
* web-compat: add getCapabilitiesShim setting for enumerateDevices
Adds a per-site config switch under webCompat.enumerateDevices so we can react to breakage in the wild without redeploying code:
- 'syntheticPrototype' (default, unchanged): intermediate prototype with own getCapabilities. Hides hasOwnProperty('getCapabilities'); one-level prototype-chain depth difference.
- 'instanceOwn': preserve InputDeviceInfo.prototype as the direct prototype; place an own masked getCapabilities on the instance. Restores Object.getPrototypeOf(d) === InputDeviceInfo.prototype.
- 'disabled': no shim; native brand check throws TypeError 'Illegal invocation'.
Also fixes the previously flaky disabled/enabled tests by waiting for the renderResults 'results-ready' event before reading window.results, and adds coverage for the two new modes.
Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
* web-compat: drop redundant getCapabilitiesShim=disabled mode
If a site needs the unaltered native semantics, the entire enumerateDevices feature can already be disabled, in which case no synthetic devices are produced and the native getCapabilities works normally. The 'disabled' shim mode kept producing synthetic devices but reintroduced the illegal-invocation throw, which is just the original bug.
Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
* web-compat: extract defineSyntheticGetCapabilities helper
Both shim modes (syntheticPrototype, instanceOwn) build the same masked no-op descriptor for getCapabilities and call this.defineProperty with the same flags. Pull that into a single helper to keep the createMediaDeviceInfo branches readable.
Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
* web-compat: shim all brand-checked methods on MediaDeviceInfo/InputDeviceInfo prototypes
Replaces the single-method getCapabilities shim with a generic prototype-walker that defines a masked no-op for every function-valued own property on MediaDeviceInfo.prototype and InputDeviceInfo.prototype. Explicit instance-level definitions (deviceId, kind, label, groupId, toJSON) win, so toJSON semantics are preserved.
Renames the per-site setting from 'getCapabilitiesShim' to 'shimMode' since it now covers every method in the space, not just getCapabilities. Adds a regression test that plants a synthetic future method on InputDeviceInfo.prototype and verifies it gets auto-shimmed with a native-looking name/toString, while toJSON keeps its explicit own behavior.
Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
* web-compat: revert generic prototype-walking shim; keep shimMode setting name
Per review: the generic for-loop over MediaDeviceInfo/InputDeviceInfo prototypes is overkill for the only known problem method (getCapabilities). Revert to the single defineSyntheticGetCapabilities helper, but keep the per-site setting named 'shimMode' (not 'getCapabilitiesShim') so we can extend it to other shims later without another config rename.
Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
* Fix Playwright extension path resolution
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
…comparison table (#2702)
* added new line in the comparison table plus translations
* fixed indentation
* fixed expectation on tests
* updated screeshots
* Support value descriptors in apiManipulation
* Refine apiManipulation value descriptor handling
* Trim apiManipulation descriptor duplication
* Test value descriptor override without define: true
Adds two unit tests for apiManipulation:
- Confirms an existing DOM-style value function descriptor (set via
Object.defineProperty with writable/configurable/enumerable) can be
remotely overridden via a value descriptor change *without*
define: true, and that the original descriptor attributes
(writable/configurable/enumerable) are preserved through the merge
in wrapProperty().
- Confirms that setting define: true on a change for a property that
already exists still falls through to wrapProperty(), so the new
function replaces the existing one rather than being skipped.
Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
* Mask method replacements to preserve native identity
When apiManipulation replaces an existing method-valued descriptor with a
configured function, mask the replacement against the original DOM method
so that toString(), toString.toString(), .name and .length still resemble
the native method. Without this, third-party scripts can trivially detect
the override via fn.toString() ('function noop() {}') or fn.name.
Implementation: in wrapApiDescriptor, when the new descriptor's value is
a function and an existing own value descriptor with a function value
exists, wrap the configured replacement with maskMethodReplacement().
That helper creates a fresh wrapper function (so we never mutate shared
singletons such as functionMap.noop), copies the original function's
name and length onto it, and applies wrapToString() against the original
to preserve toString output. The resulting value is then defined via
ContentFeature.defineProperty in the normal way, which adds its debug
flag and a second wrapToString layer; the two layers compose correctly
because the inner mask is wrapToString rather than wrapFunction (the
latter creates a non-configurable own toString that violates the Proxy
get-trap invariant once the outer wrapToString is added on top).
Adds unit tests asserting:
- toString() returns the original method's source rather than the
replacement's body
- toString.toString() still resolves to Function.prototype.toString
- .name and .length match the original method
- When the original is a real native function (Object.prototype
.hasOwnProperty), the masked toString() contains '[native code]'
- The configured replacement is still what executes at call time
Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
* Use captured globals in apiManipulation method masking
The masking helper added in the previous commit invoked the configured
replacement via the page-visible `Reflect.apply` and set `name`/`length`
on the wrapper via the page-visible `Object.defineProperty`. Both are
re-readable by hostile pages after content scope initialisation:
- `Reflect.apply` is reassignable on `globalThis.Reflect`, letting a
page observe or alter every invocation of the protected method.
- `Object.defineProperty` is reassignable on `globalThis.Object`,
letting a page block or poison the name/length masking.
Switch the helper to:
- `ReflectApply` — new bound primitive exported from captured-globals,
matching the existing `ReflectDeleteProperty` pattern.
- `objectDefineProperty` — already-exported captured `Object.defineProperty`.
Adds a unit test that swaps in spies for `globalThis.Reflect.apply` and
`globalThis.Object.defineProperty` after feature construction and asserts:
1. `Object.defineProperty` is not called during `wrapApiDescriptor`
(the helper used captured `objectDefineProperty`).
2. Masked `name`/`length`/`toString()` remain intact under tampering.
3. End-to-end invocation still routes to the configured replacement.
Note: `ContentFeature.defineProperty`'s outer debug-wrapper still reads
`Reflect.apply` from the page; that is pre-existing and out of scope for
this helper. The captured `ReflectApply` here guarantees the helper's
own inner call (helper → configured replacement) cannot be intercepted.
Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
* apiManipulation: shadow-define inherited methods + setterValue support
Adds two capabilities to the descriptor-change path so configs can target
real-world media API surfaces that the existing wrapApiDescriptor cannot:
1. Shadow-define inherited methods. The previous define: true guard was
!(key in api), which is alse for any property reachable via the
prototype chain (e.g. MediaDevices.prototype.addEventListener, inherited
from EventTarget.prototype). Switch the guard to
!Object.prototype.hasOwnProperty.call(api, key) so a config can define
an own property on MediaDevices.prototype that shadows the inherited
one without modifying EventTarget.prototype (which would affect every
other EventTarget consumer - window, document, elements, etc.).
2. setterValue field on descriptor changes. Accessor properties such as
MediaDevices.prototype.ondevicechange (an EventHandler IDL attribute)
are a getter+setter pair. The existing getterValue path produces only
{ get }, so wrapProperty's {...origDescriptor, ...descriptor} merge
preserves the original setter; assigning mediaDevices.ondevicechange = fn
still registers a real listener. Adding setterValue to the descriptor
shape lets configs override the setter half too, invoking the configured
function via captured ReflectApply on each assignment. getterValue
and setterValue may be supplied independently or together.
Validation is updated to accept the accessor shape when only setterValue
is present, and to reject mixing accessor (getterValue/setterValue) with
value (�alue) shapes in a single change.
Adds unit tests covering:
- shadow-define an inherited method with define: true
- no-op behaviour when define: true is omitted and key is inherited
- setter override alongside getter
- setter-only override (original getter preserved)
- validation for setterValue-only changes
- validation rejecting setterValue + value mixing
Motivation: the original Claude camera-prompt-on-login report turned out
to be triggered by Claude subscribing to mediaDevices.devicechange during
its bootstrap (via both �ddEventListener('devicechange', ...) and
mediaDevices.ondevicechange = ...). With these two additions a single
apiManipulation config can suppress every JS-side device-enumeration trigger
without touching EventTarget.prototype or shipping a separate JS shim.
* Mask setterValue replacement against original native setter
When apiManipulation overrides a setter (`setterValue`) on an existing
accessor descriptor, the replacement setter is now masked against the
original `origDescriptor.set` via the existing `maskMethodReplacement`
helper. This preserves the observable identity of the accessor for
descriptor inspection: `Object.getOwnPropertyDescriptor(...).set.toString()`,
`.set.name`, and `.set.length` mirror the original setter rather than
exposing our internal `setter(v) {...}` shape. For native event-handler
IDL attributes (e.g. `Element.prototype.onclick`,
`MediaDevices.prototype.ondevicechange`) this prevents trivial detection
of the override via descriptor probing.
Implementation: extract `origDescriptor = getOwnPropertyDescriptor(api,
key)` once per call in `wrapApiDescriptor`, then branch on
`descriptorKind`:
- For value descriptors, mask the replacement against the original
function value (existing behaviour, refactored to share the lookup).
- For accessor descriptors, additionally mask `descriptor.set` against
`origDescriptor.set` whenever both are functions. The getter path is
left alone because configured getters are generated by processAttr
per-read and don't typically need toString-fidelity.
Also fixes blocking TypeScript errors from the previous commit:
- `createApiDescriptor` now uses a permissive intermediate object type
internally so `descriptor.get` / `descriptor.set` assignments inside
the accessor branch type-check, and returns a cast back to
`Partial<StrictPropertyDescriptor>`.
- The `configSetting` parameter type now allows `undefined` (matches
the setter-only call shape), with a localised cast inside the value
branch where `wrapApiDescriptor`'s early-return already guarantees a
defined value.
Adds a unit test asserting that, after applying a `setterValue` change
over an existing accessor descriptor, the wrapped `descriptor.set`'s
`toString()`, `toString.toString()`, `.name`, and `.length` match the
original setter, and the configured replacement still swallows the
assignment (original setter not invoked).
Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
* apiManipulation: define only for missing APIs; wrap inherited methods
Retain define:true for properties absent from the prototype chain only.
When an API exists (own or inherited), apply value/getterValue/setterValue
overrides—including shadow-defining inherited methods such as
MediaDevices.prototype.addEventListener without requiring define.
* Revert define behavior and JSDoc to branch baseline
Restore unchanged define semantics from c7be658. Value/getterValue/setterValue
and method masking remain; inherited APIs still require define: true to shadow-define.
* Restore define JSDoc and main semantics (key in api)
Revert expanded define documentation and hasOwnProperty shadow-define
behavior. Remove inherited shadow-define tests. define:true remains for
adding properties that are absent from the target object.
* Integrate privacy-configuration #5215 MediaDevices apiManipulation config
Bump @duckduckgo/privacy-configuration to 1779290401870 (merged PR #5215
schema: setterValue/value descriptors with original define comment).
Restore inherited-property shadow-defining without define: true so
MediaDevices.prototype.addEventListener/removeEventListener overrides
match the deployed remote-config shape. Add unit coverage for the
#5215 apiChanges payload and schema validation.
* apiManipulation: guard inherited descriptor merges against invalid shapes
Extract mergePropertyDescriptors from wrapProperty so shadow-defining
inherited APIs uses the same value/accessor compatibility rules. Skip
the override (instead of throwing) when remote config shape disagrees with
the live prototype descriptor kind.
Clarify define vs inherited shadow-define JSDoc. Add unit coverage for
descriptor-kind mismatches on inherited properties.
* apiManipulation: require define:true for inherited shadow-define
Remove the implicit else branch that shadow-defined prototype-chain
properties without define. Inherited overrides now use the same
define:true && !hasOwnProperty path as intended, with descriptor-kind
guards via mergePropertyDescriptors.
Update unit tests; privacy-configuration #5215 entries will need
define: true added in remote config.
* Fix mergePropertyDescriptors return type for StrictPropertyDescriptor
Normalize merged descriptors so tsc accepts them at defineProperty and
wrapProperty call sites. No remote config change required.
* Fix tab-suspension test config for updated tabSuspension schema
privacy-configuration #1779290401870 expects inputFieldFocusDetection
settings as { state } object, not a bare string.
* apiManipulation: restore implicit inherited shadow-define
Re-add the else path for prototype-chain properties so remote config
(e.g. MediaDevices addEventListener) works without define: true.
Properties absent from the entire chain still no-op unless define: true.
Descriptor-kind mismatches still skip via mergePropertyDescriptors.
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Jonathan Kingston <jonathanKingston@users.noreply.github.com>
* Gate inline Ask Duck.ai suggestion on enableAskAiSuggestion
Adds a new optional OmnibarConfig field (default true, missing/undefined
treated as true for back-compat with older native clients) and threads it
through OmnibarConsumer → Omnibar → SearchFormProvider → useSuggestions
so the inline "Ask Duck.ai: <query>" entry can be hidden when the macOS
Settings → Private Search → "Autocomplete suggestions" toggle is off.
The Duck.ai mode pills remain governed by enableAi and are unaffected.
The new flag reacts live to omnibar_onConfigUpdate via the existing
config subscription, matching the enableAi / enableVoiceChatAccess
patterns.
Coordinates with apple-browsers PR #4958 which adds the field on the
native side.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Rephrase enableAskAiSuggestion description to drop native-impl detail
Addresses Cursor Bugbot finding: the "back-compat with older native
clients" phrasing in the schema description leaked native-side detail
into the generated TS type comment. Replaced with the more neutral
"for backward compatibility", matching the framing used by other
OmnibarConfig fields like enableVoiceChatAccess.
No behaviour change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
Web Compatibility Assessment
injected/src/features/web-compat.js, lines 1197-1202, warning: shimMode: "instanceOwn" preserves direct InputDeviceInfo.prototype identity, but deleting the configurable own getCapabilities shim falls back to the native brand-checked method on an Object.create() synthetic instance. That can reintroduce the original Illegal invocation failure mode for sites that delete or normalize own properties before calling capabilities.
injected/src/features/web-compat.js, lines 1167-1176, warning: the synthetic getCapabilities() returns {}. That fixes callability, but it is not a high-fidelity native MediaTrackCapabilities shape; sites branching on returned capability keys may still misbehave. This is config-gated, so it is likely acceptable with targeted rollout monitoring.
injected/src/features/api-manipulation.js, lines 153-165 and 241-244, info: method replacements and new setterValue functions are masked, but getterValue descriptors still expose a JS closure via Object.getOwnPropertyDescriptor(...).get.toString(). This is pre-existing for getters, but the expanded API manipulation surface makes it worth hardening if getterValue is used on detector-sensitive APIs.
Security Assessment
injected/src/features/api-manipulation.js, lines 50-61, error: descriptor validation reads optional fields through the prototype chain. A hostile page can pollute Object.prototype.value, setterValue, getterValue, enumerable, or configurable before init(), changing how trusted config changes validate or causing schema-valid protections to be skipped.
injected/src/wrapper-utils.js, lines 123-150, error: mergePropertyDescriptors() uses in against ordinary descriptor objects, and then checks if ('value' in merged). With Object.prototype.value/get/set pollution, an accessor merge can be misclassified as a data descriptor and passed to Object.defineProperty() with an inherited value.
injected/src/features/api-manipulation.js, lines 185-193, warning: prototype-chain traversal uses page-visible Object.getPrototypeOf after config init. Capturing/binding this alongside getOwnPropertyDescriptor would keep inherited descriptor lookup from depending on a mutable page global.
No message bridge, origin-validation, postMessage, iframe access, or nativeData trust-boundary changes found in this diff.
Risk Level
Critical Risk by the requested rubric: the PR changes captured-globals.js, wrapper-utils.js, API descriptor manipulation, and injected web-compat shims that can affect all injected platforms.
Recommendations
Replace descriptor/config optional-key checks with captured own-property checks, and add unit tests with polluted Object.prototype.value, get, set, setterValue, and configurable.
Sanitize merged descriptor objects with null prototypes or explicit own-key copies before deciding data vs accessor shape.
Capture Object.getPrototypeOf in captured-globals.js and use it in findPropertyDescriptor().
Add an integration/unit test for shimMode: "instanceOwn" plus delete device.getCapabilities.
If configs use getterValue on APIs that scripts probe, mask replacement getters against the original getter the same way setters are masked.
The reason will be displayed to describe this comment to others. Learn more.
These optional-field checks need to be own-property checks. apiChanges are plain objects processed after page JS can run, so a page can set Object.prototype.value/setterValue/getterValue/configurable before init(). The inherited field then changes hasValue/hasSetterValue or the boolean validation, which can make a valid protection config fail validation or apply as the wrong descriptor shape. Use captured hasOwnProperty.call(change, key) before reading these keys, and add a pollution regression test.
The reason will be displayed to describe this comment to others. Learn more.
This descriptor-kind check is using in on an ordinary object, so inherited descriptor keys are observable here. If a page polluted Object.prototype.value, an accessor merge with own get/set reaches this branch and returns a data descriptor with the inherited value, changing the descriptor shape passed to Object.defineProperty(). Please use own-key checks and/or build the merged descriptor as a null-prototype sanitized object before deciding data vs accessor shape.
The reason will be displayed to describe this comment to others. Learn more.
In instanceOwn mode the shim is an own configurable property. If page code deletes device.getCapabilities, lookup falls through to InputDeviceInfo.prototype.getCapabilities, but this object was built with Object.create() and does not have the native internal brand, so calling it reintroduces TypeError: Illegal invocation. Please add coverage for delete-then-call in this mode, or make the mode avoid exposing the native brand-checked method after deletion.
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-minorNew feature — triggers minor version bump
6 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.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
github-actions
Bot
added
semver-minor
Asana Task/Github Issue:
Description
Testing Steps
Checklist
Please tick all that apply:
Note
Medium Risk
Medium risk due to non-trivial changes to injected content-scope shimming logic (
apiManipulationdescriptor overriding andwebCompatsyntheticInputDeviceInfo.getCapabilities), which can affect site compatibility and fingerprinting surfaces; the TypeScript/tooling bumps are comparatively low risk.Overview
Tooling: Switches
tsconfig.jsontomoduleResolution: "bundler", bumps TypeScript/typedoc/types tooling deps, and updatestypedoc.jstyping;.gitignorerules are also tightened.Injected runtime shims: Extends
apiManipulationto support value-descriptor (method) replacement and setter overrides, including masking replacements to look native and safely handling inherited properties via descriptor merging (mergePropertyDescriptors). EnhanceswebCompatdevice enumeration by adding a maskedInputDeviceInfo.getCapabilitiesshim for synthetic devices (configurableshimMode), and expands Playwright/unit tests to validate these behaviors.Special pages: Adds
enableAskAiSuggestionconfig to the New Tab omnibar to optionally hide the inline “Ask Duck.ai” suggestion (with integration tests and docs/schema updates), adds new RMF big-single-action examples/icons (Preview,YoutubeNew), and updates onboarding locale strings for the YouTube row copy.Reviewed by Cursor Bugbot for commit 89da4c1. Bugbot is set up for automated code reviews on this repo. Configure here.