From cf598a84054fa0c0f4f5805f74f4f36628b92780 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sun, 4 Oct 2026 08:59:51 -0400 Subject: [PATCH] Validate API paths against the page origin, not a placeholder The guard resolved paths against a fixed placeholder origin, so //same-origin.invalid/api/v1/me passed and fetch sent the CSRF token to that host. apiFetch now resolves against location.origin, refuses anything off that origin or outside /api/v1/, and fetches the validated path-absolute URL. Co-Authored-By: Claude Opus 5.5 --- web/src/api/client.test.ts | 4 ++++ web/src/api/client.ts | 26 ++++++++++++++------------ web/src/test/fetchStub.ts | 5 +++-- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/web/src/api/client.test.ts b/web/src/api/client.test.ts index 7849607..f6fa5be 100644 --- a/web/src/api/client.test.ts +++ b/web/src/api/client.test.ts @@ -37,6 +37,10 @@ describe('apiFetch', () => { it.each([ 'https://evil.example/api/v1/me', '//evil.example/api/v1/me', + // The placeholder origin an earlier guard compared against. + '//same-origin.invalid/api/v1/me', + 'https://same-origin.invalid/api/v1/me', + '/\\evil.example/api/v1/me', '/auth/logout', '/api/v1/../auth/logout', '/api/v1/%2e%2e/auth/logout', diff --git a/web/src/api/client.ts b/web/src/api/client.ts index ea3e162..a4f840e 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -8,16 +8,17 @@ export const ME_PATH = '/api/v1/me' // API_PREFIX is the only path apiFetch sends to. const API_PREFIX = '/api/v1/' -// requireApiPath refuses any path that would not resolve to a same-origin URL -// under /api/v1/, so the CSRF token is never sent anywhere else. Resolving -// against a placeholder origin catches absolute and protocol-relative URLs -// and dot segments that climb out of /api/v1/. -function requireApiPath(method: string, path: string) { - const base = 'https://same-origin.invalid' - const url = new URL(path, base) - if (!path.startsWith('/') || url.origin !== base || !url.pathname.startsWith(API_PREFIX)) { +// apiURL resolves path against the page origin and returns it as the +// path-absolute URL to fetch. It refuses any path that would not resolve to a +// same-origin URL under /api/v1/, so the CSRF token is never sent anywhere +// else: absolute and protocol-relative URLs on another origin, and dot +// segments that climb out of /api/v1/. +function apiURL(origin: string, method: string, path: string): string { + const url = new URL(path, origin) + if (!path.startsWith('/') || url.origin !== origin || !url.pathname.startsWith(API_PREFIX)) { throw new Error(`${method} ${path}: not a same-origin ${API_PREFIX} path`) } + return url.pathname + url.search } // Me is the GET /api/v1/me response: the signed-in member, the household and @@ -30,7 +31,8 @@ export type Me = { } // ApiLocation is the part of window.location the client reads and navigates. -export type ApiLocation = Pick +// origin is the page origin every request must stay on. +export type ApiLocation = Pick export type ApiRequest = { method?: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' @@ -87,15 +89,15 @@ export function createApiClient(location: ApiLocation): ApiClient { async function apiFetch(path: string, request: ApiRequest = {}): Promise { const method = request.method ?? 'GET' - requireApiPath(method, path) - let response = await send(method, path, request.body) + const url = apiURL(location.origin, method, path) + let response = await send(method, url, request.body) if (response.status === 403) { const error = await parseForbidden(method, path, response) // Only a non-GET carries the token, so only it can be refused for it. if (error.code !== 'csrf' || method === 'GET') throw error // The session's token changed: refresh it once and retry once. await refreshMe() - response = await send(method, path, request.body) + response = await send(method, url, request.body) if (response.status === 403) throw await parseForbidden(method, path, response) } return decode(method, path, response) diff --git a/web/src/test/fetchStub.ts b/web/src/test/fetchStub.ts index 9e84d14..9a7a932 100644 --- a/web/src/test/fetchStub.ts +++ b/web/src/test/fetchStub.ts @@ -42,7 +42,8 @@ export function request(fetchMock: Mock, n: number) { } } -// fakeLocation stands in for window.location, recording navigations. +// fakeLocation stands in for window.location on https://mealquest.test, +// recording navigations. export function fakeLocation(pathname = '/', search = ''): ApiLocation & { assign: Mock<(url: string | URL) => void> } { - return { pathname, search, assign: vi.fn<(url: string | URL) => void>() } + return { origin: 'https://mealquest.test', pathname, search, assign: vi.fn<(url: string | URL) => void>() } }