diff --git a/src/services/http.ts b/src/services/http.ts index affbcef..d85b4a9 100644 --- a/src/services/http.ts +++ b/src/services/http.ts @@ -2,6 +2,7 @@ import axios, { AxiosError, type AxiosInstance, type AxiosRequestConfig } from ' import { env } from '@/config' import type { ApiErrorDto } from '@/types/api' import { useUiStore } from '@/stores/useUiStore' +import { checkRequestRateLimit, validateRequestPath } from './requestValidation' /** Supplies the bearer token attached to every request. */ export type AuthTokenProvider = () => string | null | Promise @@ -241,6 +242,16 @@ function buildClient(): AxiosInstance { }) client.interceptors.request.use(async (config) => { + const pathError = validateRequestPath(config.url) + if (pathError) { + return Promise.reject(new ApiError(400, 'VALIDATION_FAILED', pathError)) + } + + const rateLimitError = checkRequestRateLimit(config.url, config.method) + if (rateLimitError) { + return Promise.reject(new ApiError(429, 'RATE_LIMITED', rateLimitError)) + } + const token = await authTokenProvider() if (token) { config.headers.Authorization = `Bearer ${token}` @@ -251,6 +262,12 @@ function buildClient(): AxiosInstance { client.interceptors.response.use( (response) => response, async (error: AxiosError) => { + // Request-interceptor guards already produce a normalized ApiError; + // preserve its status/code instead of relabelling it as a network error. + if (error instanceof ApiError) { + return Promise.reject(error) + } + const config = error.config // Refresh only for authenticated requests; login-style 401s pass through. diff --git a/src/services/requestValidation.http.test.ts b/src/services/requestValidation.http.test.ts new file mode 100644 index 0000000..5753fd6 --- /dev/null +++ b/src/services/requestValidation.http.test.ts @@ -0,0 +1,78 @@ +import type { AxiosAdapter } from 'axios' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +// The UI store is unrelated to request validation and is currently broken on +// `main`; mock it so this test exercises the real HTTP interceptor chain. +vi.mock('@/stores/useUiStore', () => ({ + useUiStore: { getState: () => ({ pushToast: vi.fn() }) }, +})) + +import { http } from './http' +import { ARTIST_RATE_LIMIT, resetRateLimitsForTests } from './requestValidation' + +let adapterCalls = 0 + +const adapter: AxiosAdapter = async (config) => { + adapterCalls += 1 + return { + data: 'ok', + status: 200, + statusText: 'OK', + headers: {}, + config, + } +} + +beforeEach(() => { + adapterCalls = 0 + resetRateLimitsForTests() +}) + +describe('HTTP request validation (#987)', () => { + it('sends safe requests through to the adapter', async () => { + await expect(http.get('/profiles/elena_art', { adapter })).resolves.toBe('ok') + expect(adapterCalls).toBe(1) + }) + + it('rejects traversal before any request is sent', async () => { + await expect(http.get('/profiles/..%2F..%2Fetc%2Fpasswd', { adapter })).rejects.toMatchObject({ + status: 400, + code: 'VALIDATION_FAILED', + }) + await expect(http.get('/profiles/../../etc/passwd', { adapter })).rejects.toMatchObject({ + status: 400, + code: 'VALIDATION_FAILED', + }) + + expect(adapterCalls).toBe(0) + }) + + it('rejects SQL injection payloads before any request is sent', async () => { + await expect( + http.get('/artworks/1%27%20OR%20%271%27%3D%271', { adapter }), + ).rejects.toMatchObject({ + status: 400, + code: 'VALIDATION_FAILED', + }) + await expect(http.get('/listings/1%3BDROP%20TABLE%20users', { adapter })).rejects.toMatchObject( + { + status: 400, + code: 'VALIDATION_FAILED', + }, + ) + + expect(adapterCalls).toBe(0) + }) + + it('returns 429 after the artist request budget is exhausted', async () => { + for (let request = 0; request < ARTIST_RATE_LIMIT.limit; request += 1) { + await http.get('/profiles/elena_art', { adapter }) + } + + await expect(http.get('/profiles/elena_art', { adapter })).rejects.toMatchObject({ + status: 429, + code: 'RATE_LIMITED', + }) + expect(adapterCalls).toBe(ARTIST_RATE_LIMIT.limit) + }) +}) diff --git a/src/services/requestValidation.test.ts b/src/services/requestValidation.test.ts new file mode 100644 index 0000000..62be730 --- /dev/null +++ b/src/services/requestValidation.test.ts @@ -0,0 +1,110 @@ +import { beforeEach, describe, expect, it } from 'vitest' +import { + ARTIST_RATE_LIMIT, + assertValidArtistId, + checkRequestRateLimit, + resetRateLimitsForTests, + validateArtistId, + validateMarketplaceId, + validatePortfolioId, + validateRequestPath, +} from './requestValidation' + +beforeEach(() => { + resetRateLimitsForTests() +}) + +describe('validateRequestPath', () => { + it('accepts normal artist, marketplace and portfolio paths', () => { + expect(validateRequestPath('/profiles/elena_art')).toBeNull() + expect(validateRequestPath('/listings/123e4567-e89b-12d3-a456-426614174000')).toBeNull() + expect(validateRequestPath('/artworks/abc-123')).toBeNull() + expect(validateRequestPath('/orders/42?page=1')).toBeNull() + expect(validateRequestPath('/transactions/deadbeef/status')).toBeNull() + expect(validateRequestPath('/auth/reset-password/token.with.dots')).toBeNull() + }) + + it('accepts the same paths when they are absolute URLs', () => { + expect(validateRequestPath('https://api.example.com/profiles/elena_art')).toBeNull() + }) + + it('rejects path traversal attempts', () => { + expect(validateRequestPath('/profiles/../../etc/passwd')).not.toBeNull() + expect(validateRequestPath('/profiles/..%2F..%2Fetc%2Fpasswd')).not.toBeNull() + expect(validateRequestPath('/profiles/%2e%2e%2f%2e%2e%2fetc')).not.toBeNull() + expect(validateRequestPath('/wallets/GABC/..%5C..%5Cwindows')).not.toBeNull() + expect(validateRequestPath('/profiles/%00')).not.toBeNull() + }) + + it('rejects SQL/NoSQL injection payloads', () => { + expect(validateRequestPath('/artworks/1%27%20OR%20%271%27%3D%271')).not.toBeNull() + expect(validateRequestPath('/listings/1%3BDROP%20TABLE%20users')).not.toBeNull() + expect(validateRequestPath('/orders/1--')).not.toBeNull() + expect(validateRequestPath('/orders/1/*comment*/')).not.toBeNull() + expect(validateRequestPath('/orders/%7B%22%24ne%22%3A%20null%7D')).not.toBeNull() + }) + + it('rejects protocol-relative and malformed URLs', () => { + expect(validateRequestPath('//evil.example.com/steal')).not.toBeNull() + expect(validateRequestPath('/profiles/%E0%A4%A')).not.toBeNull() + }) +}) + +describe('artist and resource identifiers', () => { + it('accepts positive integers and UUIDs as artist ids', () => { + expect(validateArtistId('1')).toBe(true) + expect(validateArtistId('42')).toBe(true) + expect(validateArtistId('123e4567-e89b-12d3-a456-426614174000')).toBe(true) + expect(assertValidArtistId(' 42 ')).toBe('42') + }) + + it('rejects invalid artist ids', () => { + expect(validateArtistId('0')).toBe(false) + expect(validateArtistId('-1')).toBe(false) + expect(validateArtistId('')).toBe(false) + expect(validateArtistId('abc')).toBe(false) + expect(validateArtistId('1 OR 1=1')).toBe(false) + expect(validateArtistId('../../etc/passwd')).toBe(false) + expect(validateArtistId('1;DROP TABLE users')).toBe(false) + expect(() => assertValidArtistId('../1')).toThrow(/positive integer or uuid/i) + }) + + it('accepts safe marketplace/portfolio ids and rejects unsafe ones', () => { + expect(validateMarketplaceId('listing-123')).toBe(true) + expect(validateMarketplaceId('123e4567-e89b-12d3-a456-426614174000')).toBe(true) + expect(validatePortfolioId('artwork_42')).toBe(true) + + expect(validateMarketplaceId('../listing')).toBe(false) + expect(validateMarketplaceId('listing 123')).toBe(false) + expect(validateMarketplaceId('listing--123')).toBe(false) + expect(validatePortfolioId("42' OR '1'='1")).toBe(false) + }) +}) + +describe('checkRequestRateLimit', () => { + it('allows artist GETs up to the limit and then blocks the bucket', () => { + for (let request = 0; request < ARTIST_RATE_LIMIT.limit; request += 1) { + expect(checkRequestRateLimit('/profiles/elena_art', 'get', 0)).toBeNull() + } + + const blocked = checkRequestRateLimit('/profiles/elena_art', 'GET', 0) + expect(blocked).toMatch(/too many artist requests/i) + }) + + it('resets the bucket after the window elapses', () => { + for (let request = 0; request < ARTIST_RATE_LIMIT.limit; request += 1) { + checkRequestRateLimit('/artists/42', 'get', 0) + } + + expect(checkRequestRateLimit('/artists/42', 'get', ARTIST_RATE_LIMIT.windowMs)).toBeNull() + }) + + it('does not limit non-artist or non-GET requests', () => { + for (let request = 0; request < ARTIST_RATE_LIMIT.limit + 5; request += 1) { + expect(checkRequestRateLimit('/listings/42', 'get', 0)).toBeNull() + expect(checkRequestRateLimit('/profiles/elena_art', 'post', 0)).toBeNull() + } + + expect(checkRequestRateLimit(undefined, 'get', 0)).toBeNull() + }) +}) diff --git a/src/services/requestValidation.ts b/src/services/requestValidation.ts new file mode 100644 index 0000000..43fee94 --- /dev/null +++ b/src/services/requestValidation.ts @@ -0,0 +1,186 @@ +/** + * Request-boundary validation for dynamic API path parameters (#987). + * + * This app is a Vite SPA: it has no server routes of its own, so the last + * place the client can prevent a malicious identifier from reaching the API is + * the shared HTTP client. `validateRequestPath` rejects traversal, encoded + * separators, SQL/NoSQL metacharacters and comment markers before a request is + * sent; `checkRequestRateLimit` caps GET traffic to artist endpoints. + * + * Server-side validation and parameterised queries are still required — these + * checks are defence in depth, not a replacement for backend checks. + */ + +/** A conservative token allowed in a single path segment after decoding. */ +const SAFE_PATH_SEGMENT = /^[A-Za-z0-9][A-Za-z0-9._~@:+-]{0,127}$/ + +/** Positive integer, never zero and never negative. */ +const POSITIVE_INTEGER_ID = /^[1-9]\d*$/ + +/** RFC 4122 UUID (versions 1-5, variant 8/9/a/b). */ +const UUID_ID = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i + +/** Artist GET endpoints are capped at this many requests per minute. */ +export const ARTIST_RATE_LIMIT = { limit: 30, windowMs: 60_000 } as const + +const MAX_RATE_LIMIT_BUCKETS = 500 + +interface RateLimitBucket { + count: number + resetAt: number +} + +const rateLimitBuckets = new Map() + +function getPathname(url: string): string | null { + try { + return new URL(url, 'http://lumora.invalid').pathname + } catch { + return null + } +} + +/** + * Extracts the raw path without letting `URL` resolve dot segments first. A + * literal `/../` must still be visible to the segment validator. + */ +function getRawPath(url: string): string { + let path = url + const absolute = /^[a-z][a-z0-9+.-]*:\/\//i.exec(url) + + if (absolute) { + const pathStart = url.indexOf('/', absolute[0].length) + path = pathStart === -1 ? '/' : url.slice(pathStart) + } + + const queryStart = path.search(/[?#]/) + return queryStart === -1 ? path : path.slice(0, queryStart) +} + +/** + * Returns an error message for an unsafe request path, or `null` when every + * path segment is a safe token. + */ +export function validateRequestPath(url: string | undefined): string | null { + if (!url) return null + + // Protocol-relative URLs would escape the configured API origin. + if (url.startsWith('//')) return 'Invalid request path.' + + const rawPath = getRawPath(url) + + for (const segment of rawPath.split('/').filter(Boolean)) { + let decoded: string + try { + decoded = decodeURIComponent(segment) + } catch { + return 'Malformed URL encoding in request path.' + } + + if ( + decoded === '.' || + decoded === '..' || + decoded.includes('/') || + decoded.includes('\\') || + decoded.includes('\0') + ) { + return 'Invalid request path.' + } + + if ( + !SAFE_PATH_SEGMENT.test(decoded) || + decoded.includes('--') || + decoded.includes('/*') || + decoded.includes('*/') + ) { + return 'Invalid request path parameter.' + } + } + + return null +} + +/** True when `value` is a positive integer or a valid UUID. */ +export function validateArtistId(value: string): boolean { + const id = value.trim() + return POSITIVE_INTEGER_ID.test(id) || UUID_ID.test(id) +} + +/** + * Validates and returns an artist id. Throws on invalid input so server route + * handlers / API adapters can fail fast with a 400 response. + */ +export function assertValidArtistId(value: string): string { + const id = value.trim() + if (!validateArtistId(id)) { + throw new Error('Invalid artist id: expected a positive integer or UUID.') + } + return id +} + +/** True when `value` is a safe, single-segment marketplace/portfolio id. */ +export function validateResourceId(value: string): boolean { + const id = value.trim() + return SAFE_PATH_SEGMENT.test(id) && !id.includes('--') +} + +export const validateMarketplaceId = validateResourceId +export const validatePortfolioId = validateResourceId + +function artistRateLimitKey(pathname: string): string | null { + const firstSegment = pathname.split('/').filter(Boolean)[0]?.toLowerCase() + if (firstSegment === 'profiles' || firstSegment === 'artists' || firstSegment === 'artist') { + return firstSegment + } + return null +} + +/** + * Applies a fixed-window limit to `GET` requests on artist endpoints. + * + * Returns an error message when the limit is exceeded, otherwise `null`. + */ +export function checkRequestRateLimit( + url: string | undefined, + method: string | undefined, + now = Date.now(), +): string | null { + if (!url) return null + if ((method ?? 'get').toLowerCase() !== 'get') return null + + const pathname = getPathname(url) + if (!pathname) return null + + const key = artistRateLimitKey(pathname) + if (!key) return null + + const bucket = rateLimitBuckets.get(key) + if (!bucket || now >= bucket.resetAt) { + rateLimitBuckets.set(key, { + count: 1, + resetAt: now + ARTIST_RATE_LIMIT.windowMs, + }) + + if (rateLimitBuckets.size > MAX_RATE_LIMIT_BUCKETS) { + const oldest = rateLimitBuckets.keys().next().value + if (oldest) rateLimitBuckets.delete(oldest) + } + + return null + } + + if (bucket.count >= ARTIST_RATE_LIMIT.limit) { + const retryAfterSeconds = Math.max(1, Math.ceil((bucket.resetAt - now) / 1000)) + return `Too many artist requests. Please wait ${retryAfterSeconds} second${ + retryAfterSeconds === 1 ? '' : 's' + } and try again.` + } + + bucket.count += 1 + return null +} + +/** Clears rate-limit buckets. Intended for tests. */ +export function resetRateLimitsForTests(): void { + rateLimitBuckets.clear() +}