Skip to content

Bump WebKit (oven-sh/WebKit#590 preview): ShadowRealm wraps a returned callable for the caller's realm - #42028

Open
robobun wants to merge 5 commits into
mainfrom
robobun/b1492092/shadow-realm-wrapper-caller-realm
Open

robobun wants to merge 5 commits into
mainfrom
robobun/b1492092/shadow-realm-wrapper-caller-realm

Conversation

@robobun

@robobun robobun commented Sep 8, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • ShadowRealm: when a wrapped function's target is not a plain JS function (a callable Proxy, or a built-in such as the realm's Function) and it returns a callable, the returned wrapper was created in the target realm. Object.getPrototypeOf(wrapper) is then the other realm's Function.prototype, its .constructor is the other realm's genuine Function, and F("return globalThis")() hands out that realm's global object. The leak runs both ways.
  • The cause is in JSC: remoteFunctionCallGeneric (Source/JavaScriptCore/runtime/JSRemoteFunction.cpp:158) ended with wrapReturnValue(globalObject, targetGlobalObject, result). The plain-function path and the JIT thunk wrap for the caller's realm, as OrdinaryWrappedFunctionCall specifies. Node with --experimental-shadow-realm gets this right.
const r = new ShadowRealm();
const made = r.evaluate(`Function`)("return 1");
Object.getPrototypeOf(made) === Function.prototype;            // was false
const g = Object.getPrototypeOf(made).constructor("return globalThis")();
g.injected = 1; r.evaluate(`globalThis.injected`);             // was 1

Fix

Background

  • A value that crosses a ShadowRealm boundary must be a primitive or a callable. A callable is replaced by a wrapped function (JSRemoteFunction in JSC) that lives in the receiving realm and forwards calls to the target.
  • The global object passed to JSRemoteFunction::tryCreate decides the wrapper's structure and [[Prototype]], so it decides which realm owns the wrapper.
  • JSC has two call paths for wrapped functions, chosen when the wrapper is created: a fast one for plain JSFunction targets (including bound functions) and a generic one for every other callable. Only the generic one had the swap, which is why test262's wrapped-function-proto-from-caller-realm.js (plain function target) did not catch it.
Notes
  • The gate's fail-before step restores src/ and packages/ only, and the fix lives in the WebKit pin under scripts/, so it cannot reproduce the failing side. Fail-before was checked by hand: USE_SYSTEM_BUN=1 bun test test/js/bun/jsc/shadow.test.js (bun 1.4.3) and bun bd test on pin 2e2aa2290fac both give 3 pass, 5 fail.
  • test/js/bun/jsc/domjit.test.ts million-iteration loops time out at 5 s under the local debug ASAN build. They do not touch ShadowRealm.
  • Exceptions that cross the boundary were already translated into the caller's realm on both paths (checked with a throwing Proxy target).

[policy-decision:webkit] gate passed · iteration 0 · 2 files touched

passes on PR (with fix)
Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/js/bun/jsc/shadow.test.js'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/js/bun/jsc/shadow.test.js
bun test v1.4.3 (f42e98025)

test/js/bun/jsc/shadow.test.js:
(pass) shadow realm works [72.62ms]
(pass) a callable returned through a plain function belongs to the caller's realm [47.06ms]
(pass) a callable returned through a bound function belongs to the caller's realm [18.78ms]
(pass) a callable returned through the realm's Function constructor belongs to the caller's realm [20.42ms]
(pass) a callable returned through a callable Proxy belongs to the caller's realm [20.80ms]
(pass) a callable returned through a Proxy with an apply trap belongs to the caller's realm [20.28ms]
(pass) a returned wrapper does not expose the shadow realm's global object [21.21ms]
(pass) a callable returned to the shadow realm belongs to the shadow realm [27.72ms]

 8 pass
 0 fail
 18 expect() calls
Ran 8 tests across 1 file. [3.08s]
Exit: 0
diff hotspot
scripts/build/deps/webkit.ts   |  2 +-
 test/js/bun/jsc/shadow.test.js | 49 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

gate history · 4 passed · 0 rejected · iteration 0

evidence per changed file
file                            reads  edits  tests
scripts/build/deps/webkit.ts        2      2     14
test/js/bun/jsc/shadow.test.js      3      3     13

root cause · written by the author bot

