Conversation
b39d60c to
8f03b08
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The new webdriver spoof test doesn’t reliably clean up the TaskManager on failure paths (missing try/finally), which can leak browser processes and destabilize the test suite.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a small, standalone anti-detection mitigation to OpenWPM by spoofing navigator.webdriver to false (default-on) via a Firefox content script injected at document_start, while also tightening mypy’s execution environment so type-checking reflects the real conda-installed dependencies.
Changes:
- Add
BrowserParams.spoof_webdriver(defaultTrue) and wire it through to the WebExtension, which registers a newdocument_startcontent script that patchesNavigator.prototype.webdrivervia Xray waiver +exportFunction. - Add a targeted regression test page + Python test to verify the spoof flips only the value while preserving descriptor shape and prototype property order, and update JS-instrument expectations accordingly.
- Make mypy run as a
language: systempre-commit hook and addscripts/update.pyautomation to keep[tool.mypy].python_versionsynced to the conda Python version.
File summaries
| File | Description |
|---|---|
openwpm/config.py |
Adds spoof_webdriver BrowserParam and documents behavior/escapes. |
Extension/src/webdriver-spoof.ts |
Implements the page-visible spoof by replacing Navigator.prototype.webdriver getter. |
Extension/src/background/webdriver-spoof.ts |
Registers the spoof content script across all frames at document_start. |
Extension/src/feature.ts |
Wires spoof_webdriver into extension startup ordering (before JS instrument). |
Extension/src/types/xray.d.ts |
Declares Firefox-only wrappedJSObject/exportFunction for TS typechecking. |
Extension/webpack.config.js |
Adds a webpack entry to bundle the new content script. |
Extension/eslint.config.mjs |
Ignores the new bundled output file. |
Extension/.prettierignore |
Ignores the new bundled output file. |
Extension/.gitignore |
Ignores the new bundled output file. |
test/test_pages/webdriver_spoof.html |
Adds an inline, parse-time probe page used for verification. |
test/test_webdriver_spoof.py |
Adds end-to-end test validating spoof value flip without detectable shape changes. |
test/test_js_instrument.py |
Updates expectation: instrument should record spoofed navigator.webdriver value. |
docs/Configuration.md |
Documents spoof_webdriver, default, mechanism, and known escapes. |
.pre-commit-config.yaml |
Switches mypy to a system hook (conda env) with rationale. |
pyproject.toml |
Updates mypy python_version to match conda Python feature level. |
scripts/update.py |
Drops mypy rev syncing and adds sync_mypy_python_version() to prevent drift. |
test/test_update_script.py |
Updates/update-tests for mypy hook changes and new python_version syncing. |
Review details
- Files reviewed: 16/17 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| manager, _ = task_manager_creator((manager_params, browser_params)) | ||
| sequence = CommandSequence(server.base + TEST_PAGE) | ||
| sequence.get() | ||
| sequence.append_command(DumpWebdriverProbeCommand()) | ||
| manager.execute_command_sequence(sequence) | ||
| manager.close() | ||
| return json.loads((data_directory / PROBE_FILE).read_text()) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1230 +/- ##
==========================================
- Coverage 62.34% 61.59% -0.75%
==========================================
Files 40 41 +1
Lines 3930 3992 +62
==========================================
+ Hits 2450 2459 +9
- Misses 1480 1533 +53 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
8f03b08 to
446fe4f
Compare
|
Updated: the coverage claims in the original description were inherited from #526 (2019) and the stealth instrument's D10 notes rather than measured. I tested them on Firefox 155 and both are stale.
So the I also tried Unrelated finding worth keepingChasing the above raised the question of whether OpenWPM's content scripts reach The Docs, the |
90f561f to
62f9ca8
Compare
20cabcc to
1ef9ea0
Compare
Selenium sets `navigator.webdriver = true` on every page OpenWPM visits. It is a one-line check, it is the loudest signal that a visit is automated, and it is independent of everything else an anti-detection instrument does -- which makes it worth isolating. New `BrowserParams.spoof_webdriver`, default `True`. It records nothing and needs no instrument, so it can be toggled on its own to measure how much of a site's differential treatment of a crawl is attributable to this one signal. A privileged window actor replaces the page's `Navigator.prototype.webdriver` getter with one compiled in the page's compartment via `Cu.exportFunction`. The replacement is matched to the native accessor on every axis a page can inspect: `[native code]` from `toString()`, the native name `get webdriver`, the descriptor flags, the absent setter, the getter's own property order, and its behaviour on a wrong receiver. The last two are easy to get wrong -- `exportFunction` installs `name` before `length` where a native accessor reports the reverse, and a plain `() => false` returns `false` where the native getter throws a TypeError -- so receiver validation is delegated to the native getter and the two properties are redefined in native order. Why an actor rather than a content script ----------------------------------------- An earlier revision used a content script. It covers almost everything, but not a frame's *uncommitted initial about:blank* -- the placeholder between `appendChild` and the real document committing. Firefox deliberately never injects content scripts there (ExtensionContent.sys.mjs, isUncommittedInitialDocument, bug 1415539), and no configuration changes it. That document is observable: a page that appends a `srcdoc` iframe and reads `frame.contentWindow.navigator.webdriver` in the same task reads exactly it. The actor patches each realm from `content-document-global-created`, which fires for every window global including that one. There is a narrower mechanism for this specific flag -- `Navigator::Webdriver()` reads Marionette/RemoteAgent state out of `sharedData`, which the parent process can clear, leaving the native getter untouched. It is deliberately not used here: it can only move that one flag, and the instrument this is a slice of needs to overwrite arbitrary properties in the page and to run at realm creation. Matching the accessor exactly is the part that generalises. What the actor costs -------------------- `resource:` is the only scheme the system ESM loader accepts for an actor module; `moz-extension:` and the XPI's own `jar:` URL are silently declined, with no error, because JSWindowActorProtocol::Observe fetches the actor with IgnoreErrors(). `safeForUntrustedWebProcess` is mandatory or the actor is never instantiated for web content. The `resource:` mapping is registered in each content process by bootstrap.js rather than relying on the parent-side registration alone. The handler does propagate it (SubstitutingProtocolHandler::SendSubstitution), and the shipped webcompat about-compat API relies on exactly that, but it propagates over IPC asynchronously with respect to the actor registration: registering the actor immediately after setSubstitution was measured to fail with a bare "Failed to load resource://openwpm/...", because a content process is asked for the module before the mapping lands. webcompat avoids this by doing its setup in onStartup(), long before anything needs the mapping. Processes that already exist when the bootstrap is registered do not pick it up, so the preallocated pool is replaced rather than patched, by toggling `dom.ipc.processPrelaunch.enabled` once at startup. That pref is therefore a requirement: turning it off while the spoof is on is a ConfigError rather than a crawl with some content processes unhooked. `enable()` throws on any failure. A crawl that silently lost the spoof would produce plausible-looking data with the automation flag still visible to sites. Verification ------------ `test_spoof_webdriver_reaches_other_realms` checks every realm the page can reach against a `spoof_webdriver=False` baseline: pop-ups, an iframe appended and read in the same task, `about:blank` and `srcdoc` frames, indexed frame access, and calling a frame's own accessor directly. `test_spoof_webdriver_is_invisible_to_the_page` pins that nothing else about the accessor moves, including the property order and wrong-receiver behaviour above. Both fail against a forwarder that gets those wrong, so they guard the implementation rather than restating it. Also adds a test that instrumentation reaches `srcdoc` frames at all, and a regression test for the preallocation requirement. The spoof is enabled before the JavaScript instrument so that, with both on, the instrument records every read of `navigator.webdriver` with the value the page actually saw. That is why TestJSInstrumentByPython now expects `false`.
1ef9ea0 to
f7f90ce
Compare
What this is
navigator.webdriverreads asfalsein every realm the page can reach. NewBrowserParams.spoof_webdriver, defaultTrue.This is deliberately the smallest useful slice of #1154.
navigator.webdriveris the one detection vector in that PR's D1–D10 suite that is independent of instrumentation — it is true whether or not OpenWPM instruments anything, because Selenium sets it. Splitting it out makes it possible to measure how much of a crawl's differential treatment by sites is attributable to this one flag versus the rest of the stealth instrument's surface.It also serves a second purpose: it is a complete, reproducible demonstration of what "cover every realm" actually costs on current Firefox. Ported from #526, open since 2019.
The mypy fix that used to ride along here is now #1236.
Closes #526.
Mechanism
A privileged window actor replaces the page's
Navigator.prototype.webdrivergetter:The getter is compiled inside the page's compartment, so it reports
[native code]under the native accessor name. The native descriptor is reused apart fromget, so nothing else moves. Nothing in the content realm is hooked — no patchedHTMLIFrameElement.prototype, no proxy — so the spoof adds no page-observable surface of its own.Why the prototype and not the instance
Object.defineProperty(navigator, "webdriver", {value: false})adds an own property thenavigatorinstance does not normally have and reorders enumeration — the tell that led #526 to proxy the wholenavigatorobject. Replacing the accessor where Firefox defines it avoids both.Why an actor and not a content script
An earlier revision of this PR used a content script. It covers almost everything, but not a frame's uncommitted initial
about:blank— the placeholder that exists betweenappendChildand the real document committing. Firefox deliberately never injects content scripts there (bug 1415539, cited inExtensionContent.sys.mjs), and no configuration changes it:matchAboutBlankdoes not reach it, andmatchOriginAsFallbackdisablesmatchAboutBlankwhile requiring a wildcard path glob<all_urls>does not provide.That document is not merely transient — it is observable. A page can append a
srcdociframe and readframe.contentWindow.navigator.webdriverin the same task and get the uncommittedabout:blank, uninstrumented. I verified this by reading back the realm the parent actually sees: in both the plain-iframe and srcdoc same-task caseslocation.hrefisabout:blank, and only the srcdoc one is unpatched.The distinction that matters:
Those differ exactly here, and only the second is sufficient for a measurement tool. The actor works below the content-script layer: its child observes
content-document-global-created, which fires synchronously for every window global, uncommitted ones included.What full coverage costs
Documented because the stealth work will hit all of it:
resource:is the only scheme the system ESM loader accepts for an actor module.moz-extension:and the XPI's ownjar:URL are both silently declined — no error, the actor simply never runs.resource:substitution is per-process. Registering it only in the parent leaves content processes unable to resolve the module, reported as a bareFailed to load <uri>.bootstrap.jsre-registers it per process.safeForUntrustedWebProcess: trueis mandatory, orregisterWindowActorsucceeds, reports nothing, and the actor is never instantiated for web content.process-type-setand pushing a script when a process is claimed does not fix it: that fires in the parent while the process is already loading, and the IPC can lose. Draining the stale pool once at startup does, and preallocation stays enabled for the rest of the crawl.Verification
test_spoof_webdriver_reaches_other_realmschecks every reachable realm against aspoof_webdriver=Falsebaseline:window.open(""),window.open("about:blank"), an iframe appended and read in the same task,about:blankandsrcdocframes, indexed frame access viawindow[n], and calling a frame's ownNavigator.prototypegetter directly. All spoofed — including the uncommitted-about:blankcase no content script can reach.test_spoof_webdriver_is_invisible_to_the_pagepins that nothing else moves:descriptor.get.toString(),.name, the descriptor flags, the absent setter,hasOwnProperty(navigator, "webdriver"), and the fullObject.getOwnPropertyNames(Navigator.prototype)array are byte-identical to baseline.Both escapes named in the earlier literature are closed on Firefox 155 — #526's
window.open("")case and the synchronous-iframe race from #1154's D10 notes — measured rather than inherited.Run green locally against the unbranded Firefox 155 build this repo pins:
test_webdriver_spoof.py+test_js_instrument.py(10),test_update_script.py(15),test_dataclass_validations.py(8), plusnpm run lintandblack/isort/mypyover all tracked Python files.Bonus test: instrumentation reaches srcdoc frames
Chasing the above raised a broader question — do OpenWPM's content scripts reach
srcdocframes at all? A gap there would be a silent measurement hole in every crawl, since ad frames and embeds use them heavily. They do: a<script>inside thesrcdocattribute is captured underabout:srcdoc, so injection lands before the frame's own code.TestSrcdocFrameInstrumentationpins it. This tests pre-existing behaviour rather than anything this PR changes, so it is easy to drop if you would rather it landed separately.Findings
Investigation notes, kept so they are not lost. Measured on Firefox 155.0.1
(unbranded add-on-devel) and Chromium 152.0.7977.64, Linux x86_64.
Two loaders, two gates
IsTrustedSchemeor(allowUnsafeURL || security.allow_unsafe_subscript_loads)forfile/jar/moz-extensionIsTrustedSchemeonlyIsTrustedSchemeisresource:,chrome:,moz-src:only(
dom/security/nsContentSecurityUtils.cpp). Actors reach it throughJSActorManager.cppcallingImportESModuleforesModuleURI. AnesModuleURIofmoz-extension:or the XPI's ownjar:URL is silentlydeclined —
JSWindowActorProtocol::ObserveusesGetActor(..., IgnoreErrors()),so the actor simply never instantiates and nothing is logged.
So the
resource://openwpm/substitution does not bypass that gate, it satisfiesit by aliasing a trusted scheme onto the XPI.
Why the bootstrap and the pool drain are needed
SubstitutingProtocolHandlerdoes propagate a substitution to contentprocesses (
SendSubstitution, andCollectSubstitutionsfor new ones), and theshipped webcompat
about-compatAPI relies on exactly that: parent-onlysetSubstitution, then a process script loaded overresource://webcompat/.But propagation is asynchronous with respect to actor registration. Registering
the actor immediately after
setSubstitutionwas measured to fail with a bareFailed to load resource://openwpm/...: a content process is asked for themodule before the mapping arrives. webcompat avoids this by doing its setup in
onStartup(), long before anything needs the mapping. Having each contentprocess register the mapping itself, before it hosts a document, is the ordering
guarantee.
Separately, content processes that already exist when the bootstrap is
registered do not run it, so the preallocated pool is replaced rather than
patched by toggling
dom.ipc.processPrelaunch.enabledonce. That pref istherefore a requirement, enforced as a
ConfigError.Upstream state
experiment_apisscripts tomoz-extension:. Its TODO, and 1974691's, sitdirectly in
CheckAllowedURI.restrict
moz-extension:subscript loads to privileged extensions. Rob--Wnotes such loads "can happen from extension experiments only", so the branch
is already privileged-only in practice and the pref is belt-and-braces.
canUseAPIExperiment()allowsexperiment_apiswhen the add-on is privilegedor
AddonSettings.EXPERIMENTS_ENABLED. OpenWPM is the second case. Afuture check keyed on
WebExtensionPolicy::IsPrivileged()would lock us out;one keyed on
canUseAPIExperiment()would not.The one case a content script cannot reach
A frame's uncommitted initial
about:blank— the placeholder betweenappendChildand the real document committing. Firefox never injects contentscripts there (bug 1415539),
and no option changes it. It is observable: a page that appends a
srcdociframeand reads
frame.contentWindow.navigator.webdriverin the same task reads thatdocument, and re-checking the saved document object later shows it is never
injected, not merely injected late.
Chromium 152 behaves identically on every case measured, so this is not a
Firefox bug or a compat issue; it is
w3c/webextensions#1009
territory, where the general form
(#513) is closed and
opposed:by all three vendors.srcdocorigin semantics, measured: it inherits the opener's origin and is fullyscriptable.
sandbox="allow-scripts"makes it opaque and unreachable;allow-scripts allow-same-originrestores same-origin and is still neverinjected — so the gap is a property of
srcdoc, not of origin opacity, whichalso rules out
match_origin_as_fallback.A narrower mechanism exists for this one flag
Navigator::Webdriver()reads Marionette/RemoteAgent state out ofsharedData,which the parent can clear — leaving the native getter untouched entirely. It is
deliberately not used here: it can only move that one flag, while the instrument
this is a slice of needs to overwrite arbitrary properties and to run at realm
creation. Matching the native accessor exactly is the part that generalises,
which is why the tests pin property order and wrong-receiver behaviour.
Tried and rejected
setSubstitutionwithout the per-process bootstrap — loses thepropagation race, measured.
matchOriginAsFallbackon a content script — per its documentation itdisables
matchAboutBlankand needs a wildcard path glob<all_urls>doesnot provide.
moz-extension:andjar:asesModuleURI— silently declined.file:root —CheckAllowedURIgatesfile:identically.process-type-setobserver and an acknowledgement handshake, both of whichmeasured unnecessary once the pool is drained.