Channels behind a getter or as a Map key are unsanitized, never torn down, and can kill the process #3176

Summary

guardFailures deliberately does not walk accessor properties or Map keys. The codec pumps those channels anyway, so a stream or promise reached through one gets neither of the two things the guard provides:

• its rejection is not sanitized — a driver error's message rides the wire verbatim, at status 200;
• it is never torn down — after the client disconnects the producer keeps running, and its finally never executes;
• and on the promise path it can kill the process — the handler answers 200 and Node then exits on an unhandled rejection.

Tested against next @ a536e29b, built from source, Node 24.19.

Reproduction 1 — the sanitizer is bypassed

Three server functions holding the same rejecting promise, differing only in how it is held:

const mk = () => { const p = Promise.reject(new Error("user=svc_billing password=hunter2 host=10.0.0.7")); p.catch(() => {}); return p; };

registerServerFunction("plain",  async () => ({ p: mk() }));                 // data property
registerServerFunction("getter", async () => ({ get p() { return mk(); } })); // accessor
registerServerFunction("mapkey", async () => new Map([[mk(), "v"]]));        // Map key

plain    ran=1  status=200  secret on wire: no        sanitized: true
getter   ran=1  status=200  secret on wire: *** YES *** sanitized: false
mapkey   ran=1  status=200  secret on wire: *** YES *** sanitized: false

This is verbatim the disclosure sanitizeServerError exists to prevent — the module's own comment describes "a driver error's message and own-properties … riding the wire verbatim" as the thing it stops.

Reproduction 2 — the producer is never torn down

Same three shapes, holding an endless async generator. Read one chunk, cancel, wait 300 ticks:

plain    steps at cancel=1 | 300 ticks later=  1 | finally ran: true
getter   steps at cancel=1 | 300 ticks later=301 | finally ran: false
mapkey   steps at cancel=1 | 300 ticks later=301 | finally ran: false

Left running, the process dies:

FATAL: JavaScript heap out of memory  (Mark-Compact 4076.9 MB, ~82 s)

Under load this is not subtle. Driving 400 requests each abandoned after one chunk, in a separate process with forced GC, the leaked shapes produced +812 000 generator steps and +598 MB in three idle seconds, still climbing, with 400 finally blocks that never ran — i.e. 400 DB cursors or file handles never released. The plain shape over the same run: 0 live producers, heap flat.

Reproduction 3 — one request kills the process

Worse than either of the above, and found after filing. A getter returning a promise that rejects:

registerServerFunction("crash", async () => ({
  get p() { return Promise.reject(new Error("upstream down")); }
}));

handler answered status: 200
PROCESS EXIT CODE: 1

Error: upstream down
    at get p (…)
    at encodeResult (…/dist/server.js:1464)
    at dispatch (…/dist/server.js:1771)
    at async Module.handleServerFunctionRequest (…)

The client is told the call succeeded, and then Node terminates the process on an unhandled rejection. isJSONSafe invokes the getter while judging the value, discards the promise it gets back, and attaches no handler; under Node's default --unhandled-rejections=throw that is fatal. A single request to a function returning this shape takes the server down.

Note also that the getter is invoked twice on this path — once by isJSONSafe and once by the codec — so any getter with a side effect fires twice, and a getter whose value changes between the two reads is a TOCTOU on the encode decision.

Both skips are deliberate, and that is the hard part

This is not an oversight, and any fix has to respect the reasons already recorded:

• Accessors are skipped because reading a getter during the walk would invoke it here as well as when the codec encodes, and a throwing one would escape into dispatch's catch and be reported as the function failing — "the phantom error over a call that succeeded".
• Map keys are skipped because "rebuilding a key would change map identity semantics for the caller."

Both concerns are real. So "just walk them" is not the answer — walking accessors reintroduces double invocation, and rebuilding a Map key breaks identity.

The observation is narrower: the guard's job here is not to rebuild the value, it is to register the channel for sanitization and teardown. Those two things do not require replacing the node in the graph.

Options

1. Register without rebuilding. Where the walk finds an async iterable or stream it cannot safely replace, still hand it to the teardown gate and wrap its failure channel, leaving the graph node untouched. Fixes both symptoms for both shapes without touching identity or invocation counts. For a Map key nothing needs to change in the map at all; for an accessor the getter is still not invoked by the walk — only the value the codec ultimately pulls is registered, at the point the codec pulls it.
2. Move the guard to the codec seam. Whatever the codec is about to pump gets registered, wherever it came from. Structurally the most complete, since it cannot miss a shape the walk does not model; it means the guard stops being a walk and becomes a hook, which is a bigger change.
3. Refuse to encode an unguardable channel. If a stream or promise is reachable only through an accessor or as a Map key, answer a legible encode error instead of silently pumping it. Smallest and safest, and consistent with how other unencodable shapes are handled — but it turns a currently-working (if leaky) case into a failure, so it needs a judgement call about who is relying on it.
4. Document the limitation. Insufficient alone: the failure mode is credential disclosure and an unbounded leak, neither of which an author can see.

I'd suggest (1) — it is the smallest change that respects both recorded reasons — with (3) as the fallback for any shape (1) still cannot reach, so nothing is pumped ungoverned.

Note on scope

Reachability is ordinary application code, not an attacker: returning a Map keyed by a promise, or a DTO with a lazy getter, is unusual but entirely legal, and the failure only becomes visible under client disconnects. There is no existing bound that contains either symptom.

Happy to send a PR. The regression test shape is the two tables above — a shape matrix (data property, accessor, Map key, Map value, Set member, array element, class field, two levels deep, late Promise) asserting for each that a rejection is sanitized and that finally runs after cancel, with the data-property row as the control that passes today.