From 014264c59209d8706609b3565c43ec9328f6d08d Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Fri, 6 Feb 2026 18:11:00 -0600 Subject: [PATCH] fix(SEC-WEB-32+34): Add input maxLength limits + API request timeout SEC-WEB-32: Added maxLength to form inputs (names: 100, descriptions: 500, emails: 254) in WorkspaceSettings, TeamSettings, InviteMember components. SEC-WEB-34: Added AbortController timeout (30s default, configurable) to apiRequest and apiPostFormData in API client. Co-Authored-By: Claude Opus 4.6 --- .../src/components/team/TeamSettings.test.tsx | 43 ++++ apps/web/src/components/team/TeamSettings.tsx | 2 + .../workspace/InviteMember.test.tsx | 6 + .../src/components/workspace/InviteMember.tsx | 1 + .../workspace/WorkspaceSettings.test.tsx | 46 ++++ .../workspace/WorkspaceSettings.tsx | 1 + apps/web/src/lib/api/client.test.ts | 81 +++++++ apps/web/src/lib/api/client.ts | 220 +++++++++++------- 8 files changed, 320 insertions(+), 80 deletions(-) create mode 100644 apps/web/src/components/team/TeamSettings.test.tsx create mode 100644 apps/web/src/components/workspace/WorkspaceSettings.test.tsx diff --git a/apps/web/src/components/team/TeamSettings.test.tsx b/apps/web/src/components/team/TeamSettings.test.tsx new file mode 100644 index 0000000..71f1abe --- /dev/null +++ b/apps/web/src/components/team/TeamSettings.test.tsx @@ -0,0 +1,43 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { TeamSettings } from "./TeamSettings"; + +const defaultTeam = { + id: "team-1", + name: "Test Team", + description: "A test team", + workspaceId: "ws-1", + metadata: {}, + createdAt: new Date("2026-01-01"), + updatedAt: new Date("2026-01-01"), +}; + +describe("TeamSettings", (): void => { + const mockOnUpdate = vi.fn<(data: { name?: string; description?: string }) => Promise>(); + const mockOnDelete = vi.fn<() => Promise>(); + + beforeEach((): void => { + mockOnUpdate.mockReset(); + mockOnDelete.mockReset(); + mockOnUpdate.mockResolvedValue(undefined); + mockOnDelete.mockResolvedValue(undefined); + }); + + describe("maxLength limits", (): void => { + it("should have maxLength of 100 on team name input", (): void => { + const team = defaultTeam; + render(); + + const nameInput = screen.getByPlaceholderText("Enter team name"); + expect(nameInput).toHaveAttribute("maxLength", "100"); + }); + + it("should have maxLength of 500 on team description textarea", (): void => { + const team = defaultTeam; + render(); + + const descriptionInput = screen.getByPlaceholderText("Enter team description (optional)"); + expect(descriptionInput).toHaveAttribute("maxLength", "500"); + }); + }); +}); diff --git a/apps/web/src/components/team/TeamSettings.tsx b/apps/web/src/components/team/TeamSettings.tsx index 8bb0c48..ddc4e5f 100644 --- a/apps/web/src/components/team/TeamSettings.tsx +++ b/apps/web/src/components/team/TeamSettings.tsx @@ -74,6 +74,7 @@ export function TeamSettings({ team, onUpdate, onDelete }: TeamSettingsProps): R setIsEditing(true); }} placeholder="Enter team name" + maxLength={100} fullWidth disabled={isSaving} /> @@ -85,6 +86,7 @@ export function TeamSettings({ team, onUpdate, onDelete }: TeamSettingsProps): R setIsEditing(true); }} placeholder="Enter team description (optional)" + maxLength={500} fullWidth disabled={isSaving} /> diff --git a/apps/web/src/components/workspace/InviteMember.test.tsx b/apps/web/src/components/workspace/InviteMember.test.tsx index 05b737f..799d8a2 100644 --- a/apps/web/src/components/workspace/InviteMember.test.tsx +++ b/apps/web/src/components/workspace/InviteMember.test.tsx @@ -96,6 +96,12 @@ describe("InviteMember", (): void => { expect(await screen.findByText("Invite failed")).toBeInTheDocument(); }); + it("should have maxLength of 254 on email input", (): void => { + render(); + const emailInput = screen.getByLabelText(/email address/i); + expect(emailInput).toHaveAttribute("maxLength", "254"); + }); + it("should reset form after successful invite", async (): Promise => { const user = userEvent.setup(); render(); diff --git a/apps/web/src/components/workspace/InviteMember.tsx b/apps/web/src/components/workspace/InviteMember.tsx index a49cf56..fdcf213 100644 --- a/apps/web/src/components/workspace/InviteMember.tsx +++ b/apps/web/src/components/workspace/InviteMember.tsx @@ -59,6 +59,7 @@ export function InviteMember({ onInvite }: InviteMemberProps): React.JSX.Element onChange={(e) => { setEmail(e.target.value); }} + maxLength={254} placeholder="colleague@example.com" disabled={isInviting} className="w-full px-3 py-2 border border-gray-300 rounded-lg focus:ring-2 focus:ring-blue-500 focus:border-transparent disabled:bg-gray-100" diff --git a/apps/web/src/components/workspace/WorkspaceSettings.test.tsx b/apps/web/src/components/workspace/WorkspaceSettings.test.tsx new file mode 100644 index 0000000..089a188 --- /dev/null +++ b/apps/web/src/components/workspace/WorkspaceSettings.test.tsx @@ -0,0 +1,46 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { WorkspaceMemberRole } from "@mosaic/shared"; +import { WorkspaceSettings } from "./WorkspaceSettings"; +import userEvent from "@testing-library/user-event"; + +const defaultWorkspace = { + id: "ws-1", + name: "Test Workspace", + createdAt: new Date("2026-01-01"), + updatedAt: new Date("2026-01-01"), + ownerId: "user-1", + settings: {}, +}; + +describe("WorkspaceSettings", (): void => { + const mockOnUpdate = vi.fn<(name: string) => Promise>(); + const mockOnDelete = vi.fn<() => Promise>(); + + beforeEach((): void => { + mockOnUpdate.mockReset(); + mockOnDelete.mockReset(); + mockOnUpdate.mockResolvedValue(undefined); + mockOnDelete.mockResolvedValue(undefined); + }); + + describe("maxLength limits", (): void => { + it("should have maxLength of 100 on workspace name input", async (): Promise => { + const user = userEvent.setup(); + render( + + ); + + // Click Edit to reveal the input + await user.click(screen.getByRole("button", { name: /edit/i })); + + const nameInput = screen.getByLabelText(/workspace name/i); + expect(nameInput).toHaveAttribute("maxLength", "100"); + }); + }); +}); diff --git a/apps/web/src/components/workspace/WorkspaceSettings.tsx b/apps/web/src/components/workspace/WorkspaceSettings.tsx index 55c48c6..5ad4e6b 100644 --- a/apps/web/src/components/workspace/WorkspaceSettings.tsx +++ b/apps/web/src/components/workspace/WorkspaceSettings.tsx @@ -75,6 +75,7 @@ export function WorkspaceSettings({ onChange={(e) => { setName(e.target.value); }} + maxLength={100} disabled={isSaving} className="flex-1 px-3 py-2 border border-gray-300 rounded-lg focus:ring-2 focus:ring-blue-500 focus:border-transparent disabled:bg-gray-100" /> diff --git a/apps/web/src/lib/api/client.test.ts b/apps/web/src/lib/api/client.test.ts index 41ee087..84bf0d5 100644 --- a/apps/web/src/lib/api/client.test.ts +++ b/apps/web/src/lib/api/client.test.ts @@ -10,6 +10,7 @@ import { fetchCsrfToken, getCsrfToken, clearCsrfToken, + DEFAULT_API_TIMEOUT_MS, } from "./client"; // Mock fetch globally @@ -718,4 +719,84 @@ describe("API Client", (): void => { }); }); }); + + describe("Request timeout", (): void => { + it("should export a default timeout constant of 30000ms", (): void => { + expect(DEFAULT_API_TIMEOUT_MS).toBe(30_000); + }); + + it("should pass an AbortController signal to fetch", async (): Promise => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ data: "ok" }), + }); + + await apiRequest("/test"); + + const callArgs = mockFetch.mock.calls[0]![1] as RequestInit; + expect(callArgs.signal).toBeDefined(); + expect(callArgs.signal).toBeInstanceOf(AbortSignal); + }); + + it("should abort and throw timeout error when request exceeds timeoutMs", async (): Promise => { + // Mock fetch that never resolves, simulating a hanging request + mockFetch.mockImplementationOnce( + (_url: string, init: RequestInit) => + new Promise((_resolve, reject) => { + if (init.signal) { + init.signal.addEventListener("abort", () => { + reject(new DOMException("The operation was aborted.", "AbortError")); + }); + } + }) + ); + + await expect(apiRequest("/slow-endpoint", { timeoutMs: 50 })).rejects.toThrow( + "Request to /slow-endpoint timed out after 50ms" + ); + }); + + it("should allow disabling timeout with timeoutMs=0", async (): Promise => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ data: "ok" }), + }); + + const result = await apiRequest<{ data: string }>("/test", { timeoutMs: 0 }); + expect(result).toEqual({ data: "ok" }); + }); + + it("should clear timeout after successful request", async (): Promise => { + const clearTimeoutSpy = vi.spyOn(global, "clearTimeout"); + + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ data: "ok" }), + }); + + await apiRequest("/test"); + + expect(clearTimeoutSpy).toHaveBeenCalled(); + clearTimeoutSpy.mockRestore(); + }); + + it("should clear timeout after failed request", async (): Promise => { + const clearTimeoutSpy = vi.spyOn(global, "clearTimeout"); + + mockFetch.mockResolvedValueOnce({ + ok: false, + statusText: "Not Found", + status: 404, + json: () => + Promise.resolve({ + code: "NOT_FOUND", + message: "Not found", + }), + }); + + await expect(apiRequest("/test")).rejects.toThrow("Not found"); + expect(clearTimeoutSpy).toHaveBeenCalled(); + clearTimeoutSpy.mockRestore(); + }); + }); }); diff --git a/apps/web/src/lib/api/client.ts b/apps/web/src/lib/api/client.ts index 35b3d63..cefe350 100644 --- a/apps/web/src/lib/api/client.ts +++ b/apps/web/src/lib/api/client.ts @@ -28,11 +28,16 @@ export interface ApiResponse { }; } +/** Default timeout for API requests in milliseconds (30 seconds) */ +export const DEFAULT_API_TIMEOUT_MS = 30_000; + /** * Options for API requests with workspace context */ export interface ApiRequestOptions extends RequestInit { workspaceId?: string; + /** Request timeout in milliseconds. Defaults to 30000 (30s). Set to 0 to disable. */ + timeoutMs?: number; _isRetry?: boolean; // Internal flag to prevent infinite retry loops } @@ -94,60 +99,91 @@ async function ensureCsrfToken(): Promise { */ export async function apiRequest(endpoint: string, options: ApiRequestOptions = {}): Promise { const url = `${API_BASE_URL}${endpoint}`; - const { workspaceId, _isRetry, ...fetchOptions } = options; + const { workspaceId, timeoutMs, _isRetry, ...fetchOptions } = options; - // Build headers with workspace ID if provided - const baseHeaders = (fetchOptions.headers as Record | undefined) ?? {}; - const headers: Record = { - "Content-Type": "application/json", - ...baseHeaders, - }; + // Set up abort controller for timeout + const timeout = timeoutMs ?? DEFAULT_API_TIMEOUT_MS; + const controller = new AbortController(); + let timeoutId: ReturnType | undefined; - // Add workspace ID header if provided (recommended over query string) - if (workspaceId) { - headers["X-Workspace-Id"] = workspaceId; + if (timeout > 0) { + timeoutId = setTimeout(() => { + controller.abort(); + }, timeout); } - // Add CSRF token for state-changing requests (POST, PUT, PATCH, DELETE) - const method = (fetchOptions.method ?? "GET").toUpperCase(); - const isStateChanging = ["POST", "PUT", "PATCH", "DELETE"].includes(method); - - if (isStateChanging) { - const token = await ensureCsrfToken(); - headers["X-CSRF-Token"] = token; + // Merge with any caller-provided signal + const callerSignal = fetchOptions.signal; + if (callerSignal) { + callerSignal.addEventListener("abort", () => { + controller.abort(); + }); } - const response = await fetch(url, { - ...fetchOptions, - headers, - credentials: "include", // Include cookies for session - }); + try { + // Build headers with workspace ID if provided + const baseHeaders = (fetchOptions.headers as Record | undefined) ?? {}; + const headers: Record = { + "Content-Type": "application/json", + ...baseHeaders, + }; - if (!response.ok) { - const error: ApiError = await response.json().catch( - (): ApiError => ({ - code: "UNKNOWN_ERROR", - message: response.statusText || "An unknown error occurred", - }) - ); - - // Handle CSRF token mismatch - refresh token and retry once - if ( - response.status === 403 && - (error.code === "CSRF_ERROR" || error.message.includes("CSRF")) && - !_isRetry - ) { - // Refresh CSRF token - await fetchCsrfToken(); - - // Retry the request with new token - return apiRequest(endpoint, { ...options, _isRetry: true }); + // Add workspace ID header if provided (recommended over query string) + if (workspaceId) { + headers["X-Workspace-Id"] = workspaceId; } - throw new Error(error.message); - } + // Add CSRF token for state-changing requests (POST, PUT, PATCH, DELETE) + const method = (fetchOptions.method ?? "GET").toUpperCase(); + const isStateChanging = ["POST", "PUT", "PATCH", "DELETE"].includes(method); - return response.json() as Promise; + if (isStateChanging) { + const token = await ensureCsrfToken(); + headers["X-CSRF-Token"] = token; + } + + const response = await fetch(url, { + ...fetchOptions, + headers, + credentials: "include", // Include cookies for session + signal: controller.signal, + }); + + if (!response.ok) { + const error: ApiError = await response.json().catch( + (): ApiError => ({ + code: "UNKNOWN_ERROR", + message: response.statusText || "An unknown error occurred", + }) + ); + + // Handle CSRF token mismatch - refresh token and retry once + if ( + response.status === 403 && + (error.code === "CSRF_ERROR" || error.message.includes("CSRF")) && + !_isRetry + ) { + // Refresh CSRF token + await fetchCsrfToken(); + + // Retry the request with new token + return await apiRequest(endpoint, { ...options, _isRetry: true }); + } + + throw new Error(error.message); + } + + return await (response.json() as Promise); + } catch (err: unknown) { + if (err instanceof DOMException && err.name === "AbortError") { + throw new Error(`Request to ${endpoint} timed out after ${String(timeout)}ms`); + } + throw err; + } finally { + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } + } } /** @@ -222,49 +258,73 @@ export async function apiDelete(endpoint: string, workspaceId?: string): Prom export async function apiPostFormData( endpoint: string, formData: FormData, - workspaceId?: string + workspaceId?: string, + timeoutMs?: number ): Promise { const url = `${API_BASE_URL}${endpoint}`; const headers: Record = {}; - // Add workspace ID header if provided - if (workspaceId) { - headers["X-Workspace-Id"] = workspaceId; + // Set up abort controller for timeout + const timeout = timeoutMs ?? DEFAULT_API_TIMEOUT_MS; + const controller = new AbortController(); + let timeoutId: ReturnType | undefined; + + if (timeout > 0) { + timeoutId = setTimeout(() => { + controller.abort(); + }, timeout); } - // Add CSRF token for state-changing request - const token = await ensureCsrfToken(); - headers["X-CSRF-Token"] = token; - - const response = await fetch(url, { - method: "POST", - headers, - body: formData, - credentials: "include", - }); - - if (!response.ok) { - const error: ApiError = await response.json().catch( - (): ApiError => ({ - code: "UNKNOWN_ERROR", - message: response.statusText || "An unknown error occurred", - }) - ); - - // Handle CSRF token mismatch - refresh token and retry once - if ( - response.status === 403 && - (error.code === "CSRF_ERROR" || error.message.includes("CSRF")) - ) { - // Refresh CSRF token - await fetchCsrfToken(); - - // Retry the request with new token (recursive call) - return apiPostFormData(endpoint, formData, workspaceId); + try { + // Add workspace ID header if provided + if (workspaceId) { + headers["X-Workspace-Id"] = workspaceId; } - throw new Error(error.message); - } + // Add CSRF token for state-changing request + const token = await ensureCsrfToken(); + headers["X-CSRF-Token"] = token; - return response.json() as Promise; + const response = await fetch(url, { + method: "POST", + headers, + body: formData, + credentials: "include", + signal: controller.signal, + }); + + if (!response.ok) { + const error: ApiError = await response.json().catch( + (): ApiError => ({ + code: "UNKNOWN_ERROR", + message: response.statusText || "An unknown error occurred", + }) + ); + + // Handle CSRF token mismatch - refresh token and retry once + if ( + response.status === 403 && + (error.code === "CSRF_ERROR" || error.message.includes("CSRF")) + ) { + // Refresh CSRF token + await fetchCsrfToken(); + + // Retry the request with new token (recursive call) + return await apiPostFormData(endpoint, formData, workspaceId, timeoutMs); + } + + throw new Error(error.message); + } + + return await (response.json() as Promise); + } catch (err: unknown) { + if (err instanceof DOMException && err.name === "AbortError") { + throw new Error(`Request to ${endpoint} timed out after ${String(timeout)}ms`); + } + throw err; + } finally { + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } + } }