Skip to content

Api manipulation method values - #2703

Merged
jonathanKingston merged 17 commits into
mainfrom
jkt/auto/api-manipulation-method-values-60ce
May 20, 2026
Merged

Api manipulation method values#2703
jonathanKingston merged 17 commits into
mainfrom
jkt/auto/api-manipulation-method-values-60ce

Conversation

@jonathanKingston

@jonathanKingston jonathanKingston commented May 18, 2026

Copy link
Copy Markdown
Contributor

Asana Task/Github Issue: https://app.asana.com/1/137249556945/task/1214898456412755?focus=true

Description

This should allow remote configuration of value descriptors that are functions.

Testing Steps

Set the following config:

                    {
                        "condition": [
                            {
                                "domain": "claude.ai"
                            }
                        ],
                        "patchSettings": [
                            {
                                "op": "add",
                                "path": "/apiChanges/MediaDevices.prototype.getUserMedia",
                                "value": {
                                    "type": "descriptor",
                                    "getterValue": {
                                        "type": "function",
                                        "functionName": "debug"
                                    }
                                }
                            }
                        ]
                    },
image

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
Expands DOM API override capabilities (including methods and setters) and changes descriptor wrapping/definition behavior, which could affect site compatibility or detection resistance if edge cases are missed. Risk is mitigated by extensive new unit tests and schema validation coverage.

Overview
Enables remote apiManipulation config to override value descriptors (including methods) and setter-only/accessor overrides via new value and setterValue fields, with stricter validation to prevent mixing accessor/value descriptor shapes.

When overriding functions, replacements are now masked to preserve observable identity (toString(), name, length, including native-code appearance) and use captured globals (Reflect.apply, Object.defineProperty) to reduce page tampering. Inherited/prototype-chain properties can be overridden by shadow-defining an own property (without requiring define: true), using a new mergePropertyDescriptors helper shared by wrapProperty.

Updates the privacy-config dependency and adjusts an integration test config JSON shape; adds extensive unit tests for method replacement, inherited overrides, setter interception, and schema validation against updated privacy-configuration.

Reviewed by Cursor Bugbot for commit d2209bb. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions github-actions Bot added the semver-patch Bug fix / internal — no release needed label May 18, 2026
@github-actions

github-actions Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

[Beta] Generated file diff

Time updated: Wed, 20 May 2026 16:25:05 GMT

Android
    - android/adsjsContentScope.js
  • android/autofillImport.js
  • android/brokerProtection.js
  • android/contentScope.js
  • android/duckAiChatHistory.js
  • android/duckAiDataClearing.js
  • android/pages/duckplayer/dist/index.js

File has changed

Apple
    - apple/contentScope.js
  • apple/contentScopeIsolated.js
  • apple/duckAiChatHistory.js
  • apple/duckAiDataClearing.js
  • apple/pages/duckplayer/dist/index.js
  • apple/pages/duckplayer/index.html
  • apple/pages/history/dist/index.js
  • apple/pages/new-tab/dist/index.js
  • apple/pages/onboarding/dist/index.js
  • apple/pages/release-notes/dist/index.js
  • apple/pages/special-error/dist/index.js
  • apple/pages/special-error/index.html

File has changed

Chrome-mv3
    - chrome-mv3/inject.js

File has changed

Firefox
    - firefox/inject.js

File has changed

Integration
    - integration/contentScope.js
  • integration/pages/duckplayer/dist/index.js
  • integration/pages/example/dist/index.js
  • integration/pages/history/dist/index.js
  • integration/pages/new-tab/dist/index.js
  • integration/pages/onboarding/dist/index.js
  • integration/pages/release-notes/dist/index.js
  • integration/pages/special-error/dist/index.js

File has changed

Windows
    - windows/contentScope.js
  • windows/pages/duckplayer/dist/index.js
  • windows/pages/history/dist/index.js
  • windows/pages/new-tab/dist/index.js
  • windows/pages/onboarding/dist/index.js
  • windows/pages/release-notes/dist/index.js
  • windows/pages/special-error/dist/index.js
  • windows/pages/special-error/index.html

