feat(service): origin guard for cookie-authenticated requests

Merged
DA
8d ago
+561-6

Summary

Step 19 of 📱 Web support › Refactors to land first: an Origin/CSRF guard for cookie-authenticated requests to the hosted service, so step 20 can safely let the bearer-only routes accept the browser's cookie session. Until now the service had no CORS, Origin, or CSRF handling of its own.

The rule (packages/service/src/http/origin-guard.ts, applied first thing in routeFromHttpRouter, which 109 of the 111 route files export through):

  • GET, HEAD, and OPTIONS pass. A request that carries no cookie also passes, since it holds no session to ride on. The desktop (which names its account in Authorization), the scheduled sweep, and the webhooks send no cookie.
  • Any other request (a state-changing method with a cookie) is refused unless its Origin exactly matches one of the deployment's own origins. A lowercase or unknown method counts as state-changing.
    • Where Origin is missing, Sec-Fetch-Site: same-origin counts as a match.
    • Where Origin is present, it is held to it whatever Sec-Fetch-Site says.
  • An Authorization header beside a cookie buys nothing. Better Auth's bearer plugin ignores a token it did not sign, and getSession then reads the session from the cookie. A junk Authorization could otherwise carry a cookie past the guard, with only the absence of CORS standing in the way.
    • This departs from the plan's wording, which says the guard "applies to cookie requests that carry no Authorization header". The change is in the stricter direction, and no current caller sends both.
  • The refusal is the hosted vocabulary's existing {"error":"not-permitted"} under its 403, from errorResponse. It is logged at warn with its path and Origin, each quoted, and nothing else: no query, cookie, or body. PRIVACY.md already covers this ("every line our service logs").
    • I chose an existing slug over a new one because a new slug would change the wire contract and the hostedErrorSchema JSON-schema golden. A same-origin client never receives this refusal. If you'd rather have a dedicated slug, say so and I'll add it.

The allowlist is a new ownOrigins on authDeployment (auth-deployment.ts), read from the same variables as baseURL:

  • Production: the origin of BETTER_AUTH_URL, or none if it's unset.
  • A Preview: its two hostnames, VERCEL_URL and VERCEL_BRANCH_URL, or none if neither is set.
  • No Vercel environment named: BETTER_AUTH_URL, or else the Vite dev server, http://localhost:5173.

Only https: and http: origins count, so an address that is not a web page's never makes the opaque null its own. The list never contains Better Auth's trustedOrigins, which on production trusts every luke-*-stage-review.vercel.app preview and localhost:5173 for the OAuth proxy. A test pins that production refuses those origins.

Exemptions, each with its reason

PathWhy it must take a cross-site cookie POST
/api/auth/* (prefix, shared with auth-app.ts's route as AUTH_PATH_PREFIX)Better Auth's own handler. It runs its own origin check against trustedOrigins, which the OAuth proxy needs wider than this guard's list. It is also where providers return a sign-in, possibly as a form POST.

Checked and not exempted, because the rule already passes them or they need no exemption:

  • The provider returns: /api/github/installed, /api/github/installed/authorized, /api/account/integrations/connected, /api/workspaces/linear/connected.
    • Providers send the browser back to these with a GET, which the guard never refuses. They are admitted by their connect's state, never by the cookie.
    • A provider that one day returned with a cross-site POST carrying a cookie would be refused. A SameSite=Lax cookie, Better Auth's kind, doesn't ride such a POST.
  • /api/github/webhook: GitHub's servers send no cookie.
  • /api/automations/<id>/webhook: its callers send Authorization: Bearer <trigger secret> and no cookie.
  • /api/maintenance/sweep and /api/maintenance/provider-keys-rewrap: Authorization: Bearer CRON_SECRET, and no cookie.
  • The voice and terminal sockets don't go through routeFromHttpRouter, and they already refuse any Origin. server/feedback.mjs isn't a routeFromHttpRouter route either.

Why matching on the path is safe. The guard reads the path as dispatchRoutes restored it, which is the same path the router matches. It exempts only what begins with /api/auth/ as spelled. A spelling the router would normalize to it (another case, a doubled slash, percent-encoding) is guarded like any other path, so the guard can only be stricter than the router. The only wildcard routes in any group are the not-found routes and Better Auth's own. So an exempt path reaches Better Auth or a not-found. That holds even through a hand-built /api/default.js?route=<cookie route>&route-path=auth/…, which a test pins. The exemption is keyed on the path rather than on the route file on purpose: every route file mounts its whole group.

Callers that keep working. The cookie routes' callers today:

  • The connect page's POST /api/github/connect/claim and the admin dashboard's PUT/DELETE /api/admin/favorite are same-origin fetches, so their Origin is the deployment's own.
  • The invitation and desktop-session pages call Better Auth, which is exempt.
  • The desktop sends its session bearer in Authorization for the workspace and GitHub routes. Its Node fetch keeps no cookie jar, so it never carries a cookie.

What changes. A state-changing request that carries a cookie, from a foreign origin or with no origin at all, now gets a 403 on every route but Better Auth's. On the cookie routes, such a request used to reach the route and act on the session. Those are the workspace and GitHub-connection routes behind resolveBrowserSessionUserId, and the admin dashboard's write. On the bearer-only routes, without a valid bearer, such a request was already a 401. The desktop's requests are unchanged.

Out of scope, as the step says: resolveHostedUserId accepting the cookie (step 20), and CORS response headers.

Held for the developer: this is security-sensitive and changes existing cookie routes, so it waits for your review and your ./scripts/run.sh --preview run (docs/WORKFLOW.md step 3).

Evidence

  • pnpm exec vitest run packages/service/src/http/origin-guard.test.ts: 17 passed. Through the public cookieOriginGuard, the tests show:
    • Refused:
      • A cookie POST with no Origin, including with Sec-Fetch-Site: cross-site or same-site.
      • A cookie POST from a foreign Origin: null, http:// against https://, and a look-alike host.
      • An Origin spelled other than a browser spells the deployment's own: another case, :443, or a trailing slash.
      • A cookie POST that carries a junk Authorization, whether from a foreign origin or with none.
      • A lowercase patch or an unknown method.
      • Every path but Better Auth's, the provider returns among them, and any other spelling of Better Auth's path.
      • On production, the preview and dev-server origins Better Auth trusts.
      • On a production or Preview that names no address of its own, every origin, the dev server's included.
      • Origin: null where BETTER_AUTH_URL is not a web address.
    • Passed:
      • A cookie POST, PUT, DELETE, or PATCH from the deployment's own Origin.
      • A cookie POST with no Origin but Sec-Fetch-Site: same-origin.
      • A request with no cookie: the desktop's bearer, a webhook, or one with a foreign origin.
      • GET, HEAD, and OPTIONS from any page, a provider return among them.
      • Better Auth's routes.
      • Both of a Preview's hostnames, though production's origin is foreign to a Preview.
      • The dev server, where no Vercel environment is named.
    • Logged: a refusal names its path and origin, and nothing of its query or cookie.
  • pnpm exec vitest run apps/web/tests/route-effect.test.ts: 5 passed, 3 of them new, through routeFromHttpRouter.
    • A cookie POST from a foreign page, or one naming no origin, is refused before the handler answers. The deployment's own page and the desktop reach the handler.
    • A hand-built dispatch that hands a cookie route Better Auth's path reaches only its not-found.
    • Better Auth's route takes a foreign cookie POST and answers it itself.
    • apps/web/tests/auth-app.test.ts (the same request object reaches Better Auth, and its answer is carried unchanged), function-dispatch.test.ts, and auth-deployment.test.ts all pass unchanged.
  • The new tests fail without the change:
    • With route-effect.ts reverted to main, the adaptor tests fail.
    • With the /api/auth/ exemption removed, both exemption tests fail.
    • On the first commit's behavior (Authorization skipping the guard, and ownOrigins falling back to the dev server with no web-origin filter), the junk-Authorization, misnamed-deployment, and opaque-origin tests fail.
  • pnpm check:affected: exit 0 on the head commit (00f53e890). Typecheck, lint, knip, and discarded-effect passed, and so did the tests: @luke/service's three shards (365, 221, and 213) and @luke/web (185). On this 4-core, 8 GB sandbox the parallel run sometimes loses a task to the OOM killer (tsc, discarded-effect, a vitest fork), so each pass came from rerunning it, with the rest replayed from cache.
  • CodeQL flagged the logging test's logged.includes(FOREIGN_ORIGIN) as incomplete URL substring sanitization, a false positive in a test. The test now compares the log line's quoted values with deepEqual instead (00f53e890).
  • Test count: +20 (17 in origin-guard.test.ts, 3 in route-effect.test.ts), none removed or skipped.
  • No apps/desktop file is touched, so the desktop was not booted. CI is Linux only and says nothing about the Mac. The run.sh --preview run is yours.

Invariants

  • ./scripts/check.sh green. (CI)
  • JSON Schema goldens unchanged (LUKE_UPDATE_FIXTURES not run).
  • Every behavior change has, in this PR, the test that fails without it; no change-detector test added.
  • Gateway envelope goldens unchanged.
  • No new as type assertion.
  • No Effect.runPromise/runSync/runFork outside the runtime edges: the guard is a plain function over a Request, called in route-effect.ts.
  • No new setTimeout/setInterval/new Promise/AbortController/fs.watch.
  • No Effect, Stream, or Layer written as a statement and dropped.
  • Test count: +20, listed above.
  • No hand-rolled file replaced.
  • No CLAUDE.md/AGENTS.md sentence names a changed part wrongly. apps/web/server/README.md ("A route built from a route layer") now describes the guard.
  • PRIVACY.md unchanged: the guard keeps nothing, and its one warn line (path and origin) is among "every line our service logs".
  • Package deps match imports: @luke/service gains the ./http/origin-guard export, and no new dependency.
  • Barrel/door rule: nothing new reaches a renderer bundle.

Notes

  • Blockers or follow-up verification: held for the developer's review and ./scripts/run.sh --preview against this PR's Preview.

    • On the Preview in a browser, the GitHub connect page's claim and the admin dashboard's star should still work.
    • Workspace actions, a workspace's GitHub connect, and a Linear connect from the Mac should be unchanged.
  • apps/web/server/README.md lost one extra trailing blank line to the repository's pre-commit formatter.

  • One provenance note for review. Some lines in apps/web/tests/route-effect.test.ts were not written in the agent's current turn. While the review fixes were under way, they appeared in the working tree between a read and a write, most likely from the agent's own turn before a sandbox restart, finishing a write it no longer had in context:

    • the dispatcher test "a dispatch that names Better Auth's path on another route reaches none of that route's handlers";
    • its writeRoute() helper and WRITE_ROUTE_KEY;
    • the first guard test's rename to "…and the desktop's".

    The agent reviewed every line, kept them, and they pass. They are the dispatcher case the review asked for. Every other line was written deliberately and re-read before push.

Made with Luke


Devin Review


Summary by cubic

Adds an origin guard to every request flowing through routeFromHttpRouter, so a browser's cookie session can later be accepted on the currently bearer-only routes without letting foreign pages act on it.

State-changing requests (anything but GET, HEAD, and OPTIONS) that carry a cookie are now refused with the hosted not-permitted error (403) unless the Origin matches the deployment's own origins, or the request names no origin but sends Sec-Fetch-Site: same-origin. A cookie request is held to its origin whatever Authorization rides beside it, since Better Auth reads the session from the cookie whenever that header is not a bearer it signed; the desktop, the scheduled sweep, and the webhooks send no cookie and pass untouched. Refusals are logged at warn with the path and origin only.

The allowlist is a new ownOrigins on authDeployment (production's origin from BETTER_AUTH_URL, a preview's two hostnames, or http://localhost:5173 in dev), never Better Auth's broader trustedOrigins. A production or preview that names no address of its own owns no origin. Only Better Auth's /api/auth/* routes are exempt, since they run their own origin check against trustedOrigins; the provider-return paths need no exemption because providers return with a GET the guard never refuses. The guard is covered by unit tests through the public cookieOriginGuard and adapter tests through routeFromHttpRouter, each refusal case failing without its change.