Skip to content

node:vm: allow a Proxy in the prototype chain of a global object - #42347

Closed
robobun wants to merge 1 commit into
mainfrom
robobun/a1f7783b/nodevm-proxy-in-global-prototype-chain
Closed

robobun wants to merge 1 commit into
mainfrom
robobun/a1f7783b/nodevm-proxy-in-global-prototype-chain

Conversation

@robobun

@robobun robobun commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Fix

  • [JSC] Let an embedder allow a Proxy in the prototype chain of its global object WebKit#626 adds JSGlobalObject::setAllowsProxyInPrototypeChain(). A global object that opts in skips the check. Bun::GlobalScope::finishCreation (src/jsc/bindings/BunGlobalScope.cpp) opts in, so every Bun global is covered: main, workers, shadow realms and node:vm contexts.
  • Correct because every Bun global already clears IsImmutablePrototypeExoticObject from its structure (ZigGlobalObject.cpp:443, NodeVM.cpp:941), so user code can set its prototype. Global declaration instantiation in JSC reads only own properties of the global, so no Proxy trap runs while a program links. Name lookup at run time goes through the ordinary [[HasProperty]], [[Get]] and [[Set]] paths, which handle a Proxy. That matches V8.
  • WEBKIT_VERSION points at the preview build of [JSC] Let an embedder allow a Proxy in the prototype chain of its global object WebKit#626. Repin it to the merged sha before this merges.
  • Verified: test/js/node/vm/vm.test.ts, block "a Proxy in the prototype chain of the context global" (4 tests, all fail on Bun 1.4.2). Also the rest of test/js/node/vm, the 95 vendored test/js/node/test/parallel/test-vm-*.js, and jsdom 30 under vm.runInContext.

Background

  • A ProgramExecutable is what vm.runInContext, vm.runInThisContext, new vm.Script().runIn* and the CommonJS loader evaluate. Modules and eval do not go through it, so they never hit the check.
  • An immutable prototype exotic object is an object whose [[SetPrototypeOf]] refuses a new value. The spec makes Object.prototype one, and JSC makes global objects one by default. Bun clears that flag on its globals so jsdom can set __proto__ on the window.
  • WebKit added the check in 2016 (bug 165205) for the web, where Window.prototype is mutable and var declaration walked the prototype chain at the time. JSC's declaration code is own-property only today.
Notes

Node behaviour the tests follow: in a DONT_CONTEXTIFY context a missing name throws ReferenceError. In a contextified context it reads as undefined (the sandbox interceptor). Node's has/get/set traps fire on a Proxy in the chain for unresolved names and sloppy assignments, and the Proxy target is never written (the receiver is the global). Bun now produces the same results.

Pre-existing, unrelated: var and function declarations in a DONT_CONTEXTIFY context do not create bindings in Bun, with or without a Proxy in the chain. The declaration test therefore uses a contextified context.

test/js/node/vm/script-leak.test.ts times out at 5 s in the container used here, on main as well as on this branch (16 s of parse time for 10000 scripts in a debug ASAN build).

Self-reviewed: the review found that the first version opted in only the node:vm global and missed the main realm, and that the PR body claimed the main global could not get a Proxy in its chain. Both fixed: the opt-in moved to Bun::GlobalScope, and a main realm test was added. The review also asked for the preview pin to be replaced by the merged sha, which is the repin step above.


no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/vm/vm.test.ts

JavaScriptCore rejects a program when any link of the global object's
prototype chain is a Proxy. Every Bun global clears
IsImmutablePrototypeExoticObject from its structure, so user code can set
that prototype, and jsdom 28+ puts a Proxy in the chain of its window.
Opt every Bun global out of the check, as V8 permits it. This needs
oven-sh/WebKit#626.
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

On-demand reviews are free for the next 9 days. After that, they cost $0.25 per reviewed file.

Or wait 8 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 37e369fe-9722-4de5-8242-85ffcf40e596

📥 Commits

Reviewing files that changed from the base of the PR and between 6a92015 and 776a00d.

📒 Files selected for processing (3)
  • scripts/build/deps/webkit.ts
  • src/jsc/bindings/BunGlobalScope.cpp
  • test/js/node/vm/vm.test.ts

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:07 AM PT - Sep 11th, 2026

✅ @robobun, your commit 776a00d5645519a5b843f075a881f5a2f9dec7b0 passed in Build #114362! 🎉


🧪   To try this PR locally:

bunx bun-pr 42347

That installs a local version of the PR into your bun-42347 executable, so you can run:

bun-42347 --bun

* From https://github.com/oven-sh/WebKit releases.
*/
export const WEBKIT_VERSION = "cf1b36ec8703d8e87436094d21d478d358c7d886";
export const WEBKIT_VERSION = "autobuild-preview-pr-626-f1e23f97";

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.

🔴 WEBKIT_VERSION is pinned to autobuild-preview-pr-626-f1e23f97, an ephemeral preview-PR build tag; merging this makes every fresh checkout and CI lane depend on an artifact that disappears once oven-sh/WebKit#626 closes, breaking the build for everyone (base pins a merged SHA). Fix: before merge, repin to the merged upstream SHA and verify prebuilt WebKit artifacts exist for every platform × flavor, per scripts/build/deps/README.md and the Dependencies & vendoring rule. The PR body notes this as a repin step, but the pin as committed is still the merge-blocker.

Extended reasoning...

scripts/build/deps/webkit.ts:6 sets WEBKIT_VERSION = "autobuild-preview-pr-626-f1e23f97". WebKit is a prebuilt dep — resolveDep downloads a release artifact keyed on this string from oven-sh/WebKit. Preview-PR autobuild tags are deleted when the PR is merged or closed, so after that point dep_fetch 404s and bun bd / CI fails at the WebKit fetch step on any machine without a cached copy. The base branch pins a real merged SHA (cf1b36ec…) whose artifacts are permanent. The repo's dependency-pin convention explicitly forbids merging a pin to an ephemeral artifact; the author's own PR body says "Repin it to the merged sha before this merges", confirming the tag is temporary.

Verification: normal — acknowledged in diff: the PR body states "WEBKIT_VERSION points at the preview build of oven-sh/WebKit#626. Repin it to the merged sha before this merges", which is accurate but does not resolve the hazard; the code as it stands is not mergeable. scripts/build/deps/webkit.ts:6 changes the pin from a permanent merged SHA to an ephemeral preview-PR tag: ``` -export const…

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. The preview tag is only there so CI can build against oven-sh/WebKit#626 before it merges. I will repin WEBKIT_VERSION to the merged sha as soon as that PR lands, and this PR stays a draft for merging until then. Leaving this thread open as the reminder.

@robobun
robobun marked this pull request as draft September 11, 2026 17:49
steipete added a commit to openclaw/bun that referenced this pull request Oct 4, 2026
Opt Bun globals into the pinned OpenClaw WebKit capability for Proxy prototype chains, restoring jsdom Window program execution. Cover contextified globals, declarations, inherited lookup and the main realm.

Ports oven-sh#42347. Requires OpenClaw WebKit. Linux regression and OpenClaw theme proof pass; both fork native CI lanes pass on the guarded PR head.

Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
@github-actions github-actions Bot closed this Oct 31, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Superseded by #44833.

@robobun

robobun commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator Author

Confirmed. #44833 pins a WebKit that skips the Proxy check for every Bun global (ProgramExecutable.cpp, oven-sh/WebKit#781), so this change is no longer needed. The issue repro prints 2 on 1.4.4-canary.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

node:vm rejects a Proxy in a DONT_CONTEXTIFY global prototype chain, breaking jsdom with Vitest vmThreads

1 participant