File has changed

@jonathanKingston

Copy link
Copy Markdown
Contributor Author

@claude Can you validate that if an existing value function descriptor exists, that we still can remotely override it with a new function descriptor (aka not with define: true)?

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Cursor assessed this PR as Medium Risk (only Low Risk is auto-approved).

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

@jonathanKingston

Copy link
Copy Markdown
Contributor Author

@cursoragent Can you validate that if an existing value function descriptor exists, that we still can remotely override it with a new function descriptor (aka not with define: true)?

@cursor

cursor Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

Validated and yes — an existing value function descriptor can be remotely overridden via a value-descriptor change without define: true. The path through wrapApiDescriptor skips the define-branch when either define !== true or the key already exists in the target, and then calls wrapProperty(), which merges the original descriptor with the new value. Since the new function overrides the original value key during the merge, the method is replaced. The pre-existing test replaces an existing method value descriptor if present already exercised this, but it used a plain assignment (obj.x = fn), which doesn't fully mimic a real DOM method descriptor.

I added two stronger unit tests in injected/unit-test/features.js:

  • replaces a DOM-style method value descriptor without define: true — installs the original method with Object.defineProperty({ writable: true, configurable: true, enumerable: false }) (DOM-method shape), applies a descriptor change with value only (no define), and asserts: the function reference is replaced, the new function returns the configured value, and writable/configurable/enumerable are preserved through the merge in wrapProperty.
  • overrides an existing method value descriptor even when define: true is set — confirms that define: true is gated by !(key in api), so on a property that already exists the change still falls through to wrapProperty and replaces the function rather than being skipped.

Both new tests pass alongside the existing 7 (9 specs, 0 failures). Lint passes on the touched file; the two remaining tsc errors are pre-existing (missing built locale modules unrelated to this PR). Pushed as commit b6966abe2 to jkt/auto/api-manipulation-method-values-60ce.

Open in Web Open in Cursor 

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>
@github-actions

github-actions Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

Build Branch

Branch pr-releases/jkt/auto/api-manipulation-method-values-60ce
Commit b2221a8ba0
Updated May 20, 2026 at 4:23:30 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/jkt/auto/api-manipulation-method-values-60ce

Swift Package Manager (Apple):

.package(url: "https://github.com/duckduckgo/content-scope-scripts.git", branch: "pr-releases/jkt/auto/api-manipulation-method-values-60ce")

git submodule (Windows):

git -C submodules/content-scope-scripts fetch origin pr-releases/jkt/auto/api-manipulation-method-values-60ce
git -C submodules/content-scope-scripts checkout origin/pr-releases/jkt/auto/api-manipulation-method-values-60ce
Pin to exact commit

npm (Android / Extension):

npm i github:duckduckgo/content-scope-scripts#b2221a8ba0fd645cd75c4cadf3bdd00261febdcd

Swift Package Manager (Apple):

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

git submodule (Windows):

git -C submodules/content-scope-scripts fetch origin pr-releases/jkt/auto/api-manipulation-method-values-60ce
git -C submodules/content-scope-scripts checkout b2221a8ba0fd645cd75c4cadf3bdd00261febdcd

Comment thread injected/src/features/api-manipulation.js Outdated
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>
Comment thread injected/src/features/api-manipulation.js Outdated
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>
Comment thread injected/src/features/api-manipulation.js
@jonathanKingston
jonathanKingston requested a review from laghee May 19, 2026 14:33
@jonathanKingston
jonathanKingston marked this pull request as ready for review May 19, 2026 14:34
@jonathanKingston
jonathanKingston requested a review from a team as a code owner May 19, 2026 14:34

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

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/features/api-manipulation.js:59-61 - warning: The runtime now accepts descriptor changes with value, but the bundled @duckduckgo/privacy-configuration schema still defines DescriptorAPIChange as requiring getterValue and disallowing additional value properties. I validated a minimal apiManipulation config with a method value descriptor against createValidator('CurrentGenericConfig'), and it fails with additionalProperty: value plus missing getterValue. That makes the new method-value path undeployable via the normal schema-validated remote-config flow, or forces any rollout outside the expected typed config contract.

