Skip to content

A wrapInvocation option that is not a hook removes the configured authorization gate #3238

Description

@frenzzy
## Summary

A per-handler `wrapInvocation` option whose value is not a function removes the server-wide
`wrapInvocation` hook for that request. `handleServerFunctionRequest` resolves the option with
`options.wrapInvocation !== undefined ? options.wrapInvocation : config.wrapInvocation`, so `null`
— the other spelling of "nothing to override", and the one the neighbouring `provideEvent` line
already treats as absent — is taken as "run this call with no wrap at all". Since `wrapInvocation`
is the documented per-invocation seam for auth guards, the configured gate does not run, dispatch
executes the function body, and the caller gets a 200 with the result. `false` behaves the same way;
a non-function object gives a 500 instead of a bypass.

Scope note, so nobody merges this with its neighbour: this is only about how the option *value* is
resolved. It is a separate defect from the nested-`wrapInvocation` scope issue (a wrap owning a
request not owning the in-process calls its body makes) even though both concern `wrapInvocation`
and sit about five lines apart in the same function. Reverting the resolution fixes only this;
reverting the scope threading fixes only that.

## Reproduction

`packages/web` at `f0f7531b` (`@solidjs/web` 2.0.0-rc.6), Node v24.19.0. Save as
`packages/web/repro-wrap-null.mjs` and run `node repro-wrap-null.mjs` from `packages/web`.