The generic ShadowRealm call path in JSRemoteFunction::remoteFunctionCallGeneric passed the target realm's global object to wrapReturnValue, so when a wrapped function's target was not a plain JS function (a callable Proxy or a built-in constructor such as the realm's Function or Object) and it returned a callable, the wrapper was allocated with the target realm's remote function structure. That gave the caller a function whose [[Prototype]] was the other realm's Function.prototype, exposing that realm's genuine Function constructor and, through it, its global object in both d…

…s realm

A callable that a wrapped function returns must be wrapped for the realm
of that wrapped function. JSC wrapped it for the target's realm when the
target is not a plain JS function, so the caller received a function
object whose prototype is the other realm's Function.prototype, and
through its constructor the other realm's global object.

Adds the cases to test/js/bun/jsc/shadow.test.js. The engine fix is in
oven-sh/WebKit#590. Five of the eight tests in the file fail until the
WebKit pin moves.
… target's return value for the caller's realm

Pins WEBKIT_VERSION at the preview build of oven-sh/WebKit#590. A
callable returned by a wrapped function whose target is not a plain
JSFunction (a callable Proxy, or a built-in such as the realm's own
Function) was wrapped for the target's realm instead of the caller's.
The caller could read the other realm's Function constructor off the
wrapper's prototype and reach that realm's global object, in both
directions.

The range from 2e2aa2290fac also contains oven-sh/WebKit#561, #566 and
#568, already merged on oven-sh/WebKit main.
@coderabbitai

coderabbitai Bot commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

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

Or wait 2 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: fd8da59b-e364-4a16-9924-92c4a6e3135d

📥 Commits

Reviewing files that changed from the base of the PR and between d745f03 and 99f67c1.

📒 Files selected for processing (2)
  • scripts/build/deps/webkit.ts
  • test/js/bun/jsc/shadow.test.js

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

@github-actions github-actions Bot added the claude label Sep 8, 2026
@robobun

robobun commented Sep 8, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 12:03 PM PT - Sep 8th, 2026

✅ @robobun, your commit 99f67c1034166109d4caa76af743e5c690b518c4 passed in Build #113090! 🎉


🧪   To try this PR locally:

bunx bun-pr 42028

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

bun-42028 --bun

@robobun

robobun commented Sep 8, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced on bun 1.4.3 and on a debug build at WebKit pin 2e2aa2290fac with the snippet in the PR body: Object.getPrototypeOf(made) === Function.prototype is false, and the shadow realm's global object is reachable and writable from the caller (and the caller's from the realm, through a Proxy target). test/js/bun/jsc/shadow.test.js: 3 pass, 5 fail on that pin.

With the engine fix from oven-sh/WebKit#590 (preview autobuild-preview-pr-590-c7520f66, pinned here): 8 pass, 0 fail, and the six test/js/node/test/parallel/test-shadow-realm*.js files pass.

This PR depends on oven-sh/WebKit#590. Once that merges, the pin here moves from the preview tag to the merge commit.

@claude claude 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.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Comment thread scripts/build/deps/webkit.ts Outdated
Comment thread test/js/bun/jsc/shadow.test.js Outdated
@robobun

robobun commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

Review pass on 92515b9: the test now restores the mutated global in a finally block. The preview pin stays until oven-sh/WebKit#590 merges, then it moves to the merge commit (details in the thread on scripts/build/deps/webkit.ts). No other open threads.

@claude claude 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.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Comment thread test/js/bun/jsc/shadow.test.js Outdated
Comment thread test/js/bun/jsc/shadow.test.js Outdated
@robobun

robobun commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

Review pass on 82638d7: the target matrix uses it.each and the test comment states the invariant instead of the history. Still 8 pass with the new pin and 3 pass, 5 fail on bun 1.4.3. All review threads are resolved.

@claude claude 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.

Code review found no issues

No high-confidence issues detected in this change.

@robobun

robobun commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

99f67c1 spells the pin as the full release tag, autobuild-preview-pr-590-c7520f66. The Source lints check (test/internal/source-lints/webkit-prebuilt-url.test.ts) accepts only a 40-hex sha or an autobuild-* tag, and the short form failed it. Same tarball, same result: shadow.test.js 8 pass. No open review threads.

@claude claude 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.

Code review found no issues

No high-confidence issues detected in this change.

@robobun

robobun commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

Head 99f67c1: Source lints is green now, along with the other GitHub checks. Buildkite #113090 is running on this head. Nothing open from review.

This branch has not been deployed

No deployments
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.

2 participants