Security Assessment

  • injected/src/features/api-manipulation.js:59-61 - warning: This is a configuration-trust gap rather than a page-exploitable primitive: runtime validation only checks that exactly one of getterValue/value exists, while the schema that should validate the nested ConfigSetting shape has not been updated in this repo. If a config bypassed the current schema, malformed value settings would be filtered only by processAttr() defaults instead of the privacy-config schema boundary.
  • No new messaging, origin-validation, network, iframe, or message-bridge security issues identified beyond the existing visible review comments.

Risk Level

Critical Risk - the PR changes captured-globals.js and expands config-driven replacement of DOM method-valued descriptors across injected platforms.

Recommendations

  1. Update/bump the @duckduckgo/privacy-configuration schema so DescriptorAPIChange permits exactly one of getterValue or value, both typed as CSSConfigSetting.
  2. Add a schema-validation fixture or unit test containing a method value descriptor so this delivery path stays covered.
  3. Keep the existing method identity/captured-global tests; npm run test-unit --workspace=injected -- --filter='ApiManipulation' passes locally.
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

@laghee laghee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could add some logging in here in the future to prevent potential dev confusion (e.g., Cursor claims accidentally wrapping an instance instead of the prototype would silently fail, as do invalid API changes), but otherwise LGTM

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.
@github-actions github-actions Bot added semver-minor New feature — triggers minor version bump and removed semver-patch Bug fix / internal — no release needed labels May 20, 2026
Comment thread injected/src/features/api-manipulation.js Outdated
Comment thread injected/src/features/api-manipulation.js
Comment thread injected/src/features/api-manipulation.js
Comment thread injected/src/features/api-manipulation.js
Comment thread injected/src/features/api-manipulation.js Outdated
Comment thread injected/src/features/api-manipulation.js Outdated
Comment thread injected/src/features/api-manipulation.js
Comment thread injected/src/features/api-manipulation.js Outdated
Comment thread injected/src/features/api-manipulation.js
@jonathanKingston
jonathanKingston force-pushed the jkt/auto/api-manipulation-method-values-60ce branch from 0658319 to c7be658 Compare May 20, 2026 14:31
Comment thread injected/src/features/api-manipulation.js Outdated
Comment thread injected/src/features/api-manipulation.js
Comment thread injected/src/features/api-manipulation.js
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.
Restore unchanged define semantics from c7be658. Value/getterValue/setterValue
and method masking remain; inherited APIs still require define: true to shadow-define.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

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/features/api-manipulation.js:182-189 - error: findPropertyDescriptor() uses page-visible Object.getPrototypeOf while walking inherited DOM descriptors. apiManipulation.init() runs after page script can tamper globals, so a page/polyfill can make inherited descriptor changes throw, return the wrong prototype, or loop, leaving the configured API surface unmodified on that site.
  • injected/src/features/api-manipulation.js:207-217 - error: maskMethodReplacement() still uses a fresh normal function as the replacement target. That preserves toString(), .name, and .length, but changes native DOM method constructability and exposes an own prototype, detectable via Object.prototype.hasOwnProperty.call(fn, 'prototype') or new fn().
  • injected/src/features/api-manipulation.js:110-130 - warning: Runtime validation accepts value and setterValue, but the bundled @duckduckgo/privacy-configuration schema still only defines getterValue for descriptor changes. These new config shapes remain outside the normal schema-validated remote-config contract.

