From 5519676c792acaba2b3bcca1e520389705a0f932 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sun, 4 Oct 2026 08:52:44 -0400 Subject: [PATCH 1/2] Restrict apiFetch to same-origin /api/v1/ and lint colours in the build apiFetch now refuses any path that does not resolve to a same-origin URL under /api/v1/ before attaching X-MealQuest-CSRF or calling fetch, so the cached CSRF token cannot be sent to another origin or another path. npm run build runs lint:colors first, so a hex literal outside tokens.css fails the production build. Maestro task: f1bb5c9e-ca49-4a3c-b07d-7ed79f0e7fe5 Co-Authored-By: Claude Opus 5.5 --- web/package.json | 2 +- web/src/api/client.test.ts | 20 ++++++++++++++++++++ web/src/api/client.ts | 21 +++++++++++++++++++-- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/web/package.json b/web/package.json index 0055edc..9ce06bc 100644 --- a/web/package.json +++ b/web/package.json @@ -5,7 +5,7 @@ "type": "module", "scripts": { "dev": "vite", - "build": "tsc --noEmit && vite build && npm run check:dist-fonts", + "build": "npm run lint:colors && tsc --noEmit && vite build && npm run check:dist-fonts", "preview": "vite preview", "typecheck": "tsc --noEmit", "test": "npm run lint:colors && vitest run", diff --git a/web/src/api/client.test.ts b/web/src/api/client.test.ts index 9a2d2c8..7849607 100644 --- a/web/src/api/client.test.ts +++ b/web/src/api/client.test.ts @@ -34,6 +34,26 @@ describe('apiFetch', () => { expect(fetchMock).not.toHaveBeenCalled() }) + it.each([ + 'https://evil.example/api/v1/me', + '//evil.example/api/v1/me', + '/auth/logout', + '/api/v1/../auth/logout', + '/api/v1/%2e%2e/auth/logout', + 'api/v1/me', + ])('refuses %s without sending it or the CSRF token', async (path) => { + const fetchMock = stubFetch(jsonResponse(200, ME)) + const api = createApiClient(fakeLocation()) + await api.loadMe() + + await expect(api.apiFetch(path, { method: 'POST', body: {} })).rejects.toThrow( + `POST ${path}: not a same-origin /api/v1/ path`, + ) + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(request(fetchMock, 0)).toMatchObject({ path: '/api/v1/me', method: 'GET' }) + expect(request(fetchMock, 0).headers.has(CSRF_HEADER)).toBe(false) + }) + it('rejects with a typed ApiError from the error body', async () => { stubFetch(errorResponse(503, 'unavailable', true)) const api = createApiClient(fakeLocation()) diff --git a/web/src/api/client.ts b/web/src/api/client.ts index ac5cc2c..ea3e162 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -5,6 +5,21 @@ export const CSRF_HEADER = 'X-MealQuest-CSRF' 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)) { + throw new Error(`${method} ${path}: not a same-origin ${API_PREFIX} path`) + } +} + // Me is the GET /api/v1/me response: the signed-in member, the household and // the session's CSRF token. export type Me = { @@ -24,8 +39,9 @@ export type ApiRequest = { } export type ApiClient = { - // apiFetch sends a same-origin JSON request to path and resolves with the - // decoded response body, or rejects with an ApiError. + // apiFetch sends a same-origin JSON request to path, which must be under + // /api/v1/, and resolves with the decoded response body, or rejects with an + // ApiError. apiFetch(path: string, request?: ApiRequest): Promise // loadMe fetches GET /api/v1/me and caches its CSRF token. loadMe(): Promise @@ -71,6 +87,7 @@ 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) if (response.status === 403) { const error = await parseForbidden(method, path, response) -- 2.55.0 From cf598a84054fa0c0f4f5805f74f4f36628b92780 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sun, 4 Oct 2026 08:59:51 -0400 Subject: [PATCH 2/2] 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>() } } -- 2.55.0