From b86dbba68baa317ffe39aea483e15c3bdb356fbd Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Sun, 30 Aug 2026 15:17:21 +0700 Subject: [PATCH 1/5] fix(web): sanitize failures that escape through the result graph MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sanitizeServerError guards the one road a thrown error takes out of dispatch. A failure can also escape through the RESULT GRAPH — a rejected promise, an async iterable that throws, a stream that errors — where it reaches the codec as a value to encode rather than as a throw, and never meets the sanitizer. Same failure, different road, and the leak is the exact one the sanitizer exists to stop: an ORM error's message and own-properties (failing query, connection string, bound params) riding the wire verbatim. Worse than the thrown case, because the head is already committed, so the answer is a 200 carrying no error tag. Claimed as a codec plugin rather than by walking the result: a walk would have to run before serialization, and a rejection has not happened yet at that point. The replacement is branded with markSafeError so the plugin does not claim it again, which also leaves the wire shape a plain Error node — the peer needs no matching plugin and the protocol is unchanged. The sanitizer composes ahead of an app's own plugins: a custom error type reaching the client is intent, and intent is spelled markSafeError. #3095's authoring error is branded for the same reason — it names the status the author got wrong, so it must stay readable. --- .changeset/sanitize-graph-failures.md | 5 + packages/web/server-functions/src/server.ts | 62 +++++++++-- ...er-functions-failure-sanitization.spec.tsx | 102 ++++++++++++++++++ 3 files changed, 163 insertions(+), 6 deletions(-) create mode 100644 .changeset/sanitize-graph-failures.md create mode 100644 packages/web/test/server/server-functions-failure-sanitization.spec.tsx diff --git a/.changeset/sanitize-graph-failures.md b/.changeset/sanitize-graph-failures.md new file mode 100644 index 000000000..ac5094486 --- /dev/null +++ b/.changeset/sanitize-graph-failures.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +Sanitize a failure that escapes through a server function's result graph. `sanitizeServerError` guarded the one road a thrown error takes out of dispatch; a rejected promise, an async iterable that throws, or a stream that errors reaches the codec as a value to encode instead, and shipped its `message` and every own-property to the client verbatim — an ORM error's failing query, connection string and bound params included — under a 200 carrying no error tag, because the head was already committed. The same sanitization now applies on that road, claimed as a codec plugin so it covers every channel without walking the result. `markSafeError` remains the escape hatch, and the replacement is branded with it so the wire shape stays a plain `Error` node. diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index b7f00dde4..dd8596a9a 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -16,7 +16,8 @@ import { NULL_BODY_STATUSES, REVALIDATE_HEADER, isResponseEnvelope, - isSafeError + isSafeError, + markSafeError } from "../../src/response.js"; import { RequestContext, commitEventResponse, getRequestEvent } from "../../src/server.js"; import { encodeFlashCookie } from "./flash.js"; @@ -1366,6 +1367,46 @@ function isFormPost(request) { * server function"); iterables nested inside user objects are consumed by * the codec directly and stay untouched. */ +// A failure that escapes through the RESULT GRAPH — a rejected promise, an +// async iterable that throws, a stream that errors — reaches the codec as +// a value to encode rather than as a throw, so it never passes the +// sanitizer guarding every other way an error leaves dispatch. It is the +// same failure by a different road, and the leak is the exact one +// `sanitizeServerError` exists to stop: an ORM error's message and +// own-properties (failing query, connection string, bound params) ride the +// wire verbatim. Worse than the thrown case, because the head is already +// committed — the answer is a 200 carrying no error tag. +// +// Claimed as a codec plugin rather than by walking the result: a walk +// would have to run before serialization, and a rejection has not happened +// yet at that point. The replacement is branded safe so the plugin does +// not claim it again, which also leaves the wire shape a plain Error node +// — the peer needs no matching plugin, and nothing about the protocol +// changes. +// +// The sanitizer is composed AHEAD of an app's own plugins: a custom error +// type reaching the client is intent, and intent is spelled +// `markSafeError`, not "shadowed the default". +let failureSanitizer; + +function sanitizingPlugins(createPlugin, plugins) { + if (DEV) return plugins; + if (!failureSanitizer) { + failureSanitizer = createPlugin({ + tag: "solid/server-function-failure", + test: value => value instanceof Error && !isSafeError(value), + parse: { + sync: (value, ctx) => ctx.parse(markSafeError(sanitizeServerError(value))), + async: async (value, ctx) => ctx.parse(markSafeError(sanitizeServerError(value))), + stream: (value, ctx) => ctx.parse(markSafeError(sanitizeServerError(value))) + }, + serialize: (node, ctx) => ctx.serialize(node), + deserialize: (node, ctx) => ctx.deserialize(node) + }); + } + return plugins ? [failureSanitizer, ...plugins] : [failureSanitizer]; +} + export function serializeResponseStream(value, codecOptions, signal) { let closeIterator = null; let closed = false; @@ -1437,7 +1478,9 @@ export function serializeResponseStream(value, codecOptions, signal) { }; signal.addEventListener("abort", onAbort); } - const { serializeJSON } = await import("../../serialization/src/serializer.js"); + const { createPlugin, serializeJSON } = await import( + "../../serialization/src/serializer.js" + ); if (closed) { // torn down while the codec was loading; nothing was started try { @@ -1447,6 +1490,7 @@ export function serializeResponseStream(value, codecOptions, signal) { } cancelSerialize = serializeJSON(value, { ...codecOptions, + plugins: sanitizingPlugins(createPlugin, codecOptions && codecOptions.plugins), onParse(node) { if (!closed) controller.enqueue(createChunk(JSON.stringify(node))); }, @@ -1491,10 +1535,16 @@ function encodeResult(value, headers, status, codec, signal) { headers.set(BODY_FORMAT_HEADER, BodyFormat.Void); return new Response(null, { status, headers }); } - const error = new Error( - `Server function answered status ${status}, which forbids a response body, with a value. ` + - `Return respond(undefined, { status: ${status} }) for a bodiless answer, or drop the ` + - `status to send the value.` + // Branded safe: this is an authoring error the developer must be able to + // read, so it is intentional client-facing content by definition — the + // failure sanitizer below would otherwise flatten it to the generic + // message and take the diagnosis with it. + const error = markSafeError( + new Error( + `Server function answered status ${status}, which forbids a response body, with a value. ` + + `Return respond(undefined, { status: ${status} }) for a bodiless answer, or drop the ` + + `status to send the value.` + ) ); headers.set(ERROR_HEADER, encodeErrorHeaderValue(error.message)); return encodeResult(error, headers, 500, codec, signal); diff --git a/packages/web/test/server/server-functions-failure-sanitization.spec.tsx b/packages/web/test/server/server-functions-failure-sanitization.spec.tsx new file mode 100644 index 000000000..f7d6b9431 --- /dev/null +++ b/packages/web/test/server/server-functions-failure-sanitization.spec.tsx @@ -0,0 +1,102 @@ +/** + * `sanitizeServerError` guards the one road a thrown error takes out of + * dispatch. A failure can also escape through the RESULT GRAPH — a + * rejected promise, an async iterable that throws, a stream that errors — + * where it reaches the codec as a value to encode rather than as a throw. + * Same failure, different road, and the leak is the exact one the + * sanitizer exists to stop: an ORM error's message and own-properties + * (failing query, connection string, bound params) riding the wire + * verbatim, under a 200 carrying no error tag because the head is already + * committed. + * + * `markSafeError` stays the escape hatch on both roads. + * + * Like the other server-function specs, these run against the built + * bundles (server-functions/dist/*, wired up in vite.config.server.mjs), + * which are the production variant — `DEV` is false, so the sanitizer is + * live. + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { markSafeError } from "@solidjs/web"; +import { + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +/** A driver error as one actually arrives: secrets in message and own-props. */ +function databaseError() { + return Object.assign(new Error("connect ECONNREFUSED postgres://app:hunter2@10.0.0.5:5432"), { + connectionString: "postgres://app:hunter2@10.0.0.5:5432", + query: "SELECT * FROM users WHERE token = 'abc123'" + }); +} + +function scriptedPost(id: string) { + return new Request(`https://app.example/_server/data/${id}`, { + method: "POST", + body: "[]", + headers: { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Instance": "server-function:test" + } + }); +} + +async function bodyOf(id: string) { + const response = await handleServerFunctionRequest(scriptedPost(id)); + try { + return await response.text(); + } catch { + return ""; + } +} + +describe("a failure escaping through the result graph is sanitized", () => { + const channels: Record unknown> = { + "an async iterable that throws": async function* () { + yield { page: 1 }; + throw databaseError(); + }, + "a rejected promise in the graph": async () => ({ deferred: Promise.reject(databaseError()) }), + "a stream that errors in the graph": async () => ({ + rows: new ReadableStream({ + start(controller) { + controller.enqueue("first"); + queueMicrotask(() => controller.error(databaseError())); + } + }) + }) + }; + + it.each(Object.keys(channels))("%s", async channel => { + const id = `graph-failure-${Object.keys(channels).indexOf(channel)}`; + registerServerFunction(id, channels[channel] as () => unknown); + + const body = await bodyOf(id); + + expect(body).not.toContain("hunter2"); + expect(body).not.toContain("10.0.0.5"); + expect(body).not.toContain("SELECT * FROM users"); + expect(body).not.toContain("abc123"); + }); + + it("keeps an error the author branded as intentional", async () => { + registerServerFunction("graph-failure-safe", async function* () { + yield { page: 1 }; + throw markSafeError(new Error("Order 42 not found")); + }); + + expect(await bodyOf("graph-failure-safe")).toContain("Order 42 not found"); + }); +}); From 2c22ca617e311b9f4704eaf615b0c2117b6b8c6a Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Sun, 30 Aug 2026 15:42:28 +0700 Subject: [PATCH 2/5] fix(web): sanitize failures that escape through the result graph MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sanitizeServerError guards the one road a thrown error takes out of dispatch. A failure can also escape through the RESULT GRAPH — a rejected promise, an async iterable that throws, a stream that errors — where it reaches the codec as a value to encode rather than as a throw, and never meets the sanitizer. The leak is the one the sanitizer exists to stop: a driver error's message and own-properties (failing query, connection string, bound params) riding the wire verbatim, under a 200 carrying no error tag because the head is already committed. The three channels are wrapped before the codec sees them. Not the rejection, which has not happened yet, and not the Errors already in the graph: an Error reached as a value was never thrown, so it is data and the author's to ship. Containers are rebuilt only along paths that actually contain a channel, so a healthy response allocates nothing and reference identity survives for the codec; a WeakMap keeps a repeated reference one object and terminates cycles. A first attempt claimed Error via a codec plugin. That was wrong twice over: a plugin WRAPS rather than replaces, so the wire carried a plugin node the peer had no plugin for, and sanitizeServerError's own unbranded replacement was claimed too — breaking every sanitized error, including the ordinary thrown one. Both are now regression tests. #3095's authoring error is branded markSafeError: it names the status the author got wrong, so it is intentional client-facing content. --- .changeset/sanitize-graph-failures.md | 2 +- packages/web/server-functions/src/server.ts | 147 ++++++++++++----- ...er-functions-failure-sanitization.spec.tsx | 156 +++++++++++++----- 3 files changed, 224 insertions(+), 81 deletions(-) diff --git a/.changeset/sanitize-graph-failures.md b/.changeset/sanitize-graph-failures.md index ac5094486..1f3a80e5c 100644 --- a/.changeset/sanitize-graph-failures.md +++ b/.changeset/sanitize-graph-failures.md @@ -2,4 +2,4 @@ "@solidjs/web": patch --- -Sanitize a failure that escapes through a server function's result graph. `sanitizeServerError` guarded the one road a thrown error takes out of dispatch; a rejected promise, an async iterable that throws, or a stream that errors reaches the codec as a value to encode instead, and shipped its `message` and every own-property to the client verbatim — an ORM error's failing query, connection string and bound params included — under a 200 carrying no error tag, because the head was already committed. The same sanitization now applies on that road, claimed as a codec plugin so it covers every channel without walking the result. `markSafeError` remains the escape hatch, and the replacement is branded with it so the wire shape stays a plain `Error` node. +Sanitize a failure that escapes through a server function's result graph. `sanitizeServerError` guarded the one road a thrown error takes out of dispatch; a rejected promise, an async iterable that throws, or a stream that errors reaches the codec as a value to encode instead, and shipped its `message` and every own-property to the client verbatim — a driver error's failing query, connection string and bound params included — under a 200 carrying no error tag, because the head was already committed. Those three channels are now wrapped before the codec sees them, so a failure arriving through one is sanitized like any other. `markSafeError` remains the escape hatch, an `Error` that is a returned value is untouched, and the wire format is unchanged. diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index dd8596a9a..3ad6441a7 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -1367,47 +1367,117 @@ function isFormPost(request) { * server function"); iterables nested inside user objects are consumed by * the codec directly and stay untouched. */ -// A failure that escapes through the RESULT GRAPH — a rejected promise, an -// async iterable that throws, a stream that errors — reaches the codec as -// a value to encode rather than as a throw, so it never passes the -// sanitizer guarding every other way an error leaves dispatch. It is the -// same failure by a different road, and the leak is the exact one -// `sanitizeServerError` exists to stop: an ORM error's message and -// own-properties (failing query, connection string, bound params) ride the -// wire verbatim. Worse than the thrown case, because the head is already -// committed — the answer is a 200 carrying no error tag. +// `sanitizeServerError` guards the one road a thrown error takes out of +// dispatch. A failure can also escape through the RESULT GRAPH — a rejected +// promise, an async iterable that throws, a stream that errors — where it +// reaches the codec as a value to encode rather than as a throw, and never +// meets the sanitizer. Same failure, different road, and the leak is the +// one the sanitizer exists to stop: a driver error's message and +// own-properties (failing query, connection string, bound params) riding +// the wire verbatim. Worse than the thrown case, because the head is +// already committed — the answer is a 200 carrying no error tag. // -// Claimed as a codec plugin rather than by walking the result: a walk -// would have to run before serialization, and a rejection has not happened -// yet at that point. The replacement is branded safe so the plugin does -// not claim it again, which also leaves the wire shape a plain Error node -// — the peer needs no matching plugin, and nothing about the protocol -// changes. +// So the CHANNELS are wrapped before the codec sees them. Not the rejection +// (it has not happened yet) and not the Errors already in the graph (an +// Error returned as data is a value, and values are the author's) — only +// the three shapes through which a future failure can arrive. // -// The sanitizer is composed AHEAD of an app's own plugins: a custom error -// type reaching the client is intent, and intent is spelled -// `markSafeError`, not "shadowed the default". -let failureSanitizer; - -function sanitizingPlugins(createPlugin, plugins) { - if (DEV) return plugins; - if (!failureSanitizer) { - failureSanitizer = createPlugin({ - tag: "solid/server-function-failure", - test: value => value instanceof Error && !isSafeError(value), - parse: { - sync: (value, ctx) => ctx.parse(markSafeError(sanitizeServerError(value))), - async: async (value, ctx) => ctx.parse(markSafeError(sanitizeServerError(value))), - stream: (value, ctx) => ctx.parse(markSafeError(sanitizeServerError(value))) +// Containers are rebuilt only along paths that actually contain a channel: +// everything else is passed through by reference, so the common response +// allocates nothing and reference identity survives for the codec. The +// WeakMap keeps a repeated reference one object, and terminates cycles. +function guardFailures(value, seen) { + if (value === null || typeof value !== "object") return value; + if (!seen) seen = new WeakMap(); + const cached = seen.get(value); + if (cached !== undefined) return cached; + + if (typeof value.then === "function") { + const guardedPromise = Promise.resolve(value).then( + resolved => guardFailures(resolved, seen), + error => { + throw sanitizeServerError(error); + } + ); + seen.set(value, guardedPromise); + return guardedPromise; + } + + if (typeof value[Symbol.asyncIterator] === "function") { + const source = value; + const guardedIterable = { + [Symbol.asyncIterator]() { + const iterator = source[Symbol.asyncIterator](); + return { + next: () => + iterator.next().then( + step => (step.done ? step : { done: false, value: guardFailures(step.value, seen) }), + error => { + throw sanitizeServerError(error); + } + ), + return: iterator.return && (() => iterator.return()) + }; + } + }; + seen.set(value, guardedIterable); + return guardedIterable; + } + + if (typeof ReadableStream !== "undefined" && value instanceof ReadableStream) { + const reader = value.getReader(); + const guardedStream = new ReadableStream({ + async pull(controller) { + try { + const { done, value: chunk } = await reader.read(); + done ? controller.close() : controller.enqueue(chunk); + } catch (error) { + controller.error(sanitizeServerError(error)); + } }, - serialize: (node, ctx) => ctx.serialize(node), - deserialize: (node, ctx) => ctx.deserialize(node) + cancel(reason) { + return reader.cancel(reason); + } + }); + seen.set(value, guardedStream); + return guardedStream; + } + + if (Array.isArray(value)) { + let changed = false; + const next = value.map(entry => { + const guarded = guardFailures(entry, seen); + if (guarded !== entry) changed = true; + return guarded; }); + const result = changed ? next : value; + seen.set(value, result); + return result; + } + + // Plain objects only: a class instance's own properties are not ours to + // rebuild (private fields, getters, invariants), and neither is a Date, + // a Response, or anything else the codec has its own reading of. + const prototype = Object.getPrototypeOf(value); + if (prototype !== Object.prototype && prototype !== null) { + seen.set(value, value); + return value; } - return plugins ? [failureSanitizer, ...plugins] : [failureSanitizer]; + + let changed = false; + const next = {}; + for (const key of Object.keys(value)) { + const guarded = guardFailures(value[key], seen); + if (guarded !== value[key]) changed = true; + next[key] = guarded; + } + const result = changed ? next : value; + seen.set(value, result); + return result; } export function serializeResponseStream(value, codecOptions, signal) { + value = guardFailures(value); let closeIterator = null; let closed = false; let cancelSerialize = null; @@ -1478,9 +1548,7 @@ export function serializeResponseStream(value, codecOptions, signal) { }; signal.addEventListener("abort", onAbort); } - const { createPlugin, serializeJSON } = await import( - "../../serialization/src/serializer.js" - ); + const { serializeJSON } = await import("../../serialization/src/serializer.js"); if (closed) { // torn down while the codec was loading; nothing was started try { @@ -1490,7 +1558,6 @@ export function serializeResponseStream(value, codecOptions, signal) { } cancelSerialize = serializeJSON(value, { ...codecOptions, - plugins: sanitizingPlugins(createPlugin, codecOptions && codecOptions.plugins), onParse(node) { if (!closed) controller.enqueue(createChunk(JSON.stringify(node))); }, @@ -1535,10 +1602,8 @@ function encodeResult(value, headers, status, codec, signal) { headers.set(BODY_FORMAT_HEADER, BodyFormat.Void); return new Response(null, { status, headers }); } - // Branded safe: this is an authoring error the developer must be able to - // read, so it is intentional client-facing content by definition — the - // failure sanitizer below would otherwise flatten it to the generic - // message and take the diagnosis with it. + // Branded safe: an authoring error the developer must be able to read is + // intentional client-facing content by definition. const error = markSafeError( new Error( `Server function answered status ${status}, which forbids a response body, with a value. ` + diff --git a/packages/web/test/server/server-functions-failure-sanitization.spec.tsx b/packages/web/test/server/server-functions-failure-sanitization.spec.tsx index f7d6b9431..93970bf25 100644 --- a/packages/web/test/server/server-functions-failure-sanitization.spec.tsx +++ b/packages/web/test/server/server-functions-failure-sanitization.spec.tsx @@ -3,13 +3,15 @@ * dispatch. A failure can also escape through the RESULT GRAPH — a * rejected promise, an async iterable that throws, a stream that errors — * where it reaches the codec as a value to encode rather than as a throw. - * Same failure, different road, and the leak is the exact one the - * sanitizer exists to stop: an ORM error's message and own-properties - * (failing query, connection string, bound params) riding the wire - * verbatim, under a 200 carrying no error tag because the head is already - * committed. + * Same failure, different road, and the leak is the one the sanitizer + * exists to stop: a driver error's message and own-properties (failing + * query, connection string, bound params) riding the wire verbatim, under + * a 200 carrying no error tag because the head is already committed. * - * `markSafeError` stays the escape hatch on both roads. + * Every case here round-trips through the client as well as reading the + * wire. Asserting only that the body lacks the secret passes just as + * happily when the body is empty or undecodable, which is how a broken + * first attempt at this fix went green. * * Like the other server-function specs, these run against the built * bundles (server-functions/dist/*, wired up in vite.config.server.mjs), @@ -23,6 +25,7 @@ import { handleServerFunctionRequest, registerServerFunction } from "@solidjs/web/server-functions/server"; +import { createServerReference } from "@solidjs/web/server-functions/client"; const RequestContext = Symbol.for("solid.RequestContext"); @@ -42,53 +45,85 @@ function databaseError() { }); } -function scriptedPost(id: string) { - return new Request(`https://app.example/_server/data/${id}`, { - method: "POST", - body: "[]", - headers: { - "Sec-Fetch-Site": "same-origin", - "X-Server-Function-Instance": "server-function:test" - } - }); +const SECRETS = ["hunter2", "10.0.0.5", "SELECT * FROM users", "abc123"]; + +/** Routes the client stub's fetch straight into the handler. */ +function connectTransport() { + const original = globalThis.fetch; + globalThis.fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + const address = input instanceof Request ? input.url : input.toString(); + const request = new Request( + new URL(address, "http://localhost"), + input instanceof Request ? input : init + ); + request.headers.set("Sec-Fetch-Site", "same-origin"); + return handleServerFunctionRequest(request); + }) as typeof fetch; + return () => { + globalThis.fetch = original; + }; +} + +async function wireBody(id: string) { + const response = await handleServerFunctionRequest( + new Request(`https://app.example/_server/data/${id}`, { + method: "POST", + body: "[]", + headers: { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Instance": "server-function:test" + } + }) + ); + return response.text(); } -async function bodyOf(id: string) { - const response = await handleServerFunctionRequest(scriptedPost(id)); +async function callThrough(id: string) { + const restore = connectTransport(); try { - return await response.text(); - } catch { - return ""; + return { resolved: true as const, value: await createServerReference(id)() }; + } catch (error) { + return { resolved: false as const, error }; + } finally { + restore(); } } -describe("a failure escaping through the result graph is sanitized", () => { - const channels: Record unknown> = { - "an async iterable that throws": async function* () { +describe("a failure escaping through the result graph", () => { + it("is sanitized when an async iterable throws", async () => { + registerServerFunction("graph-failure-iterable", async function* () { yield { page: 1 }; throw databaseError(); - }, - "a rejected promise in the graph": async () => ({ deferred: Promise.reject(databaseError()) }), - "a stream that errors in the graph": async () => ({ + }); + + const body = await wireBody("graph-failure-iterable"); + for (const secret of SECRETS) expect(body).not.toContain(secret); + expect(body).toContain("Internal Server Error"); + }); + + it("is sanitized when a promise in the graph rejects", async () => { + registerServerFunction("graph-failure-promise", async () => ({ + deferred: Promise.reject(databaseError()) + })); + + const body = await wireBody("graph-failure-promise"); + for (const secret of SECRETS) expect(body).not.toContain(secret); + expect(body).toContain("Internal Server Error"); + }); + + it("is sanitized when a stream in the graph errors", async () => { + registerServerFunction("graph-failure-stream", async () => ({ rows: new ReadableStream({ start(controller) { controller.enqueue("first"); queueMicrotask(() => controller.error(databaseError())); } }) - }) - }; + })); - it.each(Object.keys(channels))("%s", async channel => { - const id = `graph-failure-${Object.keys(channels).indexOf(channel)}`; - registerServerFunction(id, channels[channel] as () => unknown); - - const body = await bodyOf(id); - - expect(body).not.toContain("hunter2"); - expect(body).not.toContain("10.0.0.5"); - expect(body).not.toContain("SELECT * FROM users"); - expect(body).not.toContain("abc123"); + const body = await wireBody("graph-failure-stream"); + for (const secret of SECRETS) expect(body).not.toContain(secret); + expect(body).toContain("Internal Server Error"); }); it("keeps an error the author branded as intentional", async () => { @@ -97,6 +132,49 @@ describe("a failure escaping through the result graph is sanitized", () => { throw markSafeError(new Error("Order 42 not found")); }); - expect(await bodyOf("graph-failure-safe")).toContain("Order 42 not found"); + expect(await wireBody("graph-failure-safe")).toContain("Order 42 not found"); + }); +}); + +describe("what the guard must not disturb", () => { + // The road that already worked. A first attempt at this fix replaced the + // encoded Error with a node the client could not read, breaking every + // sanitized error rather than only the unsanitized ones — and no test + // noticed, because nothing decoded a sanitized error through the client. + it("still delivers an ordinary thrown error as a readable Error", async () => { + registerServerFunction("graph-failure-thrown", async () => { + throw databaseError(); + }); + + const outcome = await callThrough("graph-failure-thrown"); + expect(outcome.resolved).toBe(false); + expect((outcome as { error: any }).error).toBeInstanceOf(Error); + expect((outcome as { error: Error }).error.message).toBe("Internal Server Error"); + }); + + // An Error reached as a VALUE was never thrown, so it is data and the + // author's to ship. The guard wraps failure channels, not errors. + it("leaves an error that is a value alone", async () => { + registerServerFunction("graph-value-error", async () => ({ + ok: 1, + failure: new Error("returned, not thrown") + })); + + const outcome = await callThrough("graph-value-error"); + expect(outcome.resolved).toBe(true); + expect((outcome as { value: any }).value.failure.message).toBe("returned, not thrown"); + }); + + it("passes a healthy result through untouched, nested promise included", async () => { + registerServerFunction("graph-healthy", async () => ({ + items: [1, 2], + nested: { deferred: Promise.resolve("value") } + })); + + const outcome = await callThrough("graph-healthy"); + expect(outcome.resolved).toBe(true); + const value = (outcome as { value: any }).value; + expect(value.items).toEqual([1, 2]); + await expect(value.nested.deferred).resolves.toBe("value"); }); }); From c40a43b9cde7cb461f51767c970e0449524c6ccf Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Sun, 30 Aug 2026 16:14:18 +0700 Subject: [PATCH 3/5] fix(web): walk data properties only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading through a getter invoked it during the walk as well as when the codec encodes it, and a throwing one escaped into dispatch's catch to be reported as the function itself failing — the phantom error over a call that succeeded that encodeResult goes out of its way to avoid. Measured: a result with a throwing getter answered 500 with the guard, 200 without. Descriptors are carried across when a container is rebuilt, so a frozen or non-writable shape survives. A channel behind an accessor is left unguarded: invoking it is not ours to do. --- packages/web/server-functions/src/server.ts | 19 +++++++++---- ...er-functions-failure-sanitization.spec.tsx | 27 +++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index 3ad6441a7..1fb28078b 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -1464,14 +1464,23 @@ function guardFailures(value, seen) { return value; } + // Data properties only. Reading through a getter would invoke it here and + // again when the codec encodes — twice for a side-effecting one, and a + // throwing one would escape into dispatch's catch and be reported as the + // function itself failing, the phantom error encodeResult goes out of its + // way to avoid. A channel behind an accessor is left unguarded, which is + // the same bargain as the class instance above: not ours to invoke. + const descriptors = Object.getOwnPropertyDescriptors(value); let changed = false; - const next = {}; for (const key of Object.keys(value)) { - const guarded = guardFailures(value[key], seen); - if (guarded !== value[key]) changed = true; - next[key] = guarded; + const descriptor = descriptors[key]; + if (!descriptor || !("value" in descriptor)) continue; + const guarded = guardFailures(descriptor.value, seen); + if (guarded === descriptor.value) continue; + descriptors[key] = { ...descriptor, value: guarded }; + changed = true; } - const result = changed ? next : value; + const result = changed ? Object.create(prototype, descriptors) : value; seen.set(value, result); return result; } diff --git a/packages/web/test/server/server-functions-failure-sanitization.spec.tsx b/packages/web/test/server/server-functions-failure-sanitization.spec.tsx index 93970bf25..490a05e3d 100644 --- a/packages/web/test/server/server-functions-failure-sanitization.spec.tsx +++ b/packages/web/test/server/server-functions-failure-sanitization.spec.tsx @@ -165,6 +165,33 @@ describe("what the guard must not disturb", () => { expect((outcome as { value: any }).value.failure.message).toBe("returned, not thrown"); }); + // The walk reads data properties only. Reading through a getter would + // invoke it here as well as when the codec encodes, and a throwing one + // would escape into dispatch's catch to be reported as the function + // itself failing — the phantom error over a call that succeeded that + // `encodeResult` goes out of its way to avoid. + it("does not invoke an accessor while walking", async () => { + registerServerFunction("graph-accessor", async () => ({ + ok: 1, + get lazy() { + throw new Error("an accessor the runtime must not call"); + } + })); + + const response = await handleServerFunctionRequest( + new Request("https://app.example/_server/data/graph-accessor", { + method: "POST", + body: "[]", + headers: { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Instance": "server-function:test" + } + }) + ); + + expect(response.status).toBe(200); + }); + it("passes a healthy result through untouched, nested promise included", async () => { registerServerFunction("graph-healthy", async () => ({ items: [1, 2], From cf5e763e8d1e1379b3b7c2607d7edf074506d733 Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Sun, 30 Aug 2026 16:17:52 +0700 Subject: [PATCH 4/5] fix(web): guard the frames flight sink too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It encodes its outcome with its own serializer (serializeStream, not serializeResponseStream), so the guard never reached it: a rejection nested inside a flight-data slice arrived with its message and own-properties intact, under a 200 with no error tag, on the same build where the plain response path was already sanitized. Reachable whenever the response routes through frames — a mutation whose result is markup — with the failure nested one level inside a slice, which is the ordinary shape of a cache entry. A rejection at the TOP of a slice already threw into dispatch's catch and was sanitized; that is why this looked covered. The spec needs the frames server entry, hence the alias alongside the other subpath aliases in vite.config.server.mjs. --- .changeset/sanitize-graph-failures.md | 2 +- packages/web/frames/src/frame-sink.ts | 13 ++++++-- packages/web/server-functions/src/server.ts | 9 +++++- ...er-functions-failure-sanitization.spec.tsx | 31 +++++++++++++++++++ packages/web/vite.config.server.mjs | 1 + 5 files changed, 52 insertions(+), 4 deletions(-) diff --git a/.changeset/sanitize-graph-failures.md b/.changeset/sanitize-graph-failures.md index 1f3a80e5c..e374556e9 100644 --- a/.changeset/sanitize-graph-failures.md +++ b/.changeset/sanitize-graph-failures.md @@ -2,4 +2,4 @@ "@solidjs/web": patch --- -Sanitize a failure that escapes through a server function's result graph. `sanitizeServerError` guarded the one road a thrown error takes out of dispatch; a rejected promise, an async iterable that throws, or a stream that errors reaches the codec as a value to encode instead, and shipped its `message` and every own-property to the client verbatim — a driver error's failing query, connection string and bound params included — under a 200 carrying no error tag, because the head was already committed. Those three channels are now wrapped before the codec sees them, so a failure arriving through one is sanitized like any other. `markSafeError` remains the escape hatch, an `Error` that is a returned value is untouched, and the wire format is unchanged. +Sanitize a failure that escapes through a server function's result graph. `sanitizeServerError` guarded the one road a thrown error takes out of dispatch; a rejected promise, an async iterable that throws, or a stream that errors reaches the codec as a value to encode instead, and shipped its `message` and every own-property to the client verbatim — a driver error's failing query, connection string and bound params included — under a 200 carrying no error tag, because the head was already committed. Those three channels are now wrapped before the codec sees them, on the plain response path and in the frames flight sink, which encodes its outcome with a serializer of its own, so a failure arriving through one is sanitized like any other. `markSafeError` remains the escape hatch, an `Error` that is a returned value is untouched, and the wire format is unchanged. diff --git a/packages/web/frames/src/frame-sink.ts b/packages/web/frames/src/frame-sink.ts index 604ed2087..8166f4056 100644 --- a/packages/web/frames/src/frame-sink.ts +++ b/packages/web/frames/src/frame-sink.ts @@ -145,7 +145,10 @@ import { frameAddress, serializeStream } from "../../server-functions/src/shared.js"; -import { getEventServerFunctionInvocation } from "../../server-functions/src/server.js"; +import { + getEventServerFunctionInvocation, + guardFailures +} from "../../server-functions/src/server.js"; import { isResponseEnvelope } from "../../src/response.js"; import { FRAME_STREAM_HEADER, @@ -2023,7 +2026,13 @@ export function frameFlightResponse({ primary, regions = [], outcome, codec }, i // Component-valued entries serialize as flight references — the // protocol injects its own plugin (see `flightCodec`), so nothing // registers it. - const reader = new ChunkReader(serializeStream(outcome, flightCodec(codec))); + // Guarded like every other server-function body: this sink has its + // own serializer, so a rejection nested in flight data would + // otherwise reach the wire with its message and own-properties + // intact — under a 200, since the head is long committed. + const reader = new ChunkReader( + serializeStream(guardFailures(outcome), flightCodec(codec)) + ); for (let node = await reader.next(); !node.done; node = await reader.next()) { write({ type: "outcome", payload: node.value }); } diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index 1fb28078b..091b2a79d 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -1386,7 +1386,14 @@ function isFormPost(request) { // everything else is passed through by reference, so the common response // allocates nothing and reference identity survives for the codec. The // WeakMap keeps a repeated reference one object, and terminates cycles. -function guardFailures(value, seen) { +/** + * Wraps the failure channels in a value so a rejection reaching the codec + * is sanitized like any other error. Applied to every server-function + * response body; the frames flight sink applies it to its own outcome, + * which is encoded by a different serializer. + * @internal + */ +export function guardFailures(value, seen) { if (value === null || typeof value !== "object") return value; if (!seen) seen = new WeakMap(); const cached = seen.get(value); diff --git a/packages/web/test/server/server-functions-failure-sanitization.spec.tsx b/packages/web/test/server/server-functions-failure-sanitization.spec.tsx index 490a05e3d..6787da7c1 100644 --- a/packages/web/test/server/server-functions-failure-sanitization.spec.tsx +++ b/packages/web/test/server/server-functions-failure-sanitization.spec.tsx @@ -26,6 +26,7 @@ import { registerServerFunction } from "@solidjs/web/server-functions/server"; import { createServerReference } from "@solidjs/web/server-functions/client"; +import { frameTransformFlightResult } from "@solidjs/web/frames/server"; const RequestContext = Symbol.for("solid.RequestContext"); @@ -205,3 +206,33 @@ describe("what the guard must not disturb", () => { await expect(value.nested.deferred).resolves.toBe("value"); }); }); + +describe("the frames flight sink", () => { + // It encodes its outcome with its own serializer, so the guard has to be + // applied there too — the same rejection that is sanitized on the plain + // response path reached the wire intact through this one. + it("sanitizes a failure nested in flight data", async () => { + // a markup-valued result is what routes the response through frames + registerServerFunction("flight-markup", async () => () => "ok"); + + const response = await handleServerFunctionRequest( + new Request("https://app.example/_server/data/flight-markup", { + method: "POST", + body: "[]", + headers: { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Instance": "server-function:test", + "X-Single-Flight": "true" + } + }), + { + collectFlightData: () => ({ "/notes": { pending: Promise.reject(databaseError()) } }), + transformFlightResult: frameTransformFlightResult + } + ); + + const body = await response.text(); + for (const secret of SECRETS) expect(body).not.toContain(secret); + expect(body).toContain("Internal Server Error"); + }); +}); diff --git a/packages/web/vite.config.server.mjs b/packages/web/vite.config.server.mjs index c96078b27..2e640d8ce 100644 --- a/packages/web/vite.config.server.mjs +++ b/packages/web/vite.config.server.mjs @@ -23,6 +23,7 @@ export default defineConfig({ alias: { "@solidjs/web/server-functions/server": resolve(rootDir, "server-functions/dist/server.js"), "@solidjs/web/server-functions/client": resolve(rootDir, "server-functions/dist/client.js"), + "@solidjs/web/frames/server": resolve(rootDir, "frames/dist/server.js"), // the transport's lazy codec imports — without these the bare // "@solidjs/web" alias below swallows the subpath "@solidjs/web/serialization/decode": resolve(rootDir, "serialization/dist/decode.js"), From 8f226365e9cf7552f4435af02111d5747ffab106 Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Sun, 30 Aug 2026 16:24:56 +0700 Subject: [PATCH 5/5] fix(web): make the walk cycle-safe, and cover Map and Set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found three defects in the first shape of the guard, all now tests: - A cycle recursed until the stack gave out, because the container was recorded in the WeakMap AFTER its children were walked. The RangeError then escaped into dispatch's catch as a 500 — on a shape seroval encodes natively as a back-reference. Containers are now recorded before descending, and a cycle forces the rebuild to stand since a descendant already holds it. - A rejection inside a Map or Set reached the wire raw: neither was walked, and the changeset promised more than the code delivered. - A null-prototype object on a channel path was rebuilt as a plain object, changing its node type from NullConstructor to Object. The prototype carries across now. Also: the ReadableStream branch was dead — a stream is async-iterable on every server runtime, so the iterator branch claimed it first — and it acquired the reader eagerly at walk time. It now runs ahead of the iterator branch and takes the reader on first pull. The markSafeError on #3095's authoring error is dropped: that error is encoded as a value and never routed through sanitizeServerError, so the brand did nothing. Verified by removing it and re-running the spec that asserts the message reaches the client. --- .changeset/sanitize-graph-failures.md | 17 +- packages/web/server-functions/src/server.ts | 220 ++++++++++-------- ...er-functions-failure-sanitization.spec.tsx | 42 ++++ 3 files changed, 184 insertions(+), 95 deletions(-) diff --git a/.changeset/sanitize-graph-failures.md b/.changeset/sanitize-graph-failures.md index e374556e9..9150db28a 100644 --- a/.changeset/sanitize-graph-failures.md +++ b/.changeset/sanitize-graph-failures.md @@ -2,4 +2,19 @@ "@solidjs/web": patch --- -Sanitize a failure that escapes through a server function's result graph. `sanitizeServerError` guarded the one road a thrown error takes out of dispatch; a rejected promise, an async iterable that throws, or a stream that errors reaches the codec as a value to encode instead, and shipped its `message` and every own-property to the client verbatim — a driver error's failing query, connection string and bound params included — under a 200 carrying no error tag, because the head was already committed. Those three channels are now wrapped before the codec sees them, on the plain response path and in the frames flight sink, which encodes its outcome with a serializer of its own, so a failure arriving through one is sanitized like any other. `markSafeError` remains the escape hatch, an `Error` that is a returned value is untouched, and the wire format is unchanged. +Sanitize a failure that escapes through a server function's result graph. +`sanitizeServerError` guarded the one road a thrown error takes out of +dispatch; a rejected promise, an async iterable that throws, or a stream +that errors reaches the codec as a value to encode instead, and shipped +its `message` and every own-property to the client verbatim — a driver +error's failing query, connection string and bound params included — +under a 200 carrying no error tag, because the head was already +committed. Those channels are now wrapped before either serializer sees +them: the response encoder and the frames flight sink, which encodes its +outcome with a serializer of its own. + +The walk covers plain objects, arrays, `Map` and `Set`. A channel held by +a class instance or behind an accessor is left alone — rebuilding one and +invoking the other are not the runtime's to do. `markSafeError` remains +the escape hatch, an `Error` that is a returned value is untouched, and +the wire format is unchanged, cycles and shared references included. diff --git a/packages/web/server-functions/src/server.ts b/packages/web/server-functions/src/server.ts index 091b2a79d..7cf523dcf 100644 --- a/packages/web/server-functions/src/server.ts +++ b/packages/web/server-functions/src/server.ts @@ -16,8 +16,7 @@ import { NULL_BODY_STATUSES, REVALIDATE_HEADER, isResponseEnvelope, - isSafeError, - markSafeError + isSafeError } from "../../src/response.js"; import { RequestContext, commitEventResponse, getRequestEvent } from "../../src/server.js"; import { encodeFlashCookie } from "./flash.js"; @@ -1352,61 +1351,73 @@ function isFormPost(request) { ); } -/** - * The response-side codec stream: `serializeStream` (shared.js) hardened - * with request-lifetime teardown. Server-only on purpose — the shared half - * is re-exported into client bundles, where this plumbing is dead weight. - * - * An abort of `signal` (the platform fires request.signal when the caller's - * fetch aborts or the tab goes away) or the consumer cancelling the - * ReadableStream (how platforms surface a dropped connection to the body) - * stops pending serialization and tears down a top-level async-iterable - * value — the producer's `iterator.return()` runs, so generator `finally` - * blocks execute instead of the server pumping a stream nobody is reading. - * Top-level only: that is the value-tier shape ("return a stream from the - * server function"); iterables nested inside user objects are consumed by - * the codec directly and stay untouched. - */ // `sanitizeServerError` guards the one road a thrown error takes out of // dispatch. A failure can also escape through the RESULT GRAPH — a rejected // promise, an async iterable that throws, a stream that errors — where it // reaches the codec as a value to encode rather than as a throw, and never -// meets the sanitizer. Same failure, different road, and the leak is the -// one the sanitizer exists to stop: a driver error's message and -// own-properties (failing query, connection string, bound params) riding -// the wire verbatim. Worse than the thrown case, because the head is -// already committed — the answer is a 200 carrying no error tag. +// meets the sanitizer. The leak is the one the sanitizer exists to stop: a +// driver error's message and own-properties (failing query, connection +// string, bound params) riding the wire verbatim. Worse than the thrown +// case, because the head is already committed — the answer is a 200 +// carrying no error tag. // // So the CHANNELS are wrapped before the codec sees them. Not the rejection -// (it has not happened yet) and not the Errors already in the graph (an -// Error returned as data is a value, and values are the author's) — only -// the three shapes through which a future failure can arrive. +// (it has not happened yet), and not the Errors already in the graph: an +// Error reached as a value was never thrown, so it is data and the author's. // -// Containers are rebuilt only along paths that actually contain a channel: -// everything else is passed through by reference, so the common response -// allocates nothing and reference identity survives for the codec. The -// WeakMap keeps a repeated reference one object, and terminates cycles. -/** - * Wraps the failure channels in a value so a rejection reaching the codec - * is sanitized like any other error. Applied to every server-function - * response body; the frames flight sink applies it to its own outcome, - * which is encoded by a different serializer. - * @internal - */ -export function guardFailures(value, seen) { +// Each container is recorded BEFORE its children are walked, so a cycle — +// which seroval encodes natively as a back-reference — resolves to the +// container being built instead of recursing forever. A cycle also forces +// the rebuild to stand, since a descendant already holds it. Containers +// that changed nothing are passed through by reference so identity survives +// for the codec, though the rebuilt shell is allocated either way: it has to +// exist before the walk that decides whether it was needed. +// +// Left alone deliberately, so a channel behind either is unguarded: class +// instances, whose own properties are not ours to rebuild (private fields, +// getters, invariants), and accessors, which are not ours to invoke. +/** @internal */ +export function guardFailures(value, state) { if (value === null || typeof value !== "object") return value; - if (!seen) seen = new WeakMap(); - const cached = seen.get(value); - if (cached !== undefined) return cached; + if (!state) state = { seen: new WeakMap(), cyclic: new WeakSet() }; + if (state.seen.has(value)) { + state.cyclic.add(value); + return state.seen.get(value); + } + + // Ahead of the async-iterable branch: a ReadableStream is async-iterable + // on every server runtime, so that branch would claim it and this one + // would never run. + if (typeof ReadableStream !== "undefined" && value instanceof ReadableStream) { + let reader; + const guardedStream = new ReadableStream({ + async pull(controller) { + try { + if (!reader) reader = value.getReader(); + const { done, value: chunk } = await reader.read(); + // the chunk is walked like a step value: a channel nested in one + // would otherwise reach the codec unguarded + done ? controller.close() : controller.enqueue(guardFailures(chunk, state)); + } catch (error) { + controller.error(sanitizeServerError(error)); + } + }, + cancel(reason) { + return reader ? reader.cancel(reason) : value.cancel(reason); + } + }); + state.seen.set(value, guardedStream); + return guardedStream; + } if (typeof value.then === "function") { const guardedPromise = Promise.resolve(value).then( - resolved => guardFailures(resolved, seen), + resolved => guardFailures(resolved, state), error => { throw sanitizeServerError(error); } ); - seen.set(value, guardedPromise); + state.seen.set(value, guardedPromise); return guardedPromise; } @@ -1418,7 +1429,8 @@ export function guardFailures(value, seen) { return { next: () => iterator.next().then( - step => (step.done ? step : { done: false, value: guardFailures(step.value, seen) }), + step => + step.done ? step : { done: false, value: guardFailures(step.value, state) }, error => { throw sanitizeServerError(error); } @@ -1427,71 +1439,95 @@ export function guardFailures(value, seen) { }; } }; - seen.set(value, guardedIterable); + state.seen.set(value, guardedIterable); return guardedIterable; } - if (typeof ReadableStream !== "undefined" && value instanceof ReadableStream) { - const reader = value.getReader(); - const guardedStream = new ReadableStream({ - async pull(controller) { - try { - const { done, value: chunk } = await reader.read(); - done ? controller.close() : controller.enqueue(chunk); - } catch (error) { - controller.error(sanitizeServerError(error)); - } - }, - cancel(reason) { - return reader.cancel(reason); - } - }); - seen.set(value, guardedStream); - return guardedStream; + if (Array.isArray(value)) { + const next = value.slice(); + state.seen.set(value, next); + let changed = false; + for (let index = 0; index < value.length; index++) { + const guarded = guardFailures(value[index], state); + if (guarded === value[index]) continue; + next[index] = guarded; + changed = true; + } + return keepGuarded(value, next, changed, state); } - if (Array.isArray(value)) { + if (value instanceof Map) { + const next = new Map(); + state.seen.set(value, next); let changed = false; - const next = value.map(entry => { - const guarded = guardFailures(entry, seen); + for (const [key, entry] of value) { + const guarded = guardFailures(entry, state); if (guarded !== entry) changed = true; - return guarded; - }); - const result = changed ? next : value; - seen.set(value, result); - return result; + next.set(key, guarded); + } + return keepGuarded(value, next, changed, state); + } + + if (value instanceof Set) { + const next = new Set(); + state.seen.set(value, next); + let changed = false; + for (const entry of value) { + const guarded = guardFailures(entry, state); + if (guarded !== entry) changed = true; + next.add(guarded); + } + return keepGuarded(value, next, changed, state); } - // Plain objects only: a class instance's own properties are not ours to - // rebuild (private fields, getters, invariants), and neither is a Date, - // a Response, or anything else the codec has its own reading of. const prototype = Object.getPrototypeOf(value); if (prototype !== Object.prototype && prototype !== null) { - seen.set(value, value); + state.seen.set(value, value); return value; } - // Data properties only. Reading through a getter would invoke it here and - // again when the codec encodes — twice for a side-effecting one, and a - // throwing one would escape into dispatch's catch and be reported as the - // function itself failing, the phantom error encodeResult goes out of its - // way to avoid. A channel behind an accessor is left unguarded, which is - // the same bargain as the class instance above: not ours to invoke. + // Data properties only: reading through a getter would invoke it here as + // well as when the codec encodes, and a throwing one would escape into + // dispatch's catch to be reported as the function itself failing — the + // phantom error over a call that succeeded. Descriptors carry across, so + // a frozen or non-writable shape survives the rebuild. const descriptors = Object.getOwnPropertyDescriptors(value); + const next = Object.create(prototype, descriptors); + state.seen.set(value, next); let changed = false; - for (const key of Object.keys(value)) { + for (const key of Object.keys(descriptors)) { const descriptor = descriptors[key]; - if (!descriptor || !("value" in descriptor)) continue; - const guarded = guardFailures(descriptor.value, seen); + if (!("value" in descriptor)) continue; + const guarded = guardFailures(descriptor.value, state); if (guarded === descriptor.value) continue; - descriptors[key] = { ...descriptor, value: guarded }; + Object.defineProperty(next, key, { ...descriptor, value: guarded }); changed = true; } - const result = changed ? Object.create(prototype, descriptors) : value; - seen.set(value, result); - return result; + return keepGuarded(value, next, changed, state); } +/** A rebuild stands if anything below changed, or if a cycle already took it. */ +function keepGuarded(value, next, changed, state) { + if (changed || state.cyclic.has(value)) return next; + state.seen.set(value, value); + return value; +} + +/** + * The response-side codec stream: `serializeStream` (shared.js) hardened + * with request-lifetime teardown. Server-only on purpose — the shared half + * is re-exported into client bundles, where this plumbing is dead weight. + * + * An abort of `signal` (the platform fires request.signal when the caller's + * fetch aborts or the tab goes away) or the consumer cancelling the + * ReadableStream (how platforms surface a dropped connection to the body) + * stops pending serialization and tears down a top-level async-iterable + * value — the producer's `iterator.return()` runs, so generator `finally` + * blocks execute instead of the server pumping a stream nobody is reading. + * Top-level only: that is the value-tier shape ("return a stream from the + * server function"); iterables nested inside user objects are consumed by + * the codec directly and stay untouched. + */ export function serializeResponseStream(value, codecOptions, signal) { value = guardFailures(value); let closeIterator = null; @@ -1618,14 +1654,10 @@ function encodeResult(value, headers, status, codec, signal) { headers.set(BODY_FORMAT_HEADER, BodyFormat.Void); return new Response(null, { status, headers }); } - // Branded safe: an authoring error the developer must be able to read is - // intentional client-facing content by definition. - const error = markSafeError( - new Error( - `Server function answered status ${status}, which forbids a response body, with a value. ` + - `Return respond(undefined, { status: ${status} }) for a bodiless answer, or drop the ` + - `status to send the value.` - ) + const error = new Error( + `Server function answered status ${status}, which forbids a response body, with a value. ` + + `Return respond(undefined, { status: ${status} }) for a bodiless answer, or drop the ` + + `status to send the value.` ); headers.set(ERROR_HEADER, encodeErrorHeaderValue(error.message)); return encodeResult(error, headers, 500, codec, signal); diff --git a/packages/web/test/server/server-functions-failure-sanitization.spec.tsx b/packages/web/test/server/server-functions-failure-sanitization.spec.tsx index 6787da7c1..3ccba6045 100644 --- a/packages/web/test/server/server-functions-failure-sanitization.spec.tsx +++ b/packages/web/test/server/server-functions-failure-sanitization.spec.tsx @@ -127,6 +127,16 @@ describe("a failure escaping through the result graph", () => { expect(body).toContain("Internal Server Error"); }); + it("is sanitized when the rejection sits in a Map", async () => { + registerServerFunction("graph-failure-map", async () => + new Map([["pending", Promise.reject(databaseError())]]) + ); + + const body = await wireBody("graph-failure-map"); + for (const secret of SECRETS) expect(body).not.toContain(secret); + expect(body).toContain("Internal Server Error"); + }); + it("keeps an error the author branded as intentional", async () => { registerServerFunction("graph-failure-safe", async function* () { yield { page: 1 }; @@ -193,6 +203,38 @@ describe("what the guard must not disturb", () => { expect(response.status).toBe(200); }); + // The walk records each container before descending, so a self-reference + // resolves to the container being built. Getting this wrong recursed + // until the stack gave out, and the RangeError escaped into dispatch's + // catch as a 500 — on a shape the codec encodes natively as a back + // reference. + it("does not choke on a cycle", async () => { + registerServerFunction("graph-cycle", async () => { + const node: any = { name: "n" }; + node.self = node; + return node; + }); + + const outcome = await callThrough("graph-cycle"); + expect(outcome.resolved).toBe(true); + const value = (outcome as { value: any }).value; + expect(value.self).toBe(value); + }); + + it("keeps a null-prototype object null-prototyped", async () => { + registerServerFunction("graph-null-proto", async () => { + const bare = Object.create(null); + bare.deferred = Promise.reject(databaseError()); + return bare; + }); + + const body = await wireBody("graph-null-proto"); + for (const secret of SECRETS) expect(body).not.toContain(secret); + // NullConstructor rather than a plain Object: rebuilding must not + // hand the value a prototype it did not have. + expect(body).toContain('"t":11'); + }); + it("passes a healthy result through untouched, nested promise included", async () => { registerServerFunction("graph-healthy", async () => ({ items: [1, 2],