Skip to content

A GET() grant governs an id, not the function it was granted to #3237

Description

@frenzzy

Summary

A GET() declaration is recorded as the bare string "GET" against the function's id, not against the function it was made about. Because a GET declaration is also a CSRF decision — declared reads skip the origin gate (#3114) — an id whose binding changed around the declaration hands cross-site executability to a function that never signed it: a cross-site GET runs the body, un-gated, under the user's ambient cookies. The same channel disagrees with itself from the other side: withMeta(fn, { method: "POST" }) writes only the metadata bag, so getServerFunctionMetadata(fn).method reads POST while the wire still executes the function on a cross-site GET — a revocation reported but never performed.

This issue merges two symptoms that would otherwise be filed separately: (1) a GET() grant survives an id rebind that happened before the declaration ran (the mirror image of #3129, which fixed only the rebind that happens after), and (2) withMeta({ method }) misreports the declared method. They are one defect — METHODS storing a word instead of a binding — and one of them cannot be fixed coherently without the other: once withMeta stops writing method, GET() needs a channel of its own to record what it declared.

Reproduction

packages/web/get-grant-repro.mjs, run from packages/web after npm run build. It uses the compiler ABI (registerServerReference / createServerReference / registerServerFunction) directly, which is what compiled "use server" output emits — hand-writing it is how you get a repro without a build step.

import { AsyncLocalStorage } from "node:async_hooks";
import { createRequestEvent } from "./dist/server.js";
import {
  GET,
  createServerReference,
  getServerFunctionMetadata,
  handleServerFunctionRequest,
  registerServerFunction,
  registerServerReference,
  withMeta
} from "./server-functions/dist/server.js";

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

const createEvent = request => createRequestEvent(request);

// the request a hostile page can cause a browser to send, with cookies
const crossSiteGet = id =>
  new Request(`https://app.example/_server/data/${id}`, {
    method: "GET",
    headers: { "Sec-Fetch-Site": "cross-site" }
  });

const rows = [];
const row = (name, data) => rows.push({ case: name, ...data });

// CONTROL 1 — declared GET, cross-site read is allowed on purpose
{
  let ran = 0;
  GET(createServerReference(registerServerReference("control-declared", () => (ran++, "a read"))));
  const r = await handleServerFunctionRequest(crossSiteGet("control-declared"), { createEvent });
  row("CONTROL declared GET", { status: r.status, bodyRan: ran });
}

// CONTROL 2 — never declared GET, cross-site read is refused
{
  let ran = 0;
  registerServerFunction("control-undeclared", () => (ran++, "a mutation"));
  const r = await handleServerFunctionRequest(crossSiteGet("control-undeclared"), { createEvent });
  row("CONTROL never declared", { status: r.status, bodyRan: ran });
}

// CONTROL 3 — the rebind AFTER the declaration (#3129, already fixed)
{
  let ran = 0;
  GET(createServerReference(registerServerReference("rebind-after", () => "a read")));
  registerServerFunction("rebind-after", () => (ran++, "a mutation"));
  const r = await handleServerFunctionRequest(crossSiteGet("rebind-after"), { createEvent });
  row("rebind AFTER declare", { status: r.status, bodyRan: ran });
}

// CASE A — the rebind BEFORE the declaration
{
  let ran = 0;
  const declared = createServerReference(registerServerReference("rebind-before", () => "a read"));
  registerServerFunction("rebind-before", () => (ran++, "a mutation"));
  let threw = false;
  try { GET(declared); } catch { threw = true; }
  const r = await handleServerFunctionRequest(crossSiteGet("rebind-before"), { createEvent });
  row("rebind BEFORE declare", { status: r.status, bodyRan: ran, declarationThrew: threw });
}

// CASE B — withMeta narrowing the declaration back to POST
{
  let ran = 0;
  const fn = createServerReference(
    registerServerReference("withmeta-revoke", () => (ran++, "a read"))
  );
  GET(fn);
  let threw = false;
  try { withMeta(fn, { method: "POST" }); } catch { threw = true; }
  const r = await handleServerFunctionRequest(crossSiteGet("withmeta-revoke"), { createEvent });
  row("withMeta({method:'POST'})", {
    status: r.status,
    bodyRan: ran,
    withMetaThrew: threw,
    reportedMethod: getServerFunctionMetadata(fn).method
  });
}

console.table(rows);

Measured on f0f7531b, node v24.19.0:

┌─────────┬─────────────────────────────┬────────┬─────────┬──────────────────┬───────────────┬────────────────┐
│ (index) │ case                        │ status │ bodyRan │ declarationThrew │ withMetaThrew │ reportedMethod │
├─────────┼─────────────────────────────┼────────┼─────────┼──────────────────┼───────────────┼────────────────┤
│ 0       │ 'CONTROL declared GET'      │ 200    │ 1       │                  │               │                │
│ 1       │ 'CONTROL never declared'    │ 403    │ 0       │                  │               │                │
│ 2       │ 'rebind AFTER declare'      │ 403    │ 0       │                  │               │                │
│ 3       │ 'rebind BEFORE declare'     │ 200    │ 1       │ false            │               │                │
│ 4       │ "withMeta({method:'POST'})" │ 200    │ 1       │                  │ false         │ 'POST'         │
└─────────┴─────────────────────────────┴────────┴─────────┴──────────────────┴───────────────┴────────────────┘

Rows 0–2 are the controls and they are all correct: a declared read is reachable cross-site on purpose, an undeclared function is refused with 403 and never runs, and the rebind #3129 covers is refused too. Row 3 is the same rebind in the other order and it answers 200 with the mutation body executed. Row 4 accepts the method: "POST" write, reports POST, and still executes the body on a cross-site GET.

With the binding recorded on the grant and withMeta refusing method, rows 3 and 4 become 403 / bodyRan 0 and withMetaThrew true / reportedMethod "GET" respectively; rows 0–2 are unchanged.

Where

All line numbers are on f0f7531b.

The grant is stored as a word against an id

  • packages/web/server-functions/src/server.ts:1036METHODS.set(fn.id, "GET"). Unconditional: it does not ask what fn.id is currently bound to, and it does not record what the reference names.
  • packages/web/server-functions/src/server.ts:2830-2832 — dispatch's declaredRead, spelled inline as METHODS.get(functionId) === "GET". This is the value that decides both GET/HEAD dispatch and (with protectsRequest just below) whether the origin gate runs at all.
  • packages/web/server-functions/src/server.ts:2909 — the 405 Allow header, spelling the same test a second time inline. Two gates asking the same question in two places is why a fix at one site can leave the other lying.
  • packages/web/server-functions/src/server.ts:782 — the A GET() declaration outlives the function it was made about #3129 revocation, if (REGISTRATIONS.get(id) !== callback) METHODS.delete(id). Correct for the interleaving it covers, and the reason the other one is easy to miss.

Provenance: METHODS and METHODS.set(fn.id, "GET") arrived in 89a0531 ("Absorb expressions into Solid and collapse the rxcore seam", 2026-08-25) and moved to TypeScript unchanged in 7182195 (2026-08-25). The dispatch gate and the Allow header came with 258c76a ("fix(web): method allowlist, HEAD support, and cache hygiene for server function requests (#3069, #3071)", 2026-08-26). What turned a stale entry from "an extra allowed verb" into a security boundary is fc5d079 ("feat(web): name the GET read contract, add protectDeclaredReads (#3114)", 2026-08-30), which made a declared read skip the origin gate.

A fix in a neighbouring commit created the asymmetry: af4cfc8 ("fix(web): revoke GET grants on rebind; never fold single-flight into reads (#3150)", 2026-08-31) closed the rebind-after case by deleting at rebind time rather than by binding the grant, so the symmetric interleaving — the id already rebound when GET() runs — was left with nothing to delete.

The metadata channel reports what the wire did not do

  • packages/web/server-functions/src/registry.ts:78-84withMeta is a bare Object.assign(metadata, meta) with no key it refuses. Its own docstring at registry.ts:50-52 states the intent as "Writes ride the same channel GET uses … so withMeta composes with GET in either order", which is precisely the composition that does not hold for method.
  • packages/web/server-functions/src/server.ts:1038 and packages/web/server-functions/src/client.ts:992GET() records its own metadata through withMeta, so there is no write path for method that is not also a user write path.
  • packages/web/server-functions/src/shared.ts:164readonly method?: "GET" | "POST" documented as "The declared HTTP method", with nothing saying who may write it. withMeta is typed (fn: any, meta: any): any (packages/web/types/server-functions/registry.d.ts:48), so TypeScript raises nothing.
  • packages/web/server-functions/src/client.ts:1154live() branches on metadata.method === "GET" to pick its wire. The bag is not decoration; it steers code.

Provenance: withMeta and GET's use of it both arrived in 89a0531 (2026-08-25) and were carried into TypeScript by 7182195. invoke already refuses method with "The method is declaration-scoped: declare the function with GET(fn)", added in 475744c ("Add invoke — the per-call server function invocator (#3057)", 2026-08-27) — the same rule, stated in the same tree, on a channel one call away.

Why it matters

The realistic path to harm for the rebind half: a mutation becomes reachable from any origin, without the origin gate, carrying whatever cookies the browser attaches. The concrete request is a top-level cross-site GET navigation — a link the user clicks, a <form method="GET"> a hostile page submits, a redirect — which is where SameSite=Lax cookies still ride along. Same-origin policy keeps the attacker from reading the response, so the harm is the side effect, not the data: the mutation commits.

The honest limit on reachability: this needs GET() to run at a moment when the id is already bound to a different function, and ordinary compiled output does not do that. The compiler emits registerServerReference and the GET(...) wrapper in module order, back to back, so a single clean build of a single app never produces this interleaving. It takes one of:

  • an id collision between two integrations or two builds sharing an id namespace (two modules registering the same id, one of them GET-declared);
  • a declaration that is not co-located with its registration — a router or query() layer GET-wrapping a reference later, which the runtime deliberately supports via the late-bound RPC seam;
  • a hand-written registerServerFunction reusing a compiler-emitted id;
  • a live process re-evaluating modules after an edit, where the two writes can land out of order.

That is an unusual setup, and I am not claiming a browser-only, off-the-shelf exploit. What makes it worth fixing anyway is the failure mode rather than the frequency: the grant is silent, it fails open, and its consequence is CSRF-exempt execution of the wrong function. #3129 already accepted that argument for the mirror-image ordering.

The withMeta half is much easier to reach — it is ordinary application code that anybody can write and that type-checks — but its harm is narrower and I want to be precise about it: writing { method: "POST" } cannot grant anything that was not already granted, because METHODS is never touched. What it does is misreport. An audit, lint rule, route inspector or review pass that enumerates references and asserts "no mutation is GET-declared" reads POST and passes while the wire still serves the function to a cross-site GET. It is a false negative in the tools that would otherwise catch the first half, which is why the two belong in one report.

Options

For the grant/binding half

  1. Store the binding in METHODS instead of the word "GET", and answer "is this a declared read?" with METHODS.get(id) === REGISTRATIONS.get(id). Both interleavings then fall out of one rule — the after-rebind case stops depending on the eager delete, and the before-rebind case has nothing to match. Cost: a WeakMap to know which registration a reference names (createServerReference sets it), and a fallback to the live registration for references this module did not build, which leaves those exactly as they are today.
  2. Throw at declaration time when the id's current binding is not the one the reference names. Strictly louder — the mistake is reported at boot rather than tolerated at dispatch — and it composes with (1) rather than replacing it. Cost: a hard failure in setups where the ordering is merely untidy (a live process re-evaluating modules), and it can only fire for references the runtime built, so it is not a substitute for the dispatch-side check.
  3. Document that ids must be unique and GET() must follow registration. Zero runtime cost. Cost: the failure it is meant to prevent is silent and fails open, and documentation does not reach the deployment that has the collision.
  4. Push the guarantee into the compiler/adapter — emit registration and declaration as one unit. Cost: it does not cover hand-written registrations, two integrations sharing an id, or a router declaring later; the runtime is the only layer that sees both writes.

Recommended: (1), with (2) available as an addition the maintainer may or may not want. In Solid's minimalism terms it wins because it adds no public API and no new concept — one existing Map changes what it stores, the two gates that already ask the same question start calling one predicate instead of spelling it inline twice, and #3129's eager delete becomes an optimisation rather than the only thing standing between a mutation and a cross-site GET. Whether to also throw at declaration time (2) is a judgement about dev-time strictness that belongs to the maintainer; the regression test below is written to accept either answer.

For the metadata half — this one is genuinely a behaviour-vs-documentation call and I would rather frame it than assert it:

  • A. withMeta refuses method, redirecting to GET(fn) in the words invoke already uses, with GET's own write moving to an internal declareMeta. Consistent with the rule already stated one channel over; nothing new to specify. Cost: it is a breaking change — code that today calls withMeta(fn, { method }) and silently accomplishes nothing will start throwing, and live()'s branch on metadata.method stops being user-settable.
  • B. Make the write a real revocationwithMeta(fn, { method: "POST" }) drops the grant. Cost: it gives the metadata bag authority over dispatch and the origin gate, so any code path that merges a metadata bag becomes a path that can change a CSRF decision. That is a larger surface than the bug.
  • C. Document method as read-only and leave the write accepted. Cost: nothing enforces it and the misreport survives, which is the state we are in.

Recommended: A, on the minimalism argument that the tree already contains this exact rule (invoke's INVOKE_OPTION_REDIRECTS.method) and A makes one rule instead of two, while B invents a second way to change the wire. But A throws where the current code silently accepts, so it is the maintainer's call to make, not the reporter's.

Regression test

packages/web/test/server/server-functions-get-grant-binding.spec.tsx (5 tests). Three dispatch tests pin the grant to the binding — the declared-GET control, the never-declared 403, and the #3129 rebind-after — plus the rebind-before case, which reads declarationThrew back into its own expectation so that refusing the declaration loudly and dropping it quietly both pass. The fifth test asserts only that withMeta cannot report a revocation the wire did not perform, and accepts either honest answer (refuse the write, or perform the revocation) via toContainEqual, so it does not pre-decide the design call above.

Against f0f7531b:

 ❯ test/server/server-functions-get-grant-binding.spec.tsx (5 tests | 2 failed) 18ms
     × does not grant to a binding installed before the declaration ran 3ms
     × does not let withMeta report a revocation the wire did not perform 1ms

 FAIL  ... > the grant tracks the function it was granted about > does not grant to a binding installed before the declaration ran
AssertionError: expected { status: 200, mutationRan: 1, …(1) } to strictly equal { status: 403, mutationRan: +0, …(1) }

  {
    "declarationThrew": false,
-   "mutationRan": 0,
-   "status": 403,
+   "mutationRan": 1,
+   "status": 200,
  }

 FAIL  ... > what the metadata channel reports about the grant > does not let withMeta report a revocation the wire did not perform
AssertionError: expected [ { withMetaThrew: true, …(3) }, …(1) ] to deep equally contain { withMetaThrew: false, …(3) }

- Expected:
{ "ran": 1, "reportedMethod": "POST", "status": 200, "withMetaThrew": false }

+ Received:
[ { "ran": 1, "reportedMethod": "GET",  "status": 200, "withMetaThrew": true },
  { "ran": 0, "reportedMethod": "POST", "status": 403, "withMetaThrew": false } ]

It goes red on exactly the two ways one channel disagrees with itself — a grant honoured for a function it was never made about, and a metadata channel reporting a revocation the wire never performed — while the three control tests stay green, so neither failure can be green for the wrong reason.

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