Security Assessment

  • injected/src/features/api-manipulation.js:182-189 - error: The uncaptured Object.getPrototypeOf read is a hostile-page bypass for the new inherited method/accessor shadowing path. A page that reassigns Object.getPrototypeOf before config init can prevent protections such as MediaDevices.prototype.addEventListener suppression from installing, or can hang the main thread by returning the same object repeatedly. Use a captured/bound primitive from captured-globals.js (for example ReflectGetPrototypeOf = Reflect.getPrototypeOf.bind(Reflect)) for the traversal.
  • injected/src/features/api-manipulation.js:110-130 - warning: The runtime/schema mismatch is a configuration-trust gap: accepted descriptor shapes no longer match the schema boundary that should validate nested ConfigSetting values.
  • No new messaging, origin-validation, network, iframe, postMessage, or message-bridge issues identified.

Risk Level

Critical Risk - this PR changes captured-globals.js and expands config-driven DOM descriptor replacement across all injected platforms, including new prototype-chain traversal in a hostile page context.

Recommendations

  1. Capture and use a bound prototype traversal primitive in captured-globals.js, then add a unit test that tampers Object.getPrototypeOf after feature construction and verifies inherited descriptor overrides still apply.
  2. Preserve method constructability/prototype shape by using the original function as the proxy target with an apply trap that invokes the configured replacement via captured ReflectApply; add tests for own prototype parity and new fn() behavior.
  3. Update/bump @duckduckgo/privacy-configuration so DescriptorAPIChange permits the final mutually exclusive data (value) and accessor (getterValue/setterValue) shapes, with schema fixtures covering both.

Validation: npm run test-unit --workspace=injected -- --filter='ApiManipulation' passed (20 specs, 0 failures). A one-off hostile-global check confirmed the current inherited-descriptor path throws when Object.getPrototypeOf is tampered.

Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

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/features/api-manipulation.js:141-142 - error: The define: true inherited-shadow path creates the own descriptor from the configured replacement only, then returns before inspecting the inherited native descriptor. For inherited methods like MediaDevices.prototype.addEventListener, the replacement is not masked against EventTarget.prototype.addEventListener and descriptor flags default instead of preserving the inherited method shape. For inherited accessor properties with setterValue only, this also shadows the inherited getter so reads become undefined. This violates descriptor/API surface fidelity for the inherited APIs this path is meant to support.
  • injected/src/features/api-manipulation.js:190-195 - error: maskMethodReplacement() uses a fresh normal function as the callable target. That preserves toString(), .name, and .length, but it changes native DOM method constructability and exposes an own prototype on methods that are usually non-constructable. Page scripts can detect the override via Object.prototype.hasOwnProperty.call(fn, 'prototype') or new fn().
  • injected/src/features/api-manipulation.js:59-66 - warning: Runtime validation accepts value and setterValue, but the bundled @duckduckgo/privacy-configuration schema still defines descriptor changes as getterValue-only. The new config shapes remain outside the normal schema-validated remote-config contract.

Security Assessment

  • injected/src/features/api-manipulation.js:169 - warning: The new method/setter replacement path is installed through this.wrapProperty(), which routes into ContentFeature.defineProperty()'s outer debug proxy. That proxy still invokes the replacement through mutable Reflect.apply on the captured Reflect object, so a hostile page that reassigns Reflect.apply before config init or before invocation can intercept/suppress the end-to-end configured replacement despite the inner ReflectApply helper.
  • injected/src/features/api-manipulation.js:59-66 - warning: The schema/runtime mismatch is a configuration-trust gap rather than a direct page-exploitable primitive: accepted descriptor shapes no longer match the schema boundary that should validate nested ConfigSetting values.
  • No new messaging, origin-validation, network, iframe, postMessage, or message-bridge issues identified.

Risk Level

Critical Risk - this changes captured-globals.js and expands config-driven DOM descriptor replacement/definition across injected platforms.

Recommendations

  1. For inherited define: true shadows, derive from the inherited descriptor using captured/bound prototype traversal, then overlay configured value/get/set, preserving complementary accessor halves, flags, and masking against the inherited native method/setter.
  2. Preserve method constructability/prototype shape by proxying the original function target with an apply trap that calls the configured replacement via captured ReflectApply, instead of wrapping a fresh normal function.
  3. Use a bound ReflectApply in the outer ContentFeature.defineProperty() debug proxy, or otherwise ensure api-manipulation method/setter replacements do not re-enter page-mutable Reflect.apply at call time.
  4. Update/bump @duckduckgo/privacy-configuration so DescriptorAPIChange permits the final mutually exclusive data (value) and accessor (getterValue/setterValue) shapes, with schema fixtures for both.

Validation: npm run test-unit --workspace=injected -- --filter='ApiManipulation' passed (19 specs, 0 failures).

Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

…nfig

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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

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/features/api-manipulation.js:182-190 - error: findPropertyDescriptor() walks inherited DOM descriptors with page-visible Object.getPrototypeOf. Because apiManipulation.init() runs after page script can mutate globals, a site/polyfill can make inherited descriptor overrides throw, return the wrong prototype, or loop. This can leave the intended MediaDevices.prototype.addEventListener-style API surface unmodified on affected pages.
  • injected/src/features/api-manipulation.js:207-217 - error: maskMethodReplacement() still uses a fresh normal function as the callable target. That preserves toString(), .name, and .length, but changes native DOM method constructability and exposes an own prototype on methods that are usually non-constructable. A probe with Object.prototype.hasOwnProperty showed the replacement has an own prototype and is constructable.
  • injected/src/features/api-manipulation.js:50-66 - warning: Descriptor validation reads optional fields through the prototype chain ('enumerable' in change, change.value, change.setterValue, etc.). If a page or legacy library pollutes Object.prototype, valid config descriptors can be misclassified before installation. For example, Object.prototype.value = ... makes a valid getterValue descriptor fail validation.

Security Assessment

  • injected/src/features/api-manipulation.js:182-190 - error: The uncaptured Object.getPrototypeOf read is a hostile-page bypass for the new inherited descriptor path. A page that reassigns it before config init can prevent protections from installing or hang traversal. Use a captured/bound primitive from captured-globals.js.
  • injected/src/features/api-manipulation.js:50-66 - warning: Prototype-polluted inherited config fields let page code influence the runtime validation boundary for remote-config changes. Use captured hasOwnProperty.call(change, key) before reading optional descriptor fields.
  • injected/src/features/api-manipulation.js:167-172 - warning: The final installed value/setter still routes through ContentFeature.defineProperty()'s debug proxy, which calls mutable Reflect.apply at invocation time. That leaves the end-to-end method/setter replacement path interceptable if the page tampers Reflect.apply, despite the inner helper using captured ReflectApply.
  • No new messaging, origin-validation, network, iframe, postMessage, or message-bridge issues identified. The schema deployability issue from earlier reviews appears addressed by the privacy-configuration bump and fixture.

Risk Level

Critical Risk - the PR changes captured-globals.js and expands config-driven DOM descriptor replacement/definition across injected platforms, including inherited prototype traversal in hostile page contexts.

Recommendations

  1. Add a captured/bound prototype traversal primitive, e.g. ReflectGetPrototypeOf = Reflect.getPrototypeOf.bind(Reflect), use it in findPropertyDescriptor(), and add a tampered-Object.getPrototypeOf test.
  2. Base descriptor validation on own properties only via captured hasOwnProperty.call(...); add an Object.prototype.value pollution regression test.
  3. Preserve method constructability/prototype shape by proxying the original function target with an apply trap that invokes the configured replacement via captured ReflectApply.
  4. Use captured ReflectApply in the outer ContentFeature.defineProperty() debug proxy, or avoid re-wrapping already-masked api-manipulation method/setter replacements through page-mutable Reflect.apply.

Validation: npm run test-unit --workspace=injected -- --filter='ApiManipulation' passes (21 specs, 0 failures).

Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

