diff --git a/src/services/api/__tests__/errorSanitization.test.ts b/src/services/api/__tests__/errorSanitization.test.ts new file mode 100644 index 0000000..f893b59 --- /dev/null +++ b/src/services/api/__tests__/errorSanitization.test.ts @@ -0,0 +1,58 @@ +import { + buildSanitizedApiError, + containsUrlOrPath, + getSafeErrorMessage, + sanitizeErrorMessage, +} from '../errorSanitization'; + +const FOUR_XX = [400, 401, 403, 404, 408, 409, 422, 429]; +const FIVE_XX = [500, 502, 503, 504]; + +const LEAKY_MESSAGES = [ + 'Request failed for https://api.teachlink.com/api/users/42/profile', + 'GET /api/v1/courses/123/lessons failed with 404', + 'Error at /api/auth/login', +]; + +const expectNoUrl = (message: string) => { + expect(message).not.toMatch(/https?:\/\//); + expect(message).not.toMatch(/\/api\//); + expect(containsUrlOrPath(message)).toBe(false); +}; + +describe('getSafeErrorMessage', () => { + it.each([...FOUR_XX, ...FIVE_XX])('returns a non-empty, URL-free message for %i', status => { + const message = getSafeErrorMessage(status); + expect(typeof message).toBe('string'); + expect(message.length).toBeGreaterThan(0); + expectNoUrl(message); + }); + + it('falls back to a generic message for unknown/zero status', () => { + expectNoUrl(getSafeErrorMessage(undefined)); + expectNoUrl(getSafeErrorMessage(0)); + }); +}); + +describe('sanitizeErrorMessage', () => { + it.each(LEAKY_MESSAGES)('strips URLs/paths from: %s', leaked => { + expectNoUrl(sanitizeErrorMessage(leaked, 400)); + }); + + it('passes a clean message through unchanged', () => { + expect(sanitizeErrorMessage('Please try again later.', 500)).toBe('Please try again later.'); + }); + + it('falls back to a safe message when message is empty', () => { + expect(sanitizeErrorMessage(undefined, 404)).toBe(getSafeErrorMessage(404)); + }); +}); + +describe('buildSanitizedApiError', () => { + it.each([...FOUR_XX, ...FIVE_XX])('produces a URL-free payload for %i', status => { + const error = buildSanitizedApiError(status, 'TEST_CODE'); + expect(error.status).toBe(status); + expect(error.code).toBe('TEST_CODE'); + expectNoUrl(error.message); + }); +}); diff --git a/src/services/api/axios.config.ts b/src/services/api/axios.config.ts index d29ead5..d162744 100644 --- a/src/services/api/axios.config.ts +++ b/src/services/api/axios.config.ts @@ -27,6 +27,7 @@ import { invalidateCacheForBatchRequests, invalidateCacheForMutation, } from './cache'; +import { buildSanitizedApiError } from './errorSanitization'; import { requestQueue } from './requestQueue'; // ─── Helpers ──────────────────────────────────────────────────────────────── @@ -218,7 +219,10 @@ apiClient.interceptors.response.use( const receivedRequestId = response.headers['x-request-id']; if (sentRequestId && receivedRequestId && sentRequestId !== receivedRequestId) { - appLogger.warnSync('Request ID mismatch', { sent: sentRequestId, received: receivedRequestId }); + appLogger.warnSync('Request ID mismatch', { + sent: sentRequestId, + received: receivedRequestId, + }); } popLogContext(); @@ -572,8 +576,20 @@ apiClient.interceptors.response.use( // ─── Default fallback ────────────────────────────────────────────────── - return Promise.reject(error); + sentryContextService.captureException(error, { + tags: { 'api.error': 'unhandled_response' }, + contexts: { + request: { + url: originalRequest?.url, + method: originalRequest?.method?.toUpperCase(), + status: status ?? null, + }, + }, + fingerprint: ['api-unhandled-error', String(status ?? 'unknown')], + }); + + return Promise.reject(buildSanitizedApiError(status, error.code)); } ); -export default apiClient; \ No newline at end of file +export default apiClient; diff --git a/src/services/api/errorSanitization.ts b/src/services/api/errorSanitization.ts new file mode 100644 index 0000000..5075a11 --- /dev/null +++ b/src/services/api/errorSanitization.ts @@ -0,0 +1,69 @@ +/** + * API error message sanitization (Issue #579). + * + * User-facing error messages must never contain request URLs or endpoint + * paths. The full url + method are sent to Sentry request context and + * structured logs instead — never embedded in the message shown to the UI. + */ + +/** Generic, URL-free user-facing messages keyed by HTTP status. */ +const STATUS_MESSAGES: Record = { + 400: 'The request could not be processed. Please check your input.', + 401: 'Your session has expired. Please log in again.', + 403: 'You are not allowed to perform this action.', + 404: 'The requested resource could not be found.', + 408: 'The request timed out. Please try again.', + 422: 'Some information provided was invalid. Please review and retry.', +}; + +/** Return a safe, URL-free message for a given HTTP status. */ +export function getSafeErrorMessage(status?: number): string { + if (status === undefined || status === 0) { + return 'Something went wrong. Please try again.'; + } + if (STATUS_MESSAGES[status]) { + return STATUS_MESSAGES[status]; + } + if (status >= 500) { + return 'Server error. Please try again later.'; + } + if (status >= 400) { + return 'The request could not be completed. Please try again.'; + } + return 'Something went wrong. Please try again.'; +} + +// Matches absolute URLs and multi-segment paths like /api/users/42. +// Non-global so .test() is stateless. +const URL_OR_PATH = /(https?:\/\/[^\s]+)|(\/[\w.~%-]+(?:\/[\w.~%-]+)+)/; + +/** True if a string contains a URL or a multi-segment endpoint path. */ +export function containsUrlOrPath(message: string): boolean { + return URL_OR_PATH.test(message); +} + +/** + * Defense-in-depth: if a message contains a URL/path, replace it with a safe + * generic message; otherwise pass it through unchanged. + */ +export function sanitizeErrorMessage(message: string | undefined, status?: number): string { + if (!message || containsUrlOrPath(message)) { + return getSafeErrorMessage(status); + } + return message; +} + +export interface SanitizedApiError { + message: string; + status: number; + code?: string; +} + +/** Build a normalized, URL-free error payload for the UI. */ +export function buildSanitizedApiError(status?: number, code?: string): SanitizedApiError { + return { + message: getSafeErrorMessage(status), + status: status ?? 0, + ...(code ? { code } : {}), + }; +}