A non-enumerable getter is now serialized to the client — #3176's materialization drops enumerable: false

[frenzzy]

Summary

The accessor handling added for #3176 materializes every getter with a hardcoded enumerable: true, discarding the original descriptor. A property an author deliberately hid from serialization with enumerable: false is now serialized and sent to the client.

This is a regression: before #3176 the value never left the server.

Tested against next @ 1cc2feb8, built from source, Node 24.19.

Reproduction

registerServerFunction("hidden", async () => {
  const row = { name: "widget", createdAt: new Date(0) };   // the Date forces the codec road
  Object.defineProperty(row, "internalCostBasis", {
    get: () => "COST-SECRET-42",
    enumerable: false,
    configurable: true
  });
  return row;
});

status = 200   COST-SECRET-42 on the wire: *** YES ***

The baseline this replaces — what the value has always serialized to, and what the author was relying on:

JSON.stringify(row)   // {"name":"widget"}

The wire body carries the hidden key alongside the visible ones:

{"k":["name","createdAt","internalCostBasis"], … "COST-SECRET-42"}

What triggers it

Only the codec road materializes accessors, so the leak needs one other value in the result that forces it — a Date, a Map, a Set, a promise, a stream. That is not an unusual result; a row with a timestamp is the ordinary case.

A non-enumerable data property is unaffected (control: not leaked). It is specifically accessors.

Why enumerable: false is load-bearing here

Hiding a field from serialization is exactly what enumerable: false is for, and it is the mechanism JSON.stringify honours. An ORM's computed cost basis, an internal score, a derived field that is expensive or sensitive — the author marks it non-enumerable precisely so it stays server-side, and every serializer in the ecosystem respects that. This one now does not.

The value is also computed on every response, so a getter with a side effect or a cost now runs where it previously did not.

Cause

enterGuard copies the original descriptors onto the rebuilt shell, but the materialization writes the resolved value back with a fixed descriptor:

Object.defineProperty(next, key, { value, writable: true, enumerable: true, configurable: true })

enumerable: true is unconditional. It should carry descriptor.enumerable through.

Related, same line, separately filed

The same materialization also breaks on a frozen container (#3196) — there the hardcoded configurable: true collides with the frozen descriptor and the call answers 500. Both come from that one descriptor literal; a fix that threads the original descriptor's enumerable and configurable through would settle both.

Options

1. Preserve the original descriptor's flags — { value, writable: true, enumerable: descriptor.enumerable, configurable: true }. Smallest change, restores the pre-Channels behind a getter or as a Map key are unsanitized, never torn down, and can kill the process #3176 wire shape exactly, and keeps the rebuild working. configurable: true still needs to stay for the shell to be writable, which is what A frozen result with a getter or a channel answers 500 after the mutation committed #3196 is about.
2. Skip non-enumerable accessors entirely — do not materialize what the codec would not have serialized. Closer to JSON.stringify semantics, and it avoids invoking a getter whose value is never sent. Needs care that a non-enumerable slot holding a channel is still torn down even though its value is not shipped.
3. Materialize but omit from the encoded output — most faithful, most work.

I'd suggest (1) as the immediate fix and (2) as the better end state, since it also stops paying for a getter whose value nobody receives.

Happy to send a PR. The regression test shape is the reproduction above: a non-enumerable accessor beside a Date, asserting the wire body matches what JSON.stringify would have produced, with a non-enumerable data property as the control that already passes.

Provenance

Introduced by 2320bc91 — fix: guard getter-backed and Map-key channels in server function results (#3176), in the accessor materialization, which writes back a fixed descriptor literal instead of the original's flags.

Same literal is the cause of #3196. One change settles both: carry descriptor.enumerable through, and relax only what the rebuild actually needs.

A single change closes this and its sibling — implemented and verified

#3196 and #3198 are one defect wearing two hats: the accessor materialization writes back a fixed descriptor, and the shell it writes into is a replica of the source's descriptors. Frozen source → the write is illegal (#3196). Fixed literal → enumerable: false is lost (#3198).

One descriptor shape settles both, and it removes a branch rather than adding one:

-          top.accessorRead === i
-            ? { enumerable: true, configurable: true, writable: true, value: guarded }
-            : { ...top.descriptors[items[i]], value: guarded }
+          {
+            value: guarded,
+            writable: true,
+            configurable: true,
+            enumerable: top.descriptors[items[i]].enumerable
+          }

plus making the shell rewritable, since a frozen replica cannot be written to at all:

-  const next = Object.create(prototype, descriptors);
+  // The shell is scratch the codec reads once and the author never sees, so
+  // its slots stay rewritable — replicating a frozen source's descriptors
+  // would make the write-back below illegal (#3196).
+  const writable = {};
+  for (const key of Object.keys(descriptors)) {
+    writable[key] = { ...descriptors[key], writable: true, configurable: true };
+    if ("get" in writable[key]) delete writable[key].writable;
+  }
+  const next = Object.create(prototype, writable);

15 insertions, 4 deletions — three of the insertions are the comment, so roughly eight lines of code, and one conditional disappears.

The reasoning it encodes is that only enumerable ever reaches the wire. writable and configurable describe a shell the codec reads once and nobody else ever holds, so replicating them was never buying anything — it was only creating the collision.

Verified

hidden non-enumerable getter  -> 200, secret on wire: no    (was: leaked)
                                 matches the JSON.stringify baseline {"name":"widget"}
frozen + plain getter         -> 200                        (was: 500)
frozen + promise              -> 200                        (was: 500)
Object.seal + promise         -> 200   writable:false only  -> 200
configurable:false only       -> 200   frozen array/Map     -> 200
frozen plain data             -> 200

Full tracked suite with the change: 53 files, 586 passed, 2 skipped — no existing test moved.

A tighter alternative if the descriptor copy is unwanted at all: drop it and have the walk write every slot unconditionally. Less code again, at the cost of the changed short-circuit that lets an untouched graph return its original — probably not worth trading.

The docblock line "Descriptors carry across, so a frozen or non-writable shape survives the rebuild" claims exactly the property that is broken, and should be corrected either way.

[ryansolid]

Fixed by #3203.