Summary
The CSRF origin matcher's return value is coerced with !!, so a matcher that returns a truthy non-boolean allows the request. The gate fails open on the exact shapes a multi-tenant matcher is most likely to return.
Tested against next @ a536e29b, built from source, Node 24.19.
Reproduction
A matcher that means "deny" and says so with a string:
handleServerFunctionRequest(request, {
createEvent: q => createRequestEvent(q),
csrf: { origin: (origin) => "no" } // typed `boolean`, returns a string
});
Request carries Origin: https://evil.example and no Sec-Fetch-Site (so the matcher is actually consulted — with that header present the gate short-circuits before the matcher and the probe proves nothing):
status = 200 matcher called = 1 server function ran = 1
The call is allowed, cross-origin, and the function executes with ambient cookies.
Which returns are safe and which are not
| matcher returns |
outcome |
true |
allowed (correct) |
false |
403 (correct) |
undefined |
403 — fails closed, correct |
"no" / "deny" |
200 — allowed |
{ allowed: false } |
200 — allowed |
0 / "" |
403 (correct by accident) |
So the undefined case — the one a naive implementation hits by forgetting a return — is already safe. The dangerous shapes are the deliberate-looking ones: a verdict object, or a string naming the decision. Both are natural things to write in a matcher that also wants to log or explain its decision, and both read as "denied" to the author.
The declared type is boolean | Promise<boolean>, so this is out of contract — but TypeScript does not survive the .js integration, the compiled config, or a matcher whose return type widens through a helper. And the failure direction is the one that matters: a type error here disables the CSRF gate rather than breaking the build.
Why it is worth changing rather than documenting
Every other bound at this boundary refuses on a value it cannot interpret — an unusable body-format header answers 400 rather than calling the function with undefined, and an over-deep argument list is refused rather than truncated. The origin gate is the one place where an uninterpretable value is resolved permissively.
It is also the only hook whose wrong return has a security consequence rather than a correctness one. wrapInvocation returning the wrong thing produces a wrong result; csrf.origin returning the wrong thing produces an open door.
The strict idiom is already used, 23 lines below
if (typeof matcher === "function") return !!(await matcher(origin, request)); // ← coerces
…
return options.allowRequestsWithoutOriginCheck === true; // ← strict
Both decide the same thing — whether this request is allowed through the gate — and the second one already refuses to trust a truthy value. Whatever reasoning produced === true there applies here.
Options
- Require a real boolean —
(await matcher(origin, request)) === true. Fails closed on every non-boolean, matches the declared type exactly, and a matcher returning true is unaffected. Verified: every non-true row flips to 403 and the suite is unchanged (48 files / 524 passed / 2 skipped).
- Refuse a non-boolean loudly — treat it as a configuration error: 500 in production, throw in dev. Louder, and arguably right since it is a misconfiguration rather than a denied request, but it turns a currently-serving deployment into a failing one on upgrade.
- Accept
boolean and reject everything else at configuration time, when configureServerFunctionsServer runs, rather than per request. Catches it once at boot instead of on every call; cannot catch a matcher whose return varies.
- Document that the return is coerced. Weakest — the coercion is invisible at the call site and the consequence is an open gate.
(1) looks right: it is the smallest change, it makes the runtime match its own declared type, and it moves the failure to the safe side without changing any correct matcher's behaviour.
Happy to send a PR. The regression test shape is a matcher table — true, false, undefined, "no", {}, 0 — asserting 403 for everything that is not true, with a Sec-Fetch-Site-free request so the matcher is genuinely reached (with the header present the gate short-circuits and every row passes vacuously).
Summary
The CSRF origin matcher's return value is coerced with
!!, so a matcher that returns a truthy non-boolean allows the request. The gate fails open on the exact shapes a multi-tenant matcher is most likely to return.Tested against
next@a536e29b, built from source, Node 24.19.Reproduction
A matcher that means "deny" and says so with a string:
Request carries
Origin: https://evil.exampleand noSec-Fetch-Site(so the matcher is actually consulted — with that header present the gate short-circuits before the matcher and the probe proves nothing):The call is allowed, cross-origin, and the function executes with ambient cookies.
Which returns are safe and which are not
truefalseundefined"no"/"deny"{ allowed: false }0/""So the
undefinedcase — the one a naive implementation hits by forgetting areturn— is already safe. The dangerous shapes are the deliberate-looking ones: a verdict object, or a string naming the decision. Both are natural things to write in a matcher that also wants to log or explain its decision, and both read as "denied" to the author.The declared type is
boolean | Promise<boolean>, so this is out of contract — but TypeScript does not survive the.jsintegration, the compiled config, or a matcher whose return type widens through a helper. And the failure direction is the one that matters: a type error here disables the CSRF gate rather than breaking the build.Why it is worth changing rather than documenting
Every other bound at this boundary refuses on a value it cannot interpret — an unusable body-format header answers 400 rather than calling the function with
undefined, and an over-deep argument list is refused rather than truncated. The origin gate is the one place where an uninterpretable value is resolved permissively.It is also the only hook whose wrong return has a security consequence rather than a correctness one.
wrapInvocationreturning the wrong thing produces a wrong result;csrf.originreturning the wrong thing produces an open door.The strict idiom is already used, 23 lines below
Both decide the same thing — whether this request is allowed through the gate — and the second one already refuses to trust a truthy value. Whatever reasoning produced
=== truethere applies here.Options
(await matcher(origin, request)) === true. Fails closed on every non-boolean, matches the declared type exactly, and a matcher returningtrueis unaffected. Verified: every non-truerow flips to 403 and the suite is unchanged (48 files / 524 passed / 2 skipped).booleanand reject everything else at configuration time, whenconfigureServerFunctionsServerruns, rather than per request. Catches it once at boot instead of on every call; cannot catch a matcher whose return varies.(1) looks right: it is the smallest change, it makes the runtime match its own declared type, and it moves the failure to the safe side without changing any correct matcher's behaviour.
Happy to send a PR. The regression test shape is a matcher table —
true,false,undefined,"no",{},0— asserting 403 for everything that is nottrue, with aSec-Fetch-Site-free request so the matcher is genuinely reached (with the header present the gate short-circuits and every row passes vacuously).