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 <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -5,7 +5,7 @@
|
|||||||
"type": "module",
|
"type": "module",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
"dev": "vite",
|
"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",
|
"preview": "vite preview",
|
||||||
"typecheck": "tsc --noEmit",
|
"typecheck": "tsc --noEmit",
|
||||||
"test": "npm run lint:colors && vitest run",
|
"test": "npm run lint:colors && vitest run",
|
||||||
|
|||||||
@@ -34,6 +34,26 @@ describe('apiFetch', () => {
|
|||||||
expect(fetchMock).not.toHaveBeenCalled()
|
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 () => {
|
it('rejects with a typed ApiError from the error body', async () => {
|
||||||
stubFetch(errorResponse(503, 'unavailable', true))
|
stubFetch(errorResponse(503, 'unavailable', true))
|
||||||
const api = createApiClient(fakeLocation())
|
const api = createApiClient(fakeLocation())
|
||||||
|
|||||||
+19
-2
@@ -5,6 +5,21 @@ export const CSRF_HEADER = 'X-MealQuest-CSRF'
|
|||||||
|
|
||||||
export const ME_PATH = '/api/v1/me'
|
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
|
// Me is the GET /api/v1/me response: the signed-in member, the household and
|
||||||
// the session's CSRF token.
|
// the session's CSRF token.
|
||||||
export type Me = {
|
export type Me = {
|
||||||
@@ -24,8 +39,9 @@ export type ApiRequest = {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export type ApiClient = {
|
export type ApiClient = {
|
||||||
// apiFetch sends a same-origin JSON request to path and resolves with the
|
// apiFetch sends a same-origin JSON request to path, which must be under
|
||||||
// decoded response body, or rejects with an ApiError.
|
// /api/v1/, and resolves with the decoded response body, or rejects with an
|
||||||
|
// ApiError.
|
||||||
apiFetch<T>(path: string, request?: ApiRequest): Promise<T>
|
apiFetch<T>(path: string, request?: ApiRequest): Promise<T>
|
||||||
// loadMe fetches GET /api/v1/me and caches its CSRF token.
|
// loadMe fetches GET /api/v1/me and caches its CSRF token.
|
||||||
loadMe(): Promise<Me>
|
loadMe(): Promise<Me>
|
||||||
@@ -71,6 +87,7 @@ export function createApiClient(location: ApiLocation): ApiClient {
|
|||||||
|
|
||||||
async function apiFetch<T>(path: string, request: ApiRequest = {}): Promise<T> {
|
async function apiFetch<T>(path: string, request: ApiRequest = {}): Promise<T> {
|
||||||
const method = request.method ?? 'GET'
|
const method = request.method ?? 'GET'
|
||||||
|
requireApiPath(method, path)
|
||||||
let response = await send(method, path, request.body)
|
let response = await send(method, path, request.body)
|
||||||
if (response.status === 403) {
|
if (response.status === 403) {
|
||||||
const error = await parseForbidden(method, path, response)
|
const error = await parseForbidden(method, path, response)
|
||||||
|
|||||||
Reference in New Issue
Block a user