fix(SEC-WEB-32+34): Add input maxLength limits + API request timeout
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
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 <noreply@anthropic.com>
This commit is contained in:
43
apps/web/src/components/team/TeamSettings.test.tsx
Normal file
43
apps/web/src/components/team/TeamSettings.test.tsx
Normal file
@@ -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<void>>();
|
||||
const mockOnDelete = vi.fn<() => Promise<void>>();
|
||||
|
||||
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(<TeamSettings team={team} onUpdate={mockOnUpdate} onDelete={mockOnDelete} />);
|
||||
|
||||
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(<TeamSettings team={team} onUpdate={mockOnUpdate} onDelete={mockOnDelete} />);
|
||||
|
||||
const descriptionInput = screen.getByPlaceholderText("Enter team description (optional)");
|
||||
expect(descriptionInput).toHaveAttribute("maxLength", "500");
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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}
|
||||
/>
|
||||
|
||||
@@ -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(<InviteMember onInvite={mockOnInvite} />);
|
||||
const emailInput = screen.getByLabelText(/email address/i);
|
||||
expect(emailInput).toHaveAttribute("maxLength", "254");
|
||||
});
|
||||
|
||||
it("should reset form after successful invite", async (): Promise<void> => {
|
||||
const user = userEvent.setup();
|
||||
render(<InviteMember onInvite={mockOnInvite} />);
|
||||
|
||||
@@ -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"
|
||||
|
||||
46
apps/web/src/components/workspace/WorkspaceSettings.test.tsx
Normal file
46
apps/web/src/components/workspace/WorkspaceSettings.test.tsx
Normal file
@@ -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<void>>();
|
||||
const mockOnDelete = vi.fn<() => Promise<void>>();
|
||||
|
||||
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<void> => {
|
||||
const user = userEvent.setup();
|
||||
render(
|
||||
<WorkspaceSettings
|
||||
workspace={defaultWorkspace}
|
||||
userRole={WorkspaceMemberRole.OWNER}
|
||||
onUpdate={mockOnUpdate}
|
||||
onDelete={mockOnDelete}
|
||||
/>
|
||||
);
|
||||
|
||||
// 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");
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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"
|
||||
/>
|
||||
|
||||
@@ -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<void> => {
|
||||
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<void> => {
|
||||
// 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<void> => {
|
||||
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<void> => {
|
||||
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<void> => {
|
||||
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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -28,11 +28,16 @@ export interface ApiResponse<T> {
|
||||
};
|
||||
}
|
||||
|
||||
/** 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,8 +99,28 @@ async function ensureCsrfToken(): Promise<string> {
|
||||
*/
|
||||
export async function apiRequest<T>(endpoint: string, options: ApiRequestOptions = {}): Promise<T> {
|
||||
const url = `${API_BASE_URL}${endpoint}`;
|
||||
const { workspaceId, _isRetry, ...fetchOptions } = options;
|
||||
const { workspaceId, timeoutMs, _isRetry, ...fetchOptions } = options;
|
||||
|
||||
// Set up abort controller for timeout
|
||||
const timeout = timeoutMs ?? DEFAULT_API_TIMEOUT_MS;
|
||||
const controller = new AbortController();
|
||||
let timeoutId: ReturnType<typeof setTimeout> | undefined;
|
||||
|
||||
if (timeout > 0) {
|
||||
timeoutId = setTimeout(() => {
|
||||
controller.abort();
|
||||
}, timeout);
|
||||
}
|
||||
|
||||
// Merge with any caller-provided signal
|
||||
const callerSignal = fetchOptions.signal;
|
||||
if (callerSignal) {
|
||||
callerSignal.addEventListener("abort", () => {
|
||||
controller.abort();
|
||||
});
|
||||
}
|
||||
|
||||
try {
|
||||
// Build headers with workspace ID if provided
|
||||
const baseHeaders = (fetchOptions.headers as Record<string, string> | undefined) ?? {};
|
||||
const headers: Record<string, string> = {
|
||||
@@ -121,6 +146,7 @@ export async function apiRequest<T>(endpoint: string, options: ApiRequestOptions
|
||||
...fetchOptions,
|
||||
headers,
|
||||
credentials: "include", // Include cookies for session
|
||||
signal: controller.signal,
|
||||
});
|
||||
|
||||
if (!response.ok) {
|
||||
@@ -141,13 +167,23 @@ export async function apiRequest<T>(endpoint: string, options: ApiRequestOptions
|
||||
await fetchCsrfToken();
|
||||
|
||||
// Retry the request with new token
|
||||
return apiRequest<T>(endpoint, { ...options, _isRetry: true });
|
||||
return await apiRequest<T>(endpoint, { ...options, _isRetry: true });
|
||||
}
|
||||
|
||||
throw new Error(error.message);
|
||||
}
|
||||
|
||||
return response.json() as Promise<T>;
|
||||
return await (response.json() as Promise<T>);
|
||||
} 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,11 +258,24 @@ export async function apiDelete<T>(endpoint: string, workspaceId?: string): Prom
|
||||
export async function apiPostFormData<T>(
|
||||
endpoint: string,
|
||||
formData: FormData,
|
||||
workspaceId?: string
|
||||
workspaceId?: string,
|
||||
timeoutMs?: number
|
||||
): Promise<T> {
|
||||
const url = `${API_BASE_URL}${endpoint}`;
|
||||
const headers: Record<string, string> = {};
|
||||
|
||||
// Set up abort controller for timeout
|
||||
const timeout = timeoutMs ?? DEFAULT_API_TIMEOUT_MS;
|
||||
const controller = new AbortController();
|
||||
let timeoutId: ReturnType<typeof setTimeout> | undefined;
|
||||
|
||||
if (timeout > 0) {
|
||||
timeoutId = setTimeout(() => {
|
||||
controller.abort();
|
||||
}, timeout);
|
||||
}
|
||||
|
||||
try {
|
||||
// Add workspace ID header if provided
|
||||
if (workspaceId) {
|
||||
headers["X-Workspace-Id"] = workspaceId;
|
||||
@@ -241,6 +290,7 @@ export async function apiPostFormData<T>(
|
||||
headers,
|
||||
body: formData,
|
||||
credentials: "include",
|
||||
signal: controller.signal,
|
||||
});
|
||||
|
||||
if (!response.ok) {
|
||||
@@ -260,11 +310,21 @@ export async function apiPostFormData<T>(
|
||||
await fetchCsrfToken();
|
||||
|
||||
// Retry the request with new token (recursive call)
|
||||
return apiPostFormData<T>(endpoint, formData, workspaceId);
|
||||
return await apiPostFormData<T>(endpoint, formData, workspaceId, timeoutMs);
|
||||
}
|
||||
|
||||
throw new Error(error.message);
|
||||
}
|
||||
|
||||
return response.json() as Promise<T>;
|
||||
return await (response.json() as Promise<T>);
|
||||
} 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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user