bun:test: don't leak JSC scope objects through a mock's this value

[robobun]

What is the problem

jest.fn() mocks record the call's this value in mock.contexts and return it from mockReturnThis(). The implementation read callframe->thisValue() raw.

For a bare call fn() where fn is resolved through a scope (a binding captured by a closure, an imported binding, or a top level let/const), JSC places the resolved scope object in the this register and leaves sanitization to the callee: JS functions do it in their prologue (op_to_this), and host functions are expected to call JSValue::toThis() (see ProxyObject::performCall). Because the mock call path skipped that, engine internal scope objects (JSLexicalEnvironment, JSModuleEnvironment, the global lexical environment) escaped into mock.contexts and out of mockReturnThis():

import { mock } from "bun:test";

const fn = mock().mockReturnThis();
const keep = () => fn; // capturing `fn` makes `fn()` resolve through a scope object
console.log(fn()); // [native code: JSLexicalEnvironment]
console.log(fn.mock.contexts[0]); // same object

These objects are not meant to be reachable from JavaScript. Ordinary property operations on one break JSC invariants: Object.defineProperty(leaked, "x", { get() {} }) followed by a read aborts a release build (SIGABRT) and fails ASSERTION FAILED: !(attributes & PropertyAttribute::Accessor) in JSLexicalEnvironment::getOwnPropertySlot in a debug build. Fuzzilli found a flaky segfault with a script that does exactly this: call a captured mock, then mutate the value it returned.

What is the fix

Sanitize the incoming this once with toThis(globalObject, ECMAMode::strict()), the conversion JSC applies for strict JS callees and in ProxyObject::performCall. Scope objects become undefined; every other receiver is unchanged, so mock.contexts, mockReturnThis(), and the receiver passed to mock implementations behave exactly as before for normal calls. This also matches Jest, which records undefined for a bare call.

Not included: a few chaining helpers (jest.setSystemTime, jest.useRealTimers, the fake timer functions, the plugin builder methods) also return the raw this. They never store it, the leak there needs the method to be detached and called bare, and covering them uniformly needs the same conversion exposed to the Rust side, so they are left for a separate change.

How did you verify your code works

• New test in test/js/bun/test/mock-fn.test.js calls a mock through a captured binding and asserts the returned value and mock.contexts are undefined. On current Bun it fails with Received: [native code: JSLexicalEnvironment]; it passes with this change. The file also runs under Jest and Vitest and the asserted behavior matches both.
• bun bd test test/js/bun/test/mock-fn.test.js (73 pass), plus the mock module and issue-1825-jest-mock-functions tests.
• The Fuzzilli reproducer and the defineProperty abort case above run clean on the fixed build.

[coderabbitai]

Warning

Review limit reached

@robobun, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 3 minutes and 26 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6cf03f56-2cfa-4924-9d4f-ad2f04f37a2b

📥 Commits

Reviewing files that changed from the base of the PR and between 9f18300 and fed8a18.

📒 Files selected for processing (2)

• src/jsc/bindings/JSMockFunction.cpp
• test/js/bun/test/mock-fn.test.js

---

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

[robobun]

^{Updated 10:44 PM PT - Jun 27th, 2026}

❌ @robobun, your commit fed8a18 has 1 failures in Build #66209 (All Failures):

• scripts/build/ci.ts - build failed on 🍎 x64 - build-rust
• scripts/build/ci.ts - build failed on 🐧 x64-baseline - build-rust
• scripts/build/ci.ts - build failed on 🍎 aarch64 - build-rust
• scripts/build/ci.ts - build failed on :freebsd: x64 - build-rust
• scripts/build/ci.ts - build failed on 🐧 x64 - build-rust
• scripts/build/ci.ts - build failed on 🐧 x64-musl - build-cpp
• scripts/build/ci.ts - build failed on 🐧 aarch64 - build-rust
• scripts/build/ci.ts - build failed on 🐧 x64-musl - build-rust
• scripts/build/ci.ts - build failed on :freebsd: aarch64 - build-rust
• scripts/build/ci.ts - build failed on 🐧 x64-asan - build-rust

---

🧪   To try this PR locally:

bunx bun-pr 32996

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

bun-32996 --bun

[github-actions]

This PR may be a duplicate of:

1. Don't leak scope objects through host functions' raw this #32172 - Also fixes JSC scope object leaks through host functions' raw this value using toThis(), including the same JSMockFunction.cpp change, plus additional scope leak sites (jest.setSystemTime, fake timers, Bun.plugin, StringDecoder)

🤖 Generated with Claude Code

[robobun]

Closing as a duplicate of #32172, which contains the same toThis() sanitization in jsMockFunctionCall plus the other host functions that return a raw this value (jest timer helpers, Bun.plugin builder methods, StringDecoder), each with tests.

[claude]

LGTM — one-line toThis(..., ECMAMode::strict()) sanitization following the same pattern used elsewhere in the bindings, with a portable regression test.

Extended reasoning...

Overview

This PR changes a single line in src/jsc/bindings/JSMockFunction.cpp (jsMockFunctionCall): instead of reading callframe->thisValue() raw, it now calls .toThis(globalObject, JSC::ECMAMode::strict()) on it. A new test in test/js/bun/test/mock-fn.test.js exercises a bare call through a captured binding and asserts mock.contexts and mockReturnThis() see undefined rather than a leaked JSC scope object.

Security risks

This is a safety/hardening fix rather than a risk introduction. Before the change, JSC-internal JSLexicalEnvironment / JSModuleEnvironment objects could escape to JS via mock.contexts or mockReturnThis(), and mutating them could SIGABRT a release build. The fix closes that leak. toThis in strict mode is a pure conversion: scope objects → undefined, all other receivers (objects, primitives, null/undefined) pass through unchanged, so no new attack surface or behavioral change for ordinary callers.

Level of scrutiny

While JSC bindings are sensitive C++, this specific change is mechanical and idiomatic. The exact same callFrame->thisValue().toThis(globalObject, JSC::ECMAMode::strict()) pattern (with no exception check after) already appears in JSBuffer.cpp, JSDOMOperation.h, JSEventTargetCustom.h, and JSEventEmitterCustom.h, and the PR description correctly cites ProxyObject::performCall as JSC's own precedent. The throw scope is declared earlier in the function, and strict-mode toThis does not allocate or invoke user code, so no new exception handling is needed.

Other factors

• The bug-hunting system found no issues.
• No CODEOWNERS rules cover the modified files.
• The new test is written to also pass under Jest and Vitest (the file is explicitly designed to be cross-runner), so it asserts standard semantics rather than Bun-specific behavior.
• Existing this-related tests in the same file (.call, .apply, .bind, obj.fn()) cover the cases where the receiver must be preserved, and strict-mode toThis is identity for those.
• The PR description is thorough, explains the JSC op_to_this contract, and documents verification (73 tests pass, Fuzzilli reproducer clean).