Comment thread injected/src/features/api-manipulation.js Outdated
Comment thread injected/src/features/api-manipulation.js
…apes

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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

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/features/api-manipulation.js:187-194 - error: findPropertyDescriptor() walks inherited DOM descriptors via page-visible Object.getPrototypeOf. apiManipulation.init() runs after page script can mutate globals, so a site/polyfill can make inherited descriptor overrides throw, return the wrong prototype, or loop. That can leave MediaDevices.prototype.addEventListener-style API patches uninstalled on affected pages.
  • injected/src/features/api-manipulation.js:212-222 - error: maskMethodReplacement() uses a fresh normal function as the callable target. It masks toString(), .name, and .length, but changes native method constructability and adds an own prototype for DOM methods that are normally non-constructable. This remains detectable via Object.prototype.hasOwnProperty.call(fn, 'prototype') or new fn().
  • injected/src/features/api-manipulation.js:49-66 - warning: Descriptor validation reads optional config fields through the prototype chain ('enumerable' in change, change.value, change.setterValue, etc.). If page code or a legacy library pollutes Object.prototype, valid descriptors can be misclassified before installation.

Security Assessment

  • injected/src/features/api-manipulation.js:187-194 - error: The uncaptured Object.getPrototypeOf read is a hostile-page bypass for the new inherited descriptor path. A page can prevent protections from installing or hang traversal. Use a captured/bound primitive from captured-globals.js, e.g. ReflectGetPrototypeOf = Reflect.getPrototypeOf.bind(Reflect).
  • injected/src/features/api-manipulation.js:49-66 - warning: Prototype-polluted inherited config fields let page code influence the runtime validation boundary for remote-config changes. Optional descriptor fields should be checked with captured hasOwnProperty.call(change, key) before reading them.
  • injected/src/content-feature.js:469-476 - warning: The installed value/setter still routes through ContentFeature.defineProperty()'s debug proxy, whose apply trap calls mutable Reflect.apply at invocation time. That leaves the end-to-end replacement path interceptable if the page tampers Reflect.apply, despite the inner api-manipulation helper using captured ReflectApply.
  • No new messaging, origin-validation, network, iframe, postMessage, or message-bridge issues identified. The latest commit appears to have fixed the inherited descriptor cross-shape merge issue noted earlier.

Risk Level

Critical Risk - the PR changes captured-globals.js and expands config-driven DOM descriptor replacement across injected platforms, including prototype-chain traversal in a hostile page context.

Recommendations

  1. Capture and use a bound prototype traversal primitive, then add a tampered-Object.getPrototypeOf regression test for inherited overrides.
  2. Base descriptor validation on own properties only via captured hasOwnProperty.call(...); add an Object.prototype.value pollution test.
  3. Preserve method constructability/prototype shape by proxying the original function target with an apply trap that invokes the configured replacement via captured ReflectApply.
  4. Use captured ReflectApply in the outer ContentFeature.defineProperty() debug proxy, or avoid re-wrapping already-masked api-manipulation replacements through page-mutable Reflect.apply.
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

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.
Normalize merged descriptors so tsc accepts them at defineProperty and
wrapProperty call sites. No remote config change required.
privacy-configuration #1779290401870 expects inputFieldFocusDetection
settings as { state } object, not a bare string.
Comment thread injected/src/features/api-manipulation.js Outdated
Comment thread injected/src/features/api-manipulation.js Outdated
Comment thread injected/src/wrapper-utils.js
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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Web Compatibility Assessment

  • injected/src/features/api-manipulation.js:185-193 - error: findPropertyDescriptor() walks inherited DOM descriptors with page-visible Object.getPrototypeOf. apiManipulation.init() runs after page scripts can mutate globals, so a site/polyfill can make inherited descriptor overrides throw, return the wrong prototype, or loop. A one-off check with tampered Object.getPrototypeOf currently throws before the inherited override is installed.
  • injected/src/features/api-manipulation.js:210-220 - error: maskMethodReplacement() uses a fresh normal function as the callable target. It masks toString(), .name, and .length, but changes native DOM method constructability and exposes an own prototype; the current wrapper is constructable and Object.prototype.hasOwnProperty.call(fn, 'prototype') is true. That violates API surface fidelity for method-value overrides.
  • injected/src/features/api-manipulation.js:50-66 - warning: descriptor validation checks optional config fields through the prototype chain ('enumerable' in change, direct change.value / change.setterValue reads). Page-side Object.prototype pollution can misclassify otherwise valid descriptor configs before installation.
  • injected/src/wrapper-utils.js:123-148 - error: mergePropertyDescriptors() also uses inherited descriptor keys and returns normal object-literal descriptors. With Object.prototype.value polluted, an accessor/accessor merge currently returns an own data descriptor using the polluted value, changing the descriptor shape that gets installed.