```js
import { AsyncLocalStorage } from "node:async_hooks";
import {
  configureServerFunctionsServer,
  handleServerFunctionRequest,
  registerServerFunction
} from "./server-functions/dist/server.js";

globalThis[Symbol.for("solid.RequestContext")] = new AsyncLocalStorage();

// The app's authorization gate: refuse every call, never let the body run.
let gateRan = 0;
let bodyRan = 0;
configureServerFunctionsServer({
  wrapInvocation: () => {
    gateRan++;
    throw new Response(null, { status: 403 });
  }
});

registerServerFunction("secret", async () => {
  bodyRan++;
  return "the secret";
});

const post = () =>
  new Request("https://app.example/_server/data/secret", {
    method: "POST",
    body: "[]",
    headers: {
      "Sec-Fetch-Site": "same-origin",
      "content-type": "application/json",
      "X-Server-Function-Format": "8",
      "X-Server-Function-Instance": "server-function:test"
    }
  });

async function call(label, options) {
  gateRan = 0;
  bodyRan = 0;
  const response = await handleServerFunctionRequest(post(), options);
  const body = await response.text();
  console.log(
    `${label.padEnd(32)} status=${response.status}  gateRan=${gateRan}  ` +
      `bodyRan=${bodyRan}  leaked=${body.includes("the secret")}`
  );
}

await call("{}                    CONTROL", {});
await call("wrapInvocation: undefined", { wrapInvocation: undefined });
await call("provideEvent: null     CONTROL", { provideEvent: null });
await call("wrapInvocation: null", { wrapInvocation: null });
await call("wrapInvocation: false", { wrapInvocation: false });
await call("wrapInvocation: {} (not a fn)", { wrapInvocation: {} });

Measured output today:

{}                    CONTROL    status=403  gateRan=1  bodyRan=0  leaked=false
wrapInvocation: undefined        status=403  gateRan=1  bodyRan=0  leaked=false
provideEvent: null     CONTROL   status=403  gateRan=1  bodyRan=0  leaked=false
wrapInvocation: null             status=200  gateRan=0  bodyRan=1  leaked=true
wrapInvocation: false            status=200  gateRan=0  bodyRan=1  leaked=true
wrapInvocation: {} (not a fn)    status=500  gateRan=0  bodyRan=0  leaked=false

The two CONTROL rows are the point of contrast. Passing no option at all behaves correctly, and
provideEvent: null — the same absent-looking value, on the hook resolved one line earlier — also
behaves correctly, because that line spells the fallback options.provideEvent || provideEvent. Only
wrapInvocation reads null as an instruction. The {} row shows the shape of the surrounding
behaviour: a truthy non-function is not a bypass, it is a TypeError inside dispatch encoded as a
500, so the bypass is specifically the falsy-but-not-undefined values.

Measured output with the resolution changed to a typeof test, same script:

{}                    CONTROL    status=403  gateRan=1  bodyRan=0  leaked=false
wrapInvocation: undefined        status=403  gateRan=1  bodyRan=0  leaked=false
provideEvent: null     CONTROL   status=403  gateRan=1  bodyRan=0  leaked=false
wrapInvocation: null             status=403  gateRan=1  bodyRan=0  leaked=false
wrapInvocation: false            status=403  gateRan=1  bodyRan=0  leaked=false
wrapInvocation: {} (not a fn)    status=403  gateRan=1  bodyRan=0  leaked=false

Where

One site.

  • packages/web/server-functions/src/server.ts:3030-3031 — the resolution:

    const wrapInvocation =
      options.wrapInvocation !== undefined ? options.wrapInvocation : config.wrapInvocation;

    Consumed at server.ts:3166-3167, where a falsy wrapInvocation means "call run() directly".

Context for why this reads as an inconsistency rather than a deliberate choice:

  • server.ts:3021const provide = options.provideEvent || provideEvent;. Nine lines above, the
    other scope-establishing hook, and null falls back there.
  • server.ts:3024, :3029, :3032-3035collectFlightData, transformResult,
    transformFlightResult use the same !== undefined test. Those are result policy: a null there
    disables formatting, not a gate, and there is no security consequence to disabling them.
  • server.ts:445 — the option is declared wrapInvocation?: WrapInvocationHook, so null is not
    assignable. TypeScript callers cannot land here.
  • server.ts:595configureServerFunctionsServer itself uses !== undefined, so a null passed
    to configure clears the configured hook. That one is deliberate and should stay: config is where
    global policy is set, and clearing needs a spelling. The per-request option is a different
    question, because it overrides a floor rather than setting one.

Provenance: this expression has had this shape since the code entered this repository. git log -S
attributes it to 89a0531c ("Absorb expressions into Solid and collapse the rxcore seam.",
2026-08-25), which vendored the server-functions runtime in from @dom-expressions/runtime as
packages/web/src/server-functions/server.js with the line already present, and to 71821959
("Migrate the absorbed DOM runtime to TypeScript and flatten it into feature folders.", 2026-08-25),
which moved that file to today's path and carried the line through unchanged. No in-repo commit
authored it, and no neighbouring fix created it — it predates the vendoring. Worth stating plainly so
nobody goes looking for a regression that is not there.

Why it matters

The realistic path: an app that hangs authorization on wrapInvocation (the documented use — the
config option's own doc comment names "per-function middleware, auth, logging, error mapping")
serves one server-function call past its gate, with the function's real result and a 200. There is no
error, no log line, and nothing on the response that distinguishes it from an authorized call.

Now the honest limits, because they are narrow. TypeScript callers cannot produce this: null is not
assignable to WrapInvocationHook, and the app-level caller of handleServerFunctionRequest in a
typed integration will never hit it. The value has to come from somewhere with no compiler watching,
and the realistic sources are:

  • a handler-options object assembled in a plain-JS adapter or a JS integration layer;
  • a computed override — { wrapInvocation: perRoute.wrap ?? null }, or a merge helper that
    normalizes missing keys to null, which is a very ordinary thing to write and reads as
    "nothing to override" everywhere else in this file;
  • options rehydrated from a config format that has no undefined, so absence arrives as null.

An attacker cannot reach this from a request: the value is the server's own, not the caller's. This
is an app-shoots-itself defect, silently, on a security hook, where the same value is safe on the
neighbouring hook. That is the case for fixing it, and it is not the case for calling it remotely
exploitable — it is not.

Options

  1. Documentation only. Say in the option's doc comment that null disables the wrap for that
    request, and leave the code alone. Zero behaviour change, no risk to any existing caller. The
    cost is that the footgun stays, on the one hook where firing it is a 200 instead of an error, and
    it stays inconsistent with provideEvent three lines away — a reader who checks how the other
    hook resolves will draw the wrong conclusion.

  2. typeof options.wrapInvocation === "function" at the one site. Absence in any spelling means
    the configured hook still owns the call. One token's worth of runtime, no new API, no new export.
    The behaviour change is real and should be named: an app that today passes null to deliberately
    suppress the wrap for one handler stops being able to. Nothing in the repo does that, the types
    never permitted it, and the fallback it lands in — running the configured policy — is the
    conservative direction. It also turns the {} case from a 500 into a normally-gated call.

  3. Same typeof test on all four hooks (wrapInvocation, transformResult,
    collectFlightData, transformFlightResult). One rule for the whole options object, easiest to
    describe. Wider behaviour change for no safety gain: null on the result hooks disables
    formatting, which is a legitimate thing to want per handler and costs nothing when it happens by
    accident.

  4. Throw on a non-hook wrapInvocation. Loudest, and it converts a silent bypass into a visible
    failure rather than a silent recovery. But it turns a caller mistake into a 500 for a request the
    configured policy could have answered correctly, and Solid does not otherwise validate hook option
    shapes at this boundary.

  5. Leave the runtime alone, fix it in adapters. Have SolidStart and other integrations normalize
    their options before calling. Keeps the runtime minimal, but every adapter has to know, and the
    defect is in the seam that the runtime owns.

Recommendation: option 2. It is the smallest change that makes the invariant true — an option value
that is not a hook cannot remove policy — and it makes wrapInvocation agree with the
provideEvent line it sits next to, rather than adding a rule. Option 3's extra reach buys nothing
security-bearing, and options 4 and 5 both add surface (an error path, or a contract adapters must
carry) where a typeof test suffices.

Two calls here are genuinely the maintainer's, not the reporter's. First, whether the narrow fix or
the uniform one is the better contract — pinning only the security hook is deliberate here, but "one
resolution rule for the whole options object" is a defensible answer too. Second, whether a
behaviour change is warranted at all for a value the types never allowed, versus documenting it. The
patch below assumes the first answer is "narrow" and the second is "fix it", but neither is settled
by the evidence.

Regression test

packages/web/test/server/server-functions-hook-option-absence.spec.tsx, four cases: no option at
all, explicit undefined, null, and provideEvent: null as the already-correct direction to
match. It runs against the built bundles like the other server-function specs.

describe("a handler option cannot take the configured authorization gate off", () => {
  it("runs the configured wrap when no option is supplied at all", async () => {
    const call = await withConfiguredGate("absence-baseline");
    expect(await call({})).toStrictEqual({ status: 403, gateRan: 1, bodyRan: 0, leaked: false });
  });

  it("runs the configured wrap when the option is explicitly undefined", async () => {
    const call = await withConfiguredGate("absence-undefined");
    expect(await call({ wrapInvocation: undefined })).toStrictEqual({
      status: 403,
      gateRan: 1,
      bodyRan: 0,
      leaked: false
    });
  });

  it("runs the configured wrap when the option is null, the other spelling of absent", async () => {
    const call = await withConfiguredGate("absence-null");
    // today: { status: 200, gateRan: 0, bodyRan: 1, leaked: true } — the
    // gate is skipped for this one request and the body answers the caller
    expect(await call({ wrapInvocation: null })).toStrictEqual({
      status: 403,
      gateRan: 1,
      bodyRan: 0,
      leaked: false
    });
  });

  it("already treats a null provideEvent as absent, which is the direction to match", async () => {
    const call = await withConfiguredGate("absence-provide-event");
    expect(await call({ provideEvent: null })).toStrictEqual({
      status: 403,
      gateRan: 1,
      bodyRan: 0,
      leaked: false
    });
  });
});

with the fixture the rows report through:

/**
 * The app's configured gate: it refuses every call with a 403 and never
 * lets the body run. Each call reports what actually happened, so a
 * failure names the leak instead of an opaque status.
 */
function withConfiguredGate(id: string) {
  let gateRan = 0;
  let bodyRan = 0;
  configureServerFunctionsServer({
    wrapInvocation: () => {
      gateRan++;
      throw new Response(null, { status: 403 });
    }
  });
  registerServerFunction(id, async () => {
    bodyRan++;
    return "the secret";
  });
  return async (options: Record<string, unknown>) => {
    gateRan = 0;
    bodyRan = 0;
    const response = await handleServerFunctionRequest(post(id), { createEvent, ...options });
    return {
      status: response.status,
      gateRan,
      bodyRan,
      leaked: (await response.text()).includes("the secret")
    };
  };
}

Against the current resolution, the third case fails and the other three pass:

 FAIL  test/server/server-functions-hook-option-absence.spec.tsx > a handler option cannot take the
       configured authorization gate off > runs the configured wrap when the option is null, the
       other spelling of absent
AssertionError: expected { status: 200, gateRan: +0, …(2) } to strictly equal { status: 403, gateRan: 1, …(2) }

- Expected
+ Received

  {
-   "bodyRan": 0,
-   "gateRan": 1,
-   "leaked": false,
-   "status": 403,
+   "bodyRan": 1,
+   "gateRan": 0,
+   "leaked": true,
+   "status": 200,
  }

 Test Files  1 failed (1)
      Tests  1 failed | 3 passed (4)

It goes red against options.wrapInvocation !== undefined and only that: the three passing cases pin
that the fix does not change how absence, explicit undefined, or the provideEvent fallback behave.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions