From 0fcdd40630544771186f2be5ab713642a392120f Mon Sep 17 00:00:00 2001 From: Chris Kehayias Date: Sun, 13 Sep 2026 07:31:45 -0400 Subject: [PATCH] test(auth): prove disabledPaths is wired, not just declared MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The F-UPDATE-USER advisory requires the disabled endpoints to 404 at the Better Auth router. This repo asserted only that `disabledAuthPaths` — an exported array constant — contains those paths. Nothing tied that array to the running router. Commenting out `disabledPaths: disabledAuthPaths` in the `betterAuth()` options deleted the defence-in-depth control outright and the entire 805-test suite stayed green. That was verified by doing it, not inferred. `src/app/api/auth/[...all]/route.test.ts` did not cover the gap either, contrary to how it reads: it mocks `@/lib/auth` to `{}` and mocks `toNextJsHandler`, so it exercises the allowlist WRAPPER against a stub and never reaches real Better Auth. Correct for what it tests, but it means no test in the suite drove the real router. These tests call `auth.handler` with real `Request`s, deliberately bypassing Next.js routing and the allowlist, because the allowlist is the primary control and `disabledPaths` is the layer behind it. The advisory is explicit that the allowlist is an addition, not a replacement, so both must hold independently. Coverage: - `POST /update-user` with the advisory's exact payload (a well-formed foreign `User_GUID`) → 404. Refused on the path, before body parsing: `userGuid` is necessarily `input: true`, so no validator could distinguish this from `mapProfileToUser`. - Every other entry in `disabledAuthPaths`, table-driven → 404. - CONTROL: `/get-session` is NOT 404. Without it the block could pass vacuously — a handler that 404s everything would satisfy all of the above. Negative control: unwiring `disabledPaths` now fails 4 tests, including `/update-user`. Note `/set-password` and `/delete-user/callback` pass in both states — Better Auth does not mount them without an email/password provider, so those two assert intent rather than active protection, and should not be counted as coverage. 812 tests pass, eslint clean. Co-Authored-By: Claude Opus 5 (1M context) --- src/auth.test.ts | 71 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/auth.test.ts b/src/auth.test.ts index f1c09b8..47692e7 100644 --- a/src/auth.test.ts +++ b/src/auth.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { parseAdditionalUserInput } from 'better-auth/db'; import { + auth, userAdditionalFields, disabledAuthPaths, syntheticEmailForSub, @@ -277,6 +278,76 @@ describe('Auth - disabled endpoints', () => { }); }); +/** + * F-UPDATE-USER, enforcement half — the paths must 404 at the Better Auth + * ROUTER, not merely appear in an exported array. + * + * The `disabledAuthPaths` assertions above check a constant. They pass even if + * `disabledPaths: disabledAuthPaths` is deleted from the `betterAuth()` options, + * because nothing ties the array to the running router — verified by removing + * that line and watching the whole suite stay green. That is the exact failure + * mode this advisory's root cause describes: a protection silently stops being + * applied and no test notices. + * + * `src/app/api/auth/[...all]/route.test.ts` does not cover this either. It mocks + * `@/lib/auth` to `{}` and mocks `toNextJsHandler`, so it exercises the allowlist + * WRAPPER against a stub — correct for what it tests, but it never reaches real + * Better Auth. + * + * So these drive `auth.handler` directly with real `Request`s, deliberately + * BYPASSING Next.js routing and the allowlist. The allowlist is the primary + * control and sits in front of this; `disabledPaths` is the defence in depth + * behind it, and this is the only place that proves the latter is wired in. + * Both must hold independently — the advisory is explicit that the allowlist is + * an addition, not a replacement. + * + * Matched in the router's `onRequest`, these 404 before rate limiting, plugins + * and `sessionMiddleware` — so no session is needed to prove the refusal. + */ +describe('Auth - disabled endpoints 404 at the router', () => { + const url = (path: string) => `http://localhost:3000/api/auth${path}`; + + it('404s POST /update-user, the session-identity takeover vector', async () => { + const res = await auth.handler( + new Request(url('/update-user'), { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + // The exact payload from the advisory: a well-formed foreign User_GUID. + // It must be refused on the PATH, before any body parsing — `userGuid` + // is necessarily `input: true`, so a validator could never tell this + // from `mapProfileToUser`. + body: JSON.stringify({ userGuid: 'ab12cd34-ef56-7890-abcd-ef1234567890' }), + }), + ); + + expect(res.status).toBe(404); + }); + + it.each(disabledAuthPaths.filter((p) => p !== '/update-user'))( + '404s the other identity-mutating endpoint %s', + async (path) => { + const res = await auth.handler(new Request(url(path), { method: 'POST' })); + + expect(res.status).toBe(404); + }, + ); + + /** + * CONTROL — without this the block above could pass vacuously: a handler that + * 404s EVERYTHING (a bad base path, a broken instance) would satisfy every + * assertion above while proving nothing. + * + * `/get-session` is not in `disabledAuthPaths`, so it must reach the router. + * Asserting only "not 404" keeps this about routing rather than about what an + * unauthenticated session read happens to return. + */ + it('CONTROL: a non-disabled path still routes', async () => { + const res = await auth.handler(new Request(url('/get-session'))); + + expect(res.status).not.toBe(404); + }); +}); + /** * F2 — Ministry Platform enforces NO uniqueness on email addresses, but Better * Auth keys identity on email: `handleOAuthUserInfo` looks the user up by