commitEventResponse mutates the returned Response in place, so a cached Response accumulates every user's Set-Cookie #3155

Summary

commitEventResponse folds the request event's Set-Cookie values onto the outgoing response by mutating that response's headers in place. A server function that returns a module-level cached Response therefore accumulates every request's cookies onto one shared object — so one user's session cookie is sent to the next user.

Tested against next @ 320f1f5c, built from source, Node 24.19.

packages/web/src/server.ts:

export function commitEventResponse(response, event = getRequestEvent()) {
  const stub = event && event.response;
  if (!stub || !stub.headers || stub.committed) return response;
  const cookies = stub.headers.getSetCookie ? stub.headers.getSetCookie() : [];
  …
  try {
    for (const cookie of cookies) response.headers.append("Set-Cookie", cookie);

Nothing here is wrong in isolation — the rebuild path exists for immutable headers, and folding cookies entry-by-entry is deliberately correct (commas are legal inside Expires). The problem is that a mutable Response the application still holds a reference to is mutated rather than copied.

Reproduction

Runnable as-is:

import { AsyncLocalStorage } from "node:async_hooks";
const als = new AsyncLocalStorage();
globalThis[Symbol.for("solid.RequestContext")] = als;

import { createRequestEvent } from "@solidjs/web";
import {
  registerServerFunction, handleServerFunctionRequest
} from "@solidjs/web/server-functions/server";

const REDIRECT_HOME = new Response(null, { status: 302, headers: { Location: "/" } });
registerServerFunction("go", async () => REDIRECT_HOME);

for (const user of ["alice", "bob", "carol"]) {
  const response = await handleServerFunctionRequest(
    // the BARE address — /_server/data/go (scripted) does not leak
    new Request("http://localhost/_server/go", {
      method: "POST",
      headers: { "Sec-Fetch-Site": "same-origin" }   // without this: 403
    }),
    {
      // per-call option — createEvent is NOT a configureServerFunctionsServer key
      createEvent: request => {
        const event = createRequestEvent(request);
        event.response.headers.append("Set-Cookie", `sid=${user}; Path=/; HttpOnly`);
        return event;
      }
    }
  );
  console.log(user, response === REDIRECT_HOME, response.headers.getSetCookie());
}

alice  same-object=true  ["sid=alice; Path=/; HttpOnly"]
bob    same-object=true  ["sid=alice; Path=/; HttpOnly","sid=bob; Path=/; HttpOnly"]
carol  same-object=true  ["sid=alice; Path=/; HttpOnly","sid=bob; Path=/; HttpOnly","sid=carol; Path=/; HttpOnly"]

singleton now permanently holds: ["sid=alice…","sid=bob…","sid=carol…"]

Control — a fresh Response per call — gives each user exactly their own cookie.

Bob's response carries Alice's HttpOnly session cookie. No error, no warning, nothing in a log.

What makes this reachable rather than exotic

The leaking shape is the bodiless singleton, and it is a normal thing to write:

const REDIRECT_HOME = new Response(null, { status: 302, headers: { Location: "/" } });
const NO_CONTENT    = new Response(null, { status: 204 });
const cache = new Map(); // per-tenant memoized Response

A body-carrying singleton also accumulates, but self-destructs on the second call (Body is unusable), so an author hits an obvious error and switches to .clone(), which is safe. Bodiless singletons have no such tripwire and leak silently.

The second requirement is a middleware that writes a cookie — session rotation, a CSRF token, a locale preference. That is ordinary.

Options

The invariant worth stating is that the handler must not mutate a Response its caller may reuse — not only this one function (see the caveat below).

1. Always rebuild when there is anything to fold. The rebuild path already exists for immutable headers; making it unconditional whenever cookies.length || hasGaps removes the aliasing. The untouched case still short-circuits at if (!cookies.length && !hasGaps) return response;, so the cost lands only on responses that were going to be modified anyway. I checked that copyInitHeaders round-trips multiple Set-Cookie correctly and that fillsStubGap(key, headers, response) still reads the right response.body, so this is sound as far as it goes.
2. Copy on write — clone the headers, fold onto the copy, construct the response around it. Same effect, marginally less allocation in some cases.
3. Detect reuse and warn — a WeakSet of already-committed responses plus a dev-only console.error. Cheap, and it converts a silent cross-user bug into a loud one, but it does not fix it. Reasonable alongside 1 or 2.
4. Document that a returned Response must be freshly constructed. I'd argue against this as the only response: the failure is silent, cross-user, and involves session material.

I'd suggest (1), and (3) alongside it if the warning is cheap enough to keep.

Caveat on (1) — it is necessary but not sufficient. Two sites run after commitEventResponse and also write in place, falling back to a rebuild only in a catch:

• withCSRFVary — response.headers.set("Vary", vary)
• finalizeTransportResponse — response.headers.set("Cache-Control", "no-store")

On a stub with no cookies and no gaps, commitEventResponse short-circuits and those two still stamp Vary and Cache-Control permanently onto the app's singleton. Less harmful than a session cookie, but the same class of bug, so the fix is worth applying at the level of the invariant rather than to one function.

Note also that (1) does not rescue a body-carrying singleton — the rebuild wraps response.body, so the second call still hits Body is unusable. That is fine and probably correct; just not something the fix changes.

Two ways a probe on this path silently looks like a clean negative, both of which cost me a run: createEvent passed to configureServerFunctionsServer is silently ignored (it is a per-call option only), so no stub exists and nothing folds; and any pre-dispatch failure answers an unscripted caller with a bodiless, header-less 500. Assert the function actually ran.

Happy to send a PR with the fix and a regression test — three sequential requests through one cached Response, asserting each carries exactly its own cookie, with the fresh-Response control alongside.