Repository navigation
Conversation
…st entry The HMR server runtime built the per-module ssrManifest entry with client[exportName] = ... on a plain object. For an export named __proto__ that assignment hit the Object.prototype setter and replaced the entry's prototype. The export was missing from Object.keys of the entry, and the delete loop in componentManifestDelete never removed its serverManifest id. Create the entry with Object.create(null) so every export name is an own key.
|
Status: ready for review. Reproduced with the released bun (v1.4.1-canary.1) through CI: no
Both are reported for triage on main. The other listed failures passed on retry. |
|
Updated 4:18 PM PT - Aug 28th, 2026
❌ @robobun, your commit e9a659f has 2 failures in
🧪 To try this PR locally: bunx bun-pr 40828That installs a local version of the PR into your bun-40828 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 5 included reviews per hour; 1 remains after this review. WalkthroughChangesClient export manifest handling
Merge Risk: ⚪ Minimal · up to The change preserves proto exports in the development SSR manifest and removes them correctly when no longer client components. No actionable merge-blocking risk remains beyond normal checks. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the problem, implementation, compatibility considerations, regression test, and verification results. It provides the information required by the template, although it uses Problem and Fix headings instead of the template headings. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — one-line {} → Object.create(null) swap using an idiom already established in hmr-module.ts, with a test covering both the add and delete branches.
What was reviewed:
- Confirmed
client[exportName] = ...now defines an own key for"__proto__"and the delete-branchfor...inat line 193 enumerates exactly the registered export names on the null-prototype entry. - Checked the test's expected object uses computed
["__proto__"]and the actual value round-trips throughJSON.parse, so both sides carry__proto__as an own enumerable key and.toEqualcompares them correctly. - Verified the outer
serverManifest/ssrManifestmaps are keyed by<file>#<export>and project-relative paths, so leaving them as plain objects is a reasonable scope boundary.
Extended reasoning...
Overview
This PR changes a single object literal in src/runtime/bake/hmr-runtime-server.ts (registerUpdate) from {} to Object.create(null), so a client-component export named __proto__ is stored as an own key of the per-module ssrManifest entry rather than invoking the Object.prototype.__proto__ setter. It adds one devTest in test/bake/dev/bundle.test.ts that asserts both manifests contain the __proto__ export as an own key when "use client" is present, and that both manifests are fully cleared for that module when the directive is removed.
Security risks
The change is a prototype-pollution hardening, not a new risk surface. Previously, a user-controlled export name (__proto__) could mutate the prototype of the per-module manifest entry; the fix removes Object.prototype from the chain so all assignments become own data properties. The only consumer (react-server-dom-bun) reads moduleMap[id][name], which works identically on a null-prototype object. The outer serverManifest and ssrManifest maps remain plain objects, but their keys are <file>#<export> and project-relative file paths, neither of which can equal the bare string __proto__, so the PR's stated scope boundary holds.
Level of scrutiny
Low-to-moderate. The runtime edit is one line applying an idiom already present in the same subsystem (hmr-module.ts:879). The delete branch's for (const exportName in client) was the second half of the bug — with a null prototype it now enumerates exactly the registered names, so the stale serverManifest[<file>#__proto__] entry is correctly removed. The new comment is short and answers the obvious "why not {}" question, satisfying the repo's comment-discipline rule.
Other factors
The test is placed alongside the existing "use client" HMR test in the same file (per CLAUDE.md placement rules), uses devTest/minimalFramework like its neighbors, and asserts via a single .toEqual on a nested object (per test/CLAUDE.md). It deliberately uses the re-export form so the module namespace already has an own __proto__ getter, isolating this runtime fix from the separate bundler-side object-literal lowering issue referenced in the description. No CODEOWNERS entry covers these paths, there are no outstanding reviewer objections in the timeline, and the bug hunt exited on dry_streak.
Object.fromEntries defines own keys, so the entry stays a plain object with the same shape as the production manifest entry, and an export named __proto__ is an own key instead of the prototype.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This branch has not been deployed
1 participant
Problem
"use client"export named__proto__is missing from itsssrManifestentry. When the module stops being a client component,serverManifest["<file>#__proto__"]stays behind.registerUpdate(src/runtime/bake/hmr-runtime-server.ts) built the entry withclient[exportName] = {...}on a plain object. For"__proto__"that calls theObject.prototype.__proto__setter and replaces the entry prototype. The delete loop (for...in) then never sees__proto__.Fix
Object.fromEntriesover the export names. It defines own data properties, so__proto__becomes a key like any other.__proto__a computed key there). Dev and prod entries have the same keys, prototype, and attributes.react-server-dom-bun, readsmoduleMap[id][name]andmoduleMap[id]["*"]. Unchanged.test/bake/dev/bundle.test.ts("client component exported as proto ..."). The released bun fails both assertions. Also ran that whole file,esm.test.ts,react-response.test.ts,request-cookies.test.ts.Background
bun:bake/serverexposes two maps.serverManifestmaps"<file>#<export>"to a client module id.ssrManifestmaps a client file id to one entry per export,{ specifier: "ssr:<file>", name }. React's Flight client uses them during SSR.registerUpdatefills the maps after each bundle: it loads the server-side module and iteratesObject.keys(exports). Removing the directive runs the delete branch.__proto__key today, independent of Keep __proto__ object keys as written when printing and synthesizing object literals #40812.Notes
Repro output with the released bun (v1.4.1-canary.1), page returns
JSON.stringify({ exports, ssr: ssrManifest, server: Object.keys(serverManifest) }):After removing
"use client"from the file:With the fix the first response lists both
__proto__andotherunderssr, and the second response hasserver: [].The first revision of this PR used
Object.create(null)for the entry. The self-review pointed out that this makes the dev entry differ from the production one (noObject.prototype, soentry.hasOwnProperty(...)throws in dev only).Object.fromEntriesgives the same result for__proto__and keeps the shape identical. Checked with a probe:Object.getPrototypeOf(entry) === Object.prototype, and the__proto__property is enumerable, writable, and configurable.serverManifestkeys are"<file>#<export>"andssrManifestkeys are project-relative file paths, so neither can equal__proto__. Those two objects are left as they are.The export-clause form
export { Client as __proto__ }of a local function, andexport function __proto__, are lowered to a plain{ __proto__: ... }property today, which sets the prototype of the exports object itself. So those forms never reach this code with a__proto__key until the bundler fix in #40812 lands. The test uses the re-export form so it exercises this runtime fix on its own.Two adjacent observations, not changed here:
import.meta.hot.on("__proto__", cb)hits the same class of bug ineventHandlers(src/runtime/bake/hmr-module.ts:32):eventHandlers.__proto__isObject.prototype, so??= []skips and.pushthrows.DevServer.rscallsregisterUpdateand drops the returned promise. A"use client"module with three or more top-level awaits can be rendered on the first request before its manifest entry exists. The second request sees it. The test module has no top-level await, so the test is not affected.[review] gate passed · iteration 0 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file