fix: collapse invalid Vary * combination and case-duplicate tokens

Found during the Phase 5B pass-3 architectural audit: sanitizeModuleResponse's
Vary merge used a plain Set, so an existing "Vary: *" from a module handler
became "*, Origin, Cookie" (RFC 9110 §12.5.5 forbids combining "*" with any
other value), and a differently-cased existing token (e.g. "origin") produced
a redundant duplicate alongside the forced "Origin"/"Cookie". mergedVaryHeader
now de-duplicates case-insensitively, always emits canonical Origin/Cookie
casing, and collapses to a bare "*" whenever one is present, per focused unit
tests. The full app's own pre-existing cors() middleware (apps/api/src/app.ts,
unchanged here, present since Phase 4B on every /api/v1/* route including
/api/v1/principal) still unconditionally appends its own "Vary: Origin"
outside this module's control — noted in the new tests, not addressed by this
narrow fix.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
ekklesia
2026-07-25 10:14:13 -05:00
co-authored by Claude Sonnet 5
parent dfaf324a65
commit bf76f4e006
2 changed files with 122 additions and 11 deletions
+32 -10
View File
@@ -52,6 +52,35 @@ function sanitizedModuleHandlerFailure(c: Context): Response {
return c.json({ code: "MODULE_HANDLER_FAILED" }, 500);
}
/**
* `Vary` lists header *names*, which are themselves case-insensitive, so merging de-duplicates
* case-insensitively — a module's own `origin` must collapse with the forced `Origin` rather than
* appear as a separate, redundant entry — and always emits the canonical `Origin`/`Cookie` casing
* regardless of how (or whether) a module already listed them. A literal `*` (RFC 9110 §12.5.5)
* must never be combined with any other value; since `*` already means "may vary on anything," a
* strictly broader signal than any explicit list, an existing `*` collapses the result to `*` alone
* rather than producing the invalid `*, Origin, Cookie`.
*/
export function mergedVaryHeader(existingVary: string | null): string {
const canonicalByLowercase = new Map<string, string>();
for (const token of (existingVary ?? "").split(",")) {
const trimmed = token.trim();
if (trimmed.length === 0) {
continue;
}
const key = trimmed.toLowerCase();
if (!canonicalByLowercase.has(key)) {
canonicalByLowercase.set(key, trimmed);
}
}
if (canonicalByLowercase.has("*")) {
return "*";
}
canonicalByLowercase.set("origin", "Origin");
canonicalByLowercase.set("cookie", "Cookie");
return [...canonicalByLowercase.values()].join(", ");
}
/**
* The platform owns CORS and caching for every module route: strips any CORS-control header a
* handler set (defense in depth — the outer `cors()` middleware already computes the authoritative
@@ -59,7 +88,8 @@ function sanitizedModuleHandlerFailure(c: Context): Response {
* includes `Origin` and `Cookie`, since even a public-baseline handler receives an authorized
* principal and may personalize its response. A separate cache-policy contract is future work;
* this is the conservative default until then. A module's own legitimate `Vary` values (if any)
* are preserved alongside the forced ones.
* are preserved alongside the forced ones. Never reads or buffers `response.body` — it is passed
* through to the new `Response` unread, so a streaming body is neither consumed nor blocked here.
*/
function sanitizeModuleResponse(response: Response): Response {
const headers = new Headers(response.headers);
@@ -67,15 +97,7 @@ function sanitizeModuleResponse(response: Response): Response {
headers.delete(headerName);
}
headers.set("Cache-Control", "no-store");
const varyValues = new Set(
(headers.get("Vary") ?? "")
.split(",")
.map((value) => value.trim())
.filter((value) => value.length > 0),
);
varyValues.add("Origin");
varyValues.add("Cookie");
headers.set("Vary", [...varyValues].join(", "));
headers.set("Vary", mergedVaryHeader(headers.get("Vary")));
return new Response(response.body, {
headers,
status: response.status,