Security Assessment

  • injected/src/features/api-manipulation.js:185-193 - error: the uncaptured Object.getPrototypeOf read is a hostile-page bypass for the new inherited descriptor path; page code can prevent protections from installing or hang traversal.
  • injected/src/features/api-manipulation.js:50-66 - warning: inherited config fields let page-side prototype pollution influence the runtime validation boundary for remote-config changes. Optional fields should be tested with captured hasOwnProperty.call(...) before reading.
  • injected/src/wrapper-utils.js:123-148 - error: page-side prototype pollution can influence the descriptor C-S-S installs. Descriptor-kind checks and the final descriptor objects need to avoid inherited value/get/set participation in Object.defineProperty's descriptor conversion.
  • injected/src/content-feature.js:469-476 - warning: installed value/setter replacements still route through the debug proxy, whose apply trap calls mutable Reflect.apply at invocation time. This is reached from the new method/setter replacement path despite the inner helper using captured ReflectApply.
  • No new messaging, origin-validation, network, iframe, postMessage, or message-bridge issues identified in the current head.

Risk Level

Critical Risk - this PR changes captured-globals.js and expands config-driven DOM descriptor replacement across injected platforms, including inherited prototype traversal in a hostile page context.

Recommendations

  1. Capture and use a bound prototype traversal primitive, e.g. ReflectGetPrototypeOf = Reflect.getPrototypeOf.bind(Reflect), and add a tampered-Object.getPrototypeOf regression test.
  2. Preserve method constructability/prototype shape by proxying the original function target with an apply trap that invokes the configured replacement via captured ReflectApply; add tests for own prototype parity and new fn() behavior.
  3. Make descriptor validation and descriptor merging own-property-only via captured hasOwnProperty.call(...).
  4. Return null-prototype descriptor objects, or otherwise guarantee inherited value/get/set cannot participate in descriptor conversion; add tests for Object.prototype.value on accessor merges and Object.prototype.get/set on data merges.
  5. Use captured ReflectApply in the outer debug proxy, or avoid re-wrapping already-masked api-manipulation replacements through page-mutable Reflect.apply.

Validation: npm run test-unit --workspace=injected -- --filter='ApiManipulation' passes (24 specs, 0 failures); targeted one-off probes reproduced the three hostile-page/API-surface gaps above.

Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

Comment thread injected/src/features/api-manipulation.js
Comment thread injected/src/features/api-manipulation.js
Comment thread injected/src/wrapper-utils.js
Comment thread injected/src/features/api-manipulation.js

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d2209bb. Configure here.

Comment thread injected/src/features/api-manipulation.js

@GuiltyDolphin GuiltyDolphin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Discussed in call:

  • both suspicious of the shadow define, but willing to proceed
  • if the YT/linkedin/claude changes cause any problems we can roll back via config
  • if existing API overrides are affected then this is harder to roll back via config

Config change is merged so this should apply once Windows adopts new CSS version.

Once you've finished running the final validations re existing APIs and have no concerns, feel free to merge.

@jonathanKingston
jonathanKingston added this pull request to the merge queue May 20, 2026
Merged via the queue into main with commit b3ee203 May 20, 2026
54 checks passed
@jonathanKingston
jonathanKingston deleted the jkt/auto/api-manipulation-method-values-60ce branch May 20, 2026 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-minor New feature — triggers minor version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants