A returned Error's non-enumerable slots ship unsanitized — the #3235 guard walks Object.keys but seroval encodes getOwnPropertyNames

[frenzzy]

Summary

5cee0f77 (#3235) widened the result-encoding guard to descend Error-prototyped carriers, so a failure channel returned under an Error gets sanitized. The descent enumerates the carrier with Object.keys, but seroval encodes an Error's own properties with Object.getOwnPropertyNames. Everything under a non-enumerable own slot — cause (non-enumerable by spec since ES2022), AggregateError.errors, any defineProperty context slot — and its whole subtree is therefore never walked, never sanitized, and ships to the client verbatim on a head-already-committed 200.

The demonstrated payload is a Postgres DSN with credentials, which is exactly the shape a driver error carries in its cause.

Reproduction

const SECRET = "conn=postgres://u:SECRET_PW@db";

registerServerFunction("err", async () => {
  const e = new Error("query failed");
  Object.defineProperty(e, "cause", { value: new Error(SECRET), enumerable: false, configurable: true });
  return { failure: e };                       // returned as a value, not thrown
});

// CONTROL: identical secret on a non-enumerable slot of a PLAIN object
registerServerFunction("obj", async () => {
  const o = { msg: "query failed" };
  Object.defineProperty(o, "cause", { value: new Error(SECRET), enumerable: false, configurable: true });
  return { failure: o };
});

NODE_ENV=production, both posted through handleServerFunctionRequest:

ERROR carrier  : secretOnWire=true
OBJECT control : secretOnWire=false

The asymmetry is the point: a plain object's non-enumerable slot is correctly left alone (it is never encoded), but the Error carrier's is encoded by seroval and shipped unsanitized. Symbol-keyed slots do not leak; the exposure is Error own string-keyed non-enumerable slots.

Where

packages/web/server-functions/src/server.ts:2568

return new Frame(OBJECT, value, next, Object.keys(value), descriptors);

The Object.keys(value) walk was correct for plain objects but under-covers an Error, whose channel-bearing slots seroval reaches through Object.getOwnPropertyNames. Introduced by 5cee0f77 (#3235) — the widening that made Error carriers guarded in the first place; it closed the enumerable case and left the non-enumerable one.

Why it matters

Reachable by any anonymous caller of any server function that returns (does not throw) an Error carrying a live failure reason under a non-enumerable slot. cause is the ordinary place a wrapped driver or domain error puts its context, and it is non-enumerable by default — so return { error: dbError } after a caught query is enough, with no unusual code. The runtime's own guard (#3235) is what promises this is sanitized; it is not, for the slot most likely to hold the secret.

Same rebuild also drops the Error's stack (V8 installs it as an own accessor bound to the original error, which Object.create(prototype, descriptors) does not reproduce) and strips receiver-bound state from an Error subclass with an own accessor over private fields — both fallout of the same Object.keys + Object.create rebuild, both lower-stakes than the leak.

Options

1. Walk Reflect.ownKeys(value) (or Object.getOwnPropertyNames + own symbols) for the carrier, matching what seroval encodes, so every slot the codec will emit is a slot the guard descended. Minimal and it makes the walk and the encoder agree — which is the invariant the guard depends on.
2. Special-case Error carriers to walk their non-enumerable own slots explicitly. Narrower, but it leaves the walk and the encoder able to drift again for the next carrier type.

(1) is the durable fix: the guard should enumerate exactly what the encoder will.

Regression test

it("sanitizes a channel under an Error's non-enumerable slot", async () => {
  registerServerFunction(id, async () => {
    const e = new Error("x");
    Object.defineProperty(e, "cause", { value: new Error("SECRET"), enumerable: false, configurable: true });
    return { failure: e };
  });
  const body = await (await handleServerFunctionRequest(post(id))).text();
  expect(body).not.toContain("SECRET");
});

Reverting the walk to Object.keys makes body carry SECRET, so the test earns its place.

[ryansolid]

Fixed in 8f11ea7 on next. The guard now walks an Error carrier's own string keys through getOwnPropertyNames — matching what seroval encodes — with one narrowing: non-enumerable accessors stay excluded. That half is a standing ruling (4799541, pinned in server-functions-error-carrier-guard.spec.tsx): a hidden getter is the codec's read, and walking it would turn its throw into a guard-time 500 where today it fails in-band on the committed 200. Your cause case is a data slot, so it's covered — a rejected promise or erroring stream parked there is now descended, guarded, and sanitized.

One scope note on the plaintext repro: a plain new Error(SECRET) under cause still rides after this fix, because a returned error is the author's data — the same contract that ships a top-level returned error's message verbatim (leaves an error that is a value alone, pinned since #3235). What was broken, and is now fixed, is that a live failure channel hidden under the slot escaped the guard entirely: unsanitized reason on the wire plus an unowned rejection. Regression pinned in server-functions-failure-sanitization.spec.tsx with a rejected promise on a non-enumerable cause; verified leaking before the fix and sanitized after.

The stack/own-accessor rebuild fallout you flagged as lower-stakes is real but orthogonal to the walk — left as is for now.