Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/build/deps/webkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* for local mode. Override via `--webkit-version=<hash>` to test a branch.
* 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.


/**
* WebKit (JavaScriptCore) — the JS engine.
Expand Down
5 changes: 5 additions & 0 deletions src/jsc/bindings/BunGlobalScope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ void GlobalScope::finishCreation(JSC::VM& vm)
Base::finishCreation(vm);
ASSERT(inherits(info()));

// Every Bun global clears IsImmutablePrototypeExoticObject from its structure, so user
// code can put a Proxy in its prototype chain (jsdom does, for its window). V8 runs
// programs against such a chain, so JSC must not reject it.
setAllowsProxyInPrototypeChain(true);

m_encodeIntoObjectStructure.initLater(
[](const JSC::LazyProperty<JSC::JSGlobalObject, JSC::Structure>::Initializer& init) {
auto& vm = init.vm;
Expand Down
59 changes: 58 additions & 1 deletion test/js/node/vm/vm.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, isWindows, normalizeBunSnapshot } from "harness";
import { bunEnv, bunExe, isWindows, normalizeBunSnapshot, tempDir } from "harness";
import {
compileFunction,
constants,
Expand Down Expand Up @@ -1318,6 +1318,63 @@ describe("DONT_CONTEXTIFY", () => {
});
});

// https://github.com/oven-sh/bun/issues/42331
describe("a Proxy in the prototype chain of the context global", () => {
// jsdom 28+ puts a Proxy (WindowProperties) in the prototype chain of its
// window. V8 runs a program against such a global, so node:vm must too.
function installProxy(ctx: object) {
const target = { fromProxy: 7 };
const g = runInContext("this", ctx);
Object.setPrototypeOf(g, Object.create(new Proxy(target, {})));
return target;
}

test.each([
["DONT_CONTEXTIFY", () => createContext(constants.DONT_CONTEXTIFY)],
["contextified sandbox", () => createContext({})],
])("%s: programs run and resolve names through the Proxy", (_, makeContext) => {
const ctx = makeContext();
const target = installProxy(ctx);

expect(runInContext("1 + 1", ctx)).toBe(2);
expect(new Script("2 + 2").runInContext(ctx)).toBe(4);
expect(runInContext("fromProxy", ctx)).toBe(7);
expect(runInContext("typeof missingName", ctx)).toBe("undefined");
expect(runInContext("undeclaredAssign = 9; undeclaredAssign", ctx)).toBe(9);
expect(runInContext("fromProxy = 8; fromProxy", ctx)).toBe(8);
expect(target.fromProxy).toBe(7);
});

test("contextified sandbox: declarations land on the global, not on the Proxy", () => {
const ctx = createContext({});
const target = installProxy(ctx);

expect(runInContext("var declared = 5; function fn() { return 6 } declared + fn()", ctx)).toBe(11);
expect(new Script("declared * 2").runInContext(ctx)).toBe(10);
expect(target).toEqual({ fromProxy: 7 });
});

test("main realm: runInThisContext and require() run with a Proxy in the chain of globalThis", async () => {
using dir = tempDir("vm-proxy-global", { "dep.cjs": "module.exports = 42;" });
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`const vm = require("node:vm");
Object.setPrototypeOf(globalThis, Object.create(new Proxy({ viaProxy: 3 }, {})));
console.log(vm.runInThisContext("1 + 1"), vm.runInThisContext("viaProxy"), require("./dep.cjs"));`,
],
env: bunEnv,
cwd: String(dir),
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
expect(stdout).toBe("2 3 42\n");
expect(exitCode).toBe(0);
});
});

describe("defineProperty errors use vm-realm global", () => {
test("data descriptor on sandbox-only property", () => {
const sandbox = {};
Expand Down
Loading