Skip to content

fix(js-instrument): match native function arity and name on wrappers - #1210

Draft
vringar wants to merge 1 commit into
masterfrom
fix/legacy-instrument-arity-name
Draft

fix(js-instrument): match native function arity and name on wrappers#1210
vringar wants to merge 1 commit into
masterfrom
fix/legacy-instrument-arity-name

Conversation

@vringar

@vringar vringar commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

What

The legacy JavaScript instrument (Extension/src/lib/js-instruments.ts) replaces
native methods with a closure wrapper (instrumentFunction). The wrapper's
page-observable length (arity) and name did not match the native
function it wraps: every wrapped method reported length === 0 and name === "",
regardless of the native value. Both are trivially readable from page script and
make instrumented methods fingerprintable.

This PR copies the native function's own length and name property descriptors
onto the wrapper (preserving the native non-writable / non-enumerable /
configurable shape), so the wrapper matches native on these two axes.

Empirical before/after (direct-selenium, FF152, no extension)

Repro injects the real compiled getInstrumentJS (via .toString(), exactly
as the extension's ${getInstrumentJS} template-literal does) as an inline
<script>, headless, no extension. It instruments concrete native methods and
reads the wrapper's .length/.name:

method property native wrapped (before) wrapped (after)
CanvasRenderingContext2D.prototype.getImageData .length 4 0 4
CanvasRenderingContext2D.prototype.getImageData .name "getImageData" "" "getImageData"
HTMLCanvasElement.prototype.toDataURL .name "toDataURL" "" "toDataURL"

Both artifacts reproduced before the fix and are gone after. Instrumentation
still logs the calls in both runs (no capture regression).

Regression test

test/test_js_instrument.py::TestJSInstrumentFunctionArityAndName instruments
window.fetch in a real browser, reads the wrapper's observable .length/.name
in-page, and encodes them into a fetch call argument so they land in the
javascript table. Native fetch has arity 1 and name "fetch"; the test
asserts the logged call argument is https://arity-1.example.com/name-fetch.

Verified the negative: with the fix disabled (wrapper left unchanged) the test
fails (observed arity-0 / name-), confirming it is a genuine guard. The
full test/test_js_instrument.py suite passes (8 tests).

Out of scope

The separate proto-chain flattening artifact (#56 sub-item iii) is not
addressed here. For the legacy plain-JS-wrapper architecture, de-polluting the
flatten would silently drop inherited-member capture that the bundled
collection_fingerprinting relies on (per-interface attribution of inherited
members is structurally tied to the per-leaf copy-down). It is tradeoff-blocked
and intentionally left untouched.

Refs crosslink #56.

@codecov

codecov Bot commented Jun 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.20%. Comparing base (61d285f) to head (7bef7e2).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1210      +/-   ##
==========================================
- Coverage   62.22%   62.20%   -0.03%     
==========================================
  Files          40       40              
  Lines        3929     3929              
==========================================
- Hits         2445     2444       -1     
- Misses       1484     1485       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vringar
vringar force-pushed the fix/legacy-instrument-arity-name branch from 601eb76 to 4396c14 Compare June 30, 2026 18:11
@vringar
vringar force-pushed the fix/legacy-instrument-arity-name branch from 4396c14 to 1d0ba91 Compare July 20, 2026 22:54
vringar added a commit that referenced this pull request Jul 20, 2026
…-instruments mitigation (do-not-merge)

Runs PR #1210's TestJSInstrumentFunctionArityAndName against un-fixed master
(Extension/src/lib/js-instruments.ts is NOT patched). The wrapper closure
reports length 0 / name "" so the instrumented fetch call is expected to log
arity-0.example.com/name- instead of arity-1.example.com/name-fetch, making the
tests shard fail. This failure is the proof that the test is a genuine guard.

Do-not-merge verification branch.
The legacy JavaScript instrument replaces native methods with a closure
wrapper. The wrapper's page-observable `length` (arity) and `name` did not
match the native function: a wrapped method reported `length === 0` and
`name === ""` regardless of the native value. This is trivially detectable
from page script (e.g. native `getImageData.length === 4`,
`getImageData.name === "getImageData"` vs the wrapper's `0` / `""`).

Copy the native function's own `length` and `name` descriptors onto the
wrapper, preserving the native non-writable / non-enumerable / configurable
shape, so the wrapper is indistinguishable from native on these two axes.

Adds a browser-level regression test that instruments `window.fetch` and
asserts the wrapper reports the native arity (1) and name ("fetch").

The separate proto-chain flattening artifact is out of scope: de-polluting it
would silently drop inherited-member capture that collection_fingerprinting
relies on (tradeoff-blocked).
vringar added a commit that referenced this pull request Aug 25, 2026
…-instruments mitigation (do-not-merge)

Runs PR #1210's TestJSInstrumentFunctionArityAndName against un-fixed master
(Extension/src/lib/js-instruments.ts is NOT patched). The wrapper closure
reports length 0 / name "" so the instrumented fetch call is expected to log
arity-0.example.com/name- instead of arity-1.example.com/name-fetch, making the
tests shard fail. This failure is the proof that the test is a genuine guard.

Do-not-merge verification branch.
@vringar
vringar force-pushed the fix/legacy-instrument-arity-name branch from 1d0ba91 to 7bef7e2 Compare August 25, 2026 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant