fix: keep decoded __proto__ keys as own properties

[KyleJune]

Summary

Both loader-data decoders wrote object keys with result[key] = value. In browsers, assigning to __proto__ sets the object's prototype instead of creating an own property. So a loader that returns untrusted JSON such as {"__proto__": {"isAdmin": true}} reached the client with the key missing from Object.keys/JSON.stringify, and prefs.isAdmin === true through inheritance. The server and Object.prototype are unaffected. This bug already existed in 0.11.5.

Changes

• src/_serialization.ts: new private defineOwnValue helper that writes a key with Object.defineProperty (enumerable, writable, configurable). restoreValue uses it; that covers page loads, data requests, and deferred values that resolved. restoreValueWithPendingPromises uses it too; that covers the initial chunk of a streamed response.
• The encoders (processValue, processValueForStreaming) are unchanged. They copy keys from server-side source objects, and Deno assigns __proto__ as an ordinary property.
• cbor2 2.3.0 builds string-keyed maps with Object.fromEntries, so the raw decode already kept the own key. The key was lost only in juniper's own assignments.

Testing

• New test in src/_serialization.test.ts: "decoding an own proto key where assignment sets the prototype". It runs the decode in a child deno eval, so the test process never mutates globals.
  • The child encodes the payload the way a server does, then installs the standard browser Object.prototype.__proto__ accessor. Before decoding, it checks that assignment now really sets the prototype.
  • It then decodes through the page-load, data-request, streamed, and deferred paths. For each one it asserts that the own key and its value survive, the prototype is Object.prototype, and isAdmin is not inherited.
• Before the fix, the test failed on all four paths: ownKeys: ["name"], hasOwnProto: false, protoIsObjectPrototype: false, isAdmin: true. After the fix it passes.
• deno task test: 34 passed (387 steps), 0 failed. deno task check passes, including doc-lint.

Related

Found by the security review of #131, which does not introduce or change this bug. A trial merge with #131 is clean.

Closes #132

🤖 Generated with Claude Code

[KyleJune]

Security review at 92abe278: merge

No findings. The fix closes juniper#132 on every decode path that builds an object from payload keys. Both new call sites are load-bearing: reverting either one turns its own path red, and no other path. The result keeps the default prototype, and normal data comes out the same as before.

Severity of what this closes (for the record)

• Path: an app's loader returns an object whose keys come from unvalidated user input (for example JSON.parse(user.preferences) holding {"__proto__":{"isAdmin":true}}). On 0.11.5, restoreValue (src/_serialization.ts:354 on main) assigns result["__proto__"] = … in the victim's browser. The key disappears and isAdmin becomes inherited.
• Precondition: the app has to ship raw user-keyed JSON to another user's browser, and then trust inherited properties in client code.
• Impact: limited to that one object on the client. Object.prototype is not polluted and nothing changes on the server. Low for the framework.

Decode paths examined (at 92abe278)

• restoreValue, src/_serialization.ts:367: now defineOwnValue. It covers page load (v2), data requests, and deferred values once they resolve.
• restoreValueWithPendingPromises, :643-647: now defineOwnValue. It covers the first chunk of a streamed response.
• Raw decode: cbor2 2.3.0 builds string-keyed maps with Object.fromEntries (lib/container.js, function F). It never hits the setter.
• The remaining obj[key] = sites run on the server or use keys the server controls:
  • processValue :297 and processValueForStreaming :480 are encoders, and they run on the server. On Deno 2.9.6, o["__proto__"] = x creates an own property (checked with deno eval).
  • serializeAllContext :811 uses context registry names as keys.
  • _server.tsx:267 builds publicEnv from the allowlist of env keys.
• Read-only: deserializeError / built-in error deserializers (:227-243, :960+) read fields and never copy data onto an object. deserializeAllContext looks values up by registry name.
• Client code: src/_client.tsx and src/client.tsx never use Object.assign, spread, or keyed assignment on decoded data.
• constructor / prototype: not affected. On an ordinary object, assigning either creates an own property. __proto__ is the only accessor on Object.prototype.

Resulting objects

• A decoded {"__proto__": x} is an own data property (enumerable, writable, configurable) on an ordinary object whose prototype is Object.prototype, not a null-prototype object. The new test asserts protoIsObjectPrototype: true and isAdmin: false.
• Normal data: I built the same entries (integer keys, constructor) with defineProperty and with plain assignment. Key order and descriptors match exactly (deno eval), so there is no behavior change.

Mutation drill (child deno eval with the browser __proto__ accessor installed)

• M0, unmutated: ok | 2 passed (56 steps) | 0 failed.
• M1, :367 back to result[key] = restoreValue(val): red on page load, data request, and deferred data. Each shows hasOwnProto: false, isAdmin: true, protoIsObjectPrototype: false, ownKeys: ["name"]. streamed data stays green.
• M2, :643 back to result[key] = restoreValueWithPendingPromises(…): red on streamed data only, with the same four fields wrong. The other three paths stay green.

Interaction with #131 (v3 tagged-JSON payload)

• Trial merge: git merge --no-commit pr133 into feat: send hydration data as tagged JSON text #131 (d8217331) auto-merges cleanly.

• Merged tree:

  • deno task check passes.
  • deno task test --reporter=dot: 40 passed (444 steps) | 0 failed.
  • CI's deno task test --coverage: 40 passed (444 steps) | 0 failed, with no coverage panic.
  • deno coverage --detailed exits 0 (_serialization.ts at 86.0%, all files at 90.1%).

• The v3 decoder needs both halves. feat: send hydration data as tagged JSON text #131's objectFromEntries already uses defineProperty for escaped __proto__/$t objects. Its output then goes through restoreValue, which rebuilds every object, so v3 page loads depend on fix: keep decoded __proto__ keys as own properties #133 as well. Two mutations on the merged tree show this:

  • M3, revert restoreValue (merged): fix: keep decoded __proto__ keys as own properties #133's test goes red on page load (now v3), data request, and deferred data, with isAdmin: true. All of feat: send hydration data as tagged JSON text #131's tests stay green.
  • M4, objectFromEntries to plain assignment (merged): fix: keep decoded __proto__ keys as own properties #133's test goes red on page load: hasOwnProto: false, the key is dropped. isAdmin stays false because restoreValue rebuilds the object. All of feat: send hydration data as tagged JSON text #131's tests stay green.

  feat: send hydration data as tagged JSON text #131's suite runs in Deno, where assignment creates an own property, so only fix: keep decoded __proto__ keys as own properties #133's test covers either half under browser semantics.

Merge order

#133 first. It is the security fix, it is independent, and as a fix: it releases as a 0.11.x patch. That gets the fix to users without waiting on the v3 format change. Then #131:

• It needs no textual rebase; the merge is clean.
• It does need a fresh CI event on its branch after fix: keep decoded __proto__ keys as own properties #133 lands (update the branch from main). Its existing checks ran without fix: keep decoded __proto__ keys as own properties #133's code, and the combined tree is only proven here locally.

Notes (not findings)

• v2 coverage after the merge: fix: keep decoded __proto__ keys as own properties #133's page-load leg will exercise v3, because serializeHydrationData emits v3. The v2 decode shares restoreValue, which the data-request leg still pins (M3).
• Registered custom-type and error deserializers: they receive the raw decoded object. What they do with __proto__ keys belongs to the application, as before.
• Threat model: the udibo threat model gets its row when udibo adopts this release. No §5 row covers client-side decode integrity yet. T9/T10 cover the hydration payload only as a script-breakout channel.