Security and Code Quality Remediation (M6-Fixes) #343
@@ -232,4 +232,142 @@ describe("useWebSocket", (): void => {
|
||||
expect(mockSocket.off).toHaveBeenCalledWith("task:updated", expect.any(Function));
|
||||
expect(mockSocket.off).toHaveBeenCalledWith("task:deleted", expect.any(Function));
|
||||
});
|
||||
|
||||
describe("connect_error handling", (): void => {
|
||||
it("should handle connect_error events and expose error state", async (): Promise<void> => {
|
||||
const { result } = renderHook(() => useWebSocket("workspace-123", "token"));
|
||||
|
||||
expect(result.current.connectionError).toBeNull();
|
||||
|
||||
const error = new Error("Connection refused");
|
||||
|
||||
act(() => {
|
||||
eventHandlers.connect_error?.(error);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.connectionError).toEqual({
|
||||
message: "Connection refused",
|
||||
type: "connect_error",
|
||||
description: "Failed to establish WebSocket connection",
|
||||
});
|
||||
expect(result.current.isConnected).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
it("should handle connect_error with missing message", async (): Promise<void> => {
|
||||
const { result } = renderHook(() => useWebSocket("workspace-123", "token"));
|
||||
|
||||
const error = new Error();
|
||||
|
||||
act(() => {
|
||||
eventHandlers.connect_error?.(error);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.connectionError).toEqual({
|
||||
message: "Connection failed",
|
||||
type: "connect_error",
|
||||
description: "Failed to establish WebSocket connection",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it("should clear connection error on reconnect", async (): Promise<void> => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ workspaceId }: { workspaceId: string }) => useWebSocket(workspaceId, "token"),
|
||||
{ initialProps: { workspaceId: "workspace-1" } }
|
||||
);
|
||||
|
||||
// Simulate connect error
|
||||
act(() => {
|
||||
eventHandlers.connect_error?.(new Error("Connection failed"));
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.connectionError).not.toBeNull();
|
||||
});
|
||||
|
||||
// Rerender with new workspace to trigger reconnect
|
||||
rerender({ workspaceId: "workspace-2" });
|
||||
|
||||
// Connection error should be cleared when attempting new connection
|
||||
await waitFor(() => {
|
||||
expect(result.current.connectionError).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it("should register connect_error handler on socket", (): void => {
|
||||
renderHook(() => useWebSocket("workspace-123", "token"));
|
||||
|
||||
expect(mockSocket.on).toHaveBeenCalledWith("connect_error", expect.any(Function));
|
||||
});
|
||||
|
||||
it("should clean up connect_error handler on unmount", (): void => {
|
||||
const { unmount } = renderHook(() => useWebSocket("workspace-123", "token"));
|
||||
|
||||
unmount();
|
||||
|
||||
expect(mockSocket.off).toHaveBeenCalledWith("connect_error", expect.any(Function));
|
||||
});
|
||||
});
|
||||
|
||||
describe("WSS enforcement", (): void => {
|
||||
afterEach((): void => {
|
||||
vi.unstubAllEnvs();
|
||||
});
|
||||
|
||||
it("should warn when using ws:// in production", (): void => {
|
||||
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined);
|
||||
|
||||
vi.stubEnv("NODE_ENV", "production");
|
||||
vi.stubEnv("NEXT_PUBLIC_API_URL", "http://insecure-server.com");
|
||||
|
||||
renderHook(() => useWebSocket("workspace-123", "token"));
|
||||
|
||||
expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("[Security Warning]"));
|
||||
expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("insecure protocol"));
|
||||
|
||||
consoleWarnSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("should not warn when using https:// in production", (): void => {
|
||||
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined);
|
||||
|
||||
vi.stubEnv("NODE_ENV", "production");
|
||||
vi.stubEnv("NEXT_PUBLIC_API_URL", "https://secure-server.com");
|
||||
|
||||
renderHook(() => useWebSocket("workspace-123", "token"));
|
||||
|
||||
expect(consoleWarnSpy).not.toHaveBeenCalled();
|
||||
|
||||
consoleWarnSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("should not warn when using wss:// in production", (): void => {
|
||||
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined);
|
||||
|
||||
vi.stubEnv("NODE_ENV", "production");
|
||||
vi.stubEnv("NEXT_PUBLIC_API_URL", "wss://secure-server.com");
|
||||
|
||||
renderHook(() => useWebSocket("workspace-123", "token"));
|
||||
|
||||
expect(consoleWarnSpy).not.toHaveBeenCalled();
|
||||
|
||||
consoleWarnSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("should not warn in development mode even with http://", (): void => {
|
||||
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined);
|
||||
|
||||
vi.stubEnv("NODE_ENV", "development");
|
||||
vi.stubEnv("NEXT_PUBLIC_API_URL", "http://localhost:3001");
|
||||
|
||||
renderHook(() => useWebSocket("workspace-123", "token"));
|
||||
|
||||
expect(consoleWarnSpy).not.toHaveBeenCalled();
|
||||
|
||||
consoleWarnSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -31,9 +31,32 @@ interface WebSocketCallbacks {
|
||||
onProjectUpdated?: (project: Project) => void;
|
||||
}
|
||||
|
||||
interface ConnectionError {
|
||||
message: string;
|
||||
type: string;
|
||||
description?: string;
|
||||
}
|
||||
|
||||
interface UseWebSocketReturn {
|
||||
isConnected: boolean;
|
||||
socket: Socket | null;
|
||||
connectionError: ConnectionError | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if the WebSocket URL uses secure protocol (wss://)
|
||||
* Logs a warning in production when using insecure ws://
|
||||
*/
|
||||
function validateWebSocketSecurity(url: string): void {
|
||||
const isProduction = process.env.NODE_ENV === "production";
|
||||
const isSecure = url.startsWith("https://") || url.startsWith("wss://");
|
||||
|
||||
if (isProduction && !isSecure) {
|
||||
console.warn(
|
||||
"[Security Warning] WebSocket connection using insecure protocol (ws://). " +
|
||||
"Authentication tokens may be exposed. Use wss:// in production."
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -42,7 +65,7 @@ interface UseWebSocketReturn {
|
||||
* @param workspaceId - The workspace ID to subscribe to
|
||||
* @param token - Authentication token
|
||||
* @param callbacks - Event callbacks for real-time updates
|
||||
* @returns Connection status and socket instance
|
||||
* @returns Connection status, socket instance, and connection error
|
||||
*/
|
||||
export function useWebSocket(
|
||||
workspaceId: string,
|
||||
@@ -51,6 +74,7 @@ export function useWebSocket(
|
||||
): UseWebSocketReturn {
|
||||
const [socket, setSocket] = useState<Socket | null>(null);
|
||||
const [isConnected, setIsConnected] = useState<boolean>(false);
|
||||
const [connectionError, setConnectionError] = useState<ConnectionError | null>(null);
|
||||
|
||||
const {
|
||||
onTaskCreated,
|
||||
@@ -66,6 +90,12 @@ export function useWebSocket(
|
||||
// Get WebSocket URL from environment or default to API URL
|
||||
const wsUrl = process.env.NEXT_PUBLIC_API_URL ?? "http://localhost:3001";
|
||||
|
||||
// Validate WebSocket security - warn if using insecure connection in production
|
||||
validateWebSocketSecurity(wsUrl);
|
||||
|
||||
// Clear any previous connection error
|
||||
setConnectionError(null);
|
||||
|
||||
// Create socket connection
|
||||
const newSocket = io(wsUrl, {
|
||||
auth: { token },
|
||||
@@ -83,8 +113,18 @@ export function useWebSocket(
|
||||
setIsConnected(false);
|
||||
};
|
||||
|
||||
const handleConnectError = (error: Error): void => {
|
||||
setConnectionError({
|
||||
message: error.message || "Connection failed",
|
||||
type: "connect_error",
|
||||
description: "Failed to establish WebSocket connection",
|
||||
});
|
||||
setIsConnected(false);
|
||||
};
|
||||
|
||||
newSocket.on("connect", handleConnect);
|
||||
newSocket.on("disconnect", handleDisconnect);
|
||||
newSocket.on("connect_error", handleConnectError);
|
||||
|
||||
// Real-time event handlers
|
||||
if (onTaskCreated) {
|
||||
@@ -113,6 +153,7 @@ export function useWebSocket(
|
||||
return (): void => {
|
||||
newSocket.off("connect", handleConnect);
|
||||
newSocket.off("disconnect", handleDisconnect);
|
||||
newSocket.off("connect_error", handleConnectError);
|
||||
|
||||
if (onTaskCreated) newSocket.off("task:created", onTaskCreated);
|
||||
if (onTaskUpdated) newSocket.off("task:updated", onTaskUpdated);
|
||||
@@ -139,5 +180,6 @@ export function useWebSocket(
|
||||
return {
|
||||
isConnected,
|
||||
socket,
|
||||
connectionError,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -12,6 +12,7 @@ describe("WebSocketProvider", (): void => {
|
||||
mockUseWebSocket.mockReturnValue({
|
||||
isConnected: true,
|
||||
socket: null,
|
||||
connectionError: null,
|
||||
});
|
||||
|
||||
function TestComponent(): React.JSX.Element {
|
||||
@@ -33,6 +34,7 @@ describe("WebSocketProvider", (): void => {
|
||||
mockUseWebSocket.mockReturnValue({
|
||||
isConnected: false,
|
||||
socket: null,
|
||||
connectionError: null,
|
||||
});
|
||||
|
||||
const onTaskCreated = vi.fn();
|
||||
@@ -86,6 +88,7 @@ describe("WebSocketProvider", (): void => {
|
||||
mockUseWebSocket.mockReturnValue({
|
||||
isConnected: false,
|
||||
socket: null,
|
||||
connectionError: null,
|
||||
});
|
||||
|
||||
function TestComponent(): React.JSX.Element {
|
||||
@@ -105,6 +108,7 @@ describe("WebSocketProvider", (): void => {
|
||||
mockUseWebSocket.mockReturnValue({
|
||||
isConnected: true,
|
||||
socket: null,
|
||||
connectionError: null,
|
||||
});
|
||||
|
||||
rerender(
|
||||
|
||||
@@ -22,9 +22,16 @@ interface DeletePayload {
|
||||
id: string;
|
||||
}
|
||||
|
||||
interface ConnectionError {
|
||||
message: string;
|
||||
type: string;
|
||||
description?: string;
|
||||
}
|
||||
|
||||
interface WebSocketContextValue {
|
||||
isConnected: boolean;
|
||||
socket: Socket | null;
|
||||
connectionError: ConnectionError | null;
|
||||
}
|
||||
|
||||
interface WebSocketProviderProps {
|
||||
@@ -76,11 +83,12 @@ export function WebSocketProvider({
|
||||
if (onEventDeleted) callbacks.onEventDeleted = onEventDeleted;
|
||||
if (onProjectUpdated) callbacks.onProjectUpdated = onProjectUpdated;
|
||||
|
||||
const { isConnected, socket } = useWebSocket(workspaceId, token, callbacks);
|
||||
const { isConnected, socket, connectionError } = useWebSocket(workspaceId, token, callbacks);
|
||||
|
||||
const value: WebSocketContextValue = {
|
||||
isConnected,
|
||||
socket,
|
||||
connectionError,
|
||||
};
|
||||
|
||||
return <WebSocketContext.Provider value={value}>{children}</WebSocketContext.Provider>;
|
||||
|
||||
Reference in New Issue
Block a user