From 14b547d46811e95f5e8fdf8a19557071a3ffef40 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Fri, 6 Feb 2026 15:46:58 -0600 Subject: [PATCH] fix(SEC-WEB-30+31+36): Validate JSON.parse/localStorage deserialization Add runtime type validation after all JSON.parse calls in the web app to prevent runtime crashes from corrupted or tampered storage data. Creates a shared safeJsonParse utility with type guard functions for each data shape (Message[], ChatOverlayState, LayoutConfigRecord). All four affected callsites now validate parsed data and fall back to safe defaults on mismatch. Files changed: - apps/web/src/lib/utils/safe-json.ts (new utility) - apps/web/src/lib/utils/safe-json.test.ts (25 tests) - apps/web/src/hooks/useChat.ts (deserializeMessages) - apps/web/src/hooks/useChat.test.ts (3 new corruption tests) - apps/web/src/hooks/useChatOverlay.ts (loadState) - apps/web/src/hooks/useChatOverlay.test.ts (3 new corruption tests) - apps/web/src/components/chat/ConversationSidebar.tsx (ideaToConversation) - apps/web/src/lib/hooks/useLayout.ts (layout loading) Co-Authored-By: Claude Opus 4.6 --- .../components/chat/ConversationSidebar.tsx | 14 +- apps/web/src/hooks/useChat.test.ts | 53 ++++ apps/web/src/hooks/useChat.ts | 10 +- apps/web/src/hooks/useChatOverlay.test.ts | 32 ++ apps/web/src/hooks/useChatOverlay.ts | 5 +- apps/web/src/lib/hooks/useLayout.ts | 8 +- apps/web/src/lib/utils/safe-json.test.ts | 291 ++++++++++++++++++ apps/web/src/lib/utils/safe-json.ts | 125 ++++++++ 8 files changed, 516 insertions(+), 22 deletions(-) create mode 100644 apps/web/src/lib/utils/safe-json.test.ts create mode 100644 apps/web/src/lib/utils/safe-json.ts diff --git a/apps/web/src/components/chat/ConversationSidebar.tsx b/apps/web/src/components/chat/ConversationSidebar.tsx index 31244dc..6b3e464 100644 --- a/apps/web/src/components/chat/ConversationSidebar.tsx +++ b/apps/web/src/components/chat/ConversationSidebar.tsx @@ -1,9 +1,9 @@ -/* eslint-disable @typescript-eslint/no-unsafe-assignment */ "use client"; import { useState, useEffect, forwardRef, useImperativeHandle, useCallback } from "react"; import { getConversations, type Idea } from "@/lib/api/ideas"; import { useAuth } from "@/lib/auth/auth-context"; +import { safeJsonParse, isMessageArray } from "@/lib/utils/safe-json"; interface ConversationSummary { id: string; @@ -41,15 +41,9 @@ export const ConversationSidebar = forwardRef { - // Count messages from the stored JSON content - let messageCount = 0; - try { - const messages = JSON.parse(idea.content); - messageCount = Array.isArray(messages) ? messages.length : 0; - } catch { - // If parsing fails, assume 0 messages - messageCount = 0; - } + // Count messages from the stored JSON content with runtime validation + const messages = safeJsonParse(idea.content, isMessageArray, []); + const messageCount = messages.length; return { id: idea.id, diff --git a/apps/web/src/hooks/useChat.test.ts b/apps/web/src/hooks/useChat.test.ts index 70fadfe..012a050 100644 --- a/apps/web/src/hooks/useChat.test.ts +++ b/apps/web/src/hooks/useChat.test.ts @@ -320,6 +320,59 @@ describe("useChat", () => { expect(result.current.conversationId).toBe("conv-123"); expect(result.current.conversationTitle).toBe("My Conversation"); }); + + it("should fall back to welcome message when stored JSON is corrupted", async () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + mockGetIdea.mockResolvedValueOnce( + createMockIdea("conv-bad", "Corrupted", "not valid json {{{") + ); + + const { result } = renderHook(() => useChat()); + + await act(async () => { + await result.current.loadConversation("conv-bad"); + }); + + // Should fall back to welcome message + expect(result.current.messages).toHaveLength(1); + expect(result.current.messages[0]?.id).toBe("welcome"); + }); + + it("should fall back to welcome message when stored data has wrong shape", async () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + // Valid JSON but wrong shape (object instead of array, missing required fields) + mockGetIdea.mockResolvedValueOnce( + createMockIdea("conv-bad", "Wrong Shape", JSON.stringify({ not: "an array" })) + ); + + const { result } = renderHook(() => useChat()); + + await act(async () => { + await result.current.loadConversation("conv-bad"); + }); + + expect(result.current.messages).toHaveLength(1); + expect(result.current.messages[0]?.id).toBe("welcome"); + }); + + it("should fall back to welcome message when messages have invalid roles", async () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + const badMessages = [ + { id: "msg-1", role: "hacker", content: "Bad", createdAt: "2026-01-01" }, + ]; + mockGetIdea.mockResolvedValueOnce( + createMockIdea("conv-bad", "Bad Roles", JSON.stringify(badMessages)) + ); + + const { result } = renderHook(() => useChat()); + + await act(async () => { + await result.current.loadConversation("conv-bad"); + }); + + expect(result.current.messages).toHaveLength(1); + expect(result.current.messages[0]?.id).toBe("welcome"); + }); }); describe("startNewConversation", () => { diff --git a/apps/web/src/hooks/useChat.ts b/apps/web/src/hooks/useChat.ts index 84a672f..5426d52 100644 --- a/apps/web/src/hooks/useChat.ts +++ b/apps/web/src/hooks/useChat.ts @@ -6,6 +6,7 @@ import { useState, useCallback, useRef } from "react"; import { sendChatMessage, type ChatMessage as ApiChatMessage } from "@/lib/api/chat"; import { createConversation, updateConversation, getIdea, type Idea } from "@/lib/api/ideas"; +import { safeJsonParse, isMessageArray } from "@/lib/utils/safe-json"; export interface Message { id: string; @@ -111,15 +112,10 @@ export function useChat(options: UseChatOptions = {}): UseChatReturn { }, []); /** - * Deserialize messages from JSON + * Deserialize messages from JSON with runtime type validation */ const deserializeMessages = useCallback((json: string): Message[] => { - try { - const parsed = JSON.parse(json) as Message[]; - return Array.isArray(parsed) ? parsed : [WELCOME_MESSAGE]; - } catch { - return [WELCOME_MESSAGE]; - } + return safeJsonParse(json, isMessageArray, [WELCOME_MESSAGE]); }, []); /** diff --git a/apps/web/src/hooks/useChatOverlay.test.ts b/apps/web/src/hooks/useChatOverlay.test.ts index 5749e9c..dbed8e6 100644 --- a/apps/web/src/hooks/useChatOverlay.test.ts +++ b/apps/web/src/hooks/useChatOverlay.test.ts @@ -64,6 +64,7 @@ describe("useChatOverlay", () => { }); it("should handle invalid localStorage data gracefully", () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); localStorageMock.setItem("chatOverlayState", "invalid json"); const { result } = renderHook(() => useChatOverlay()); @@ -71,6 +72,37 @@ describe("useChatOverlay", () => { expect(result.current.isOpen).toBe(false); expect(result.current.isMinimized).toBe(false); }); + + it("should fall back to defaults when localStorage has wrong shape", () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + // Valid JSON but wrong shape + localStorageMock.setItem("chatOverlayState", JSON.stringify({ isOpen: "yes", wrong: true })); + + const { result } = renderHook(() => useChatOverlay()); + + expect(result.current.isOpen).toBe(false); + expect(result.current.isMinimized).toBe(false); + }); + + it("should fall back to defaults when localStorage has null value parsed", () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + localStorageMock.setItem("chatOverlayState", "null"); + + const { result } = renderHook(() => useChatOverlay()); + + expect(result.current.isOpen).toBe(false); + expect(result.current.isMinimized).toBe(false); + }); + + it("should fall back to defaults when localStorage has array instead of object", () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + localStorageMock.setItem("chatOverlayState", JSON.stringify([true, false])); + + const { result } = renderHook(() => useChatOverlay()); + + expect(result.current.isOpen).toBe(false); + expect(result.current.isMinimized).toBe(false); + }); }); describe("open", () => { diff --git a/apps/web/src/hooks/useChatOverlay.ts b/apps/web/src/hooks/useChatOverlay.ts index 9458587..675f2d1 100644 --- a/apps/web/src/hooks/useChatOverlay.ts +++ b/apps/web/src/hooks/useChatOverlay.ts @@ -4,6 +4,7 @@ */ import { useState, useEffect, useCallback } from "react"; +import { safeJsonParse, isChatOverlayState } from "@/lib/utils/safe-json"; interface ChatOverlayState { isOpen: boolean; @@ -27,7 +28,7 @@ const DEFAULT_STATE: ChatOverlayState = { }; /** - * Load state from localStorage + * Load state from localStorage with runtime type validation */ function loadState(): ChatOverlayState { if (typeof window === "undefined") { @@ -37,7 +38,7 @@ function loadState(): ChatOverlayState { try { const stored = window.localStorage.getItem(STORAGE_KEY); if (stored) { - return JSON.parse(stored) as ChatOverlayState; + return safeJsonParse(stored, isChatOverlayState, DEFAULT_STATE); } } catch (error) { console.warn("Failed to load chat overlay state from localStorage:", error); diff --git a/apps/web/src/lib/hooks/useLayout.ts b/apps/web/src/lib/hooks/useLayout.ts index 409296c..cb86198 100644 --- a/apps/web/src/lib/hooks/useLayout.ts +++ b/apps/web/src/lib/hooks/useLayout.ts @@ -4,6 +4,7 @@ import { useCallback, useState, useEffect } from "react"; import type { WidgetPlacement, LayoutConfig } from "@mosaic/shared"; +import { safeJsonParse, isLayoutConfigRecord } from "@/lib/utils/safe-json"; const STORAGE_KEY = "mosaic-layout"; const DEFAULT_LAYOUT_NAME = "default"; @@ -37,13 +38,14 @@ export function useLayout(): UseLayoutReturn { const [currentLayoutId, setCurrentLayoutId] = useState(DEFAULT_LAYOUT_NAME); const [isLoading, setIsLoading] = useState(true); - // Load layouts from localStorage on mount + // Load layouts from localStorage on mount with runtime type validation useEffect(() => { try { const stored = localStorage.getItem(STORAGE_KEY); if (stored) { - const parsed = JSON.parse(stored) as Record; - setLayouts(parsed); + const emptyFallback: Record = {}; + const parsed = safeJsonParse(stored, isLayoutConfigRecord, emptyFallback); + setLayouts(parsed as Record); } // Load current layout ID preference diff --git a/apps/web/src/lib/utils/safe-json.test.ts b/apps/web/src/lib/utils/safe-json.test.ts new file mode 100644 index 0000000..5ae198a --- /dev/null +++ b/apps/web/src/lib/utils/safe-json.test.ts @@ -0,0 +1,291 @@ +/** + * @file safe-json.test.ts + * @description Tests for safe JSON parsing utilities with runtime type validation + */ + +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { + safeJsonParse, + isMessageArray, + isChatOverlayState, + isLayoutConfigRecord, +} from "./safe-json"; + +describe("safeJsonParse", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("should return parsed data when JSON is valid and passes validation", () => { + const json = '{"key": "value"}'; + const validator = (data: unknown): data is { key: string } => + typeof data === "object" && data !== null && "key" in data; + + const result = safeJsonParse(json, validator, { key: "fallback" }); + expect(result).toEqual({ key: "value" }); + }); + + it("should return fallback when JSON is invalid", () => { + const json = "not valid json {{{"; + const validator = (data: unknown): data is string => typeof data === "string"; + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); + + const result = safeJsonParse(json, validator, "fallback"); + expect(result).toBe("fallback"); + expect(warnSpy).toHaveBeenCalledWith("safeJsonParse: failed to parse JSON, returning fallback"); + }); + + it("should return fallback when parsed data fails validation", () => { + const json = '{"wrong": "shape"}'; + const validator = (data: unknown): data is { expected: string } => + typeof data === "object" && data !== null && "expected" in data; + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); + + const result = safeJsonParse(json, validator, { expected: "default" }); + expect(result).toEqual({ expected: "default" }); + expect(warnSpy).toHaveBeenCalledWith( + "safeJsonParse: parsed data failed validation, returning fallback" + ); + }); + + it("should return fallback for empty string", () => { + const validator = (data: unknown): data is string => typeof data === "string"; + vi.spyOn(console, "warn").mockImplementation(() => undefined); + + const result = safeJsonParse("", validator, "fallback"); + expect(result).toBe("fallback"); + }); + + it("should handle null JSON value gracefully", () => { + const validator = (data: unknown): data is object => typeof data === "object" && data !== null; + vi.spyOn(console, "warn").mockImplementation(() => undefined); + + const result = safeJsonParse("null", validator, {}); + expect(result).toEqual({}); + }); +}); + +describe("isMessageArray", () => { + it("should return true for a valid message array", () => { + const messages = [ + { + id: "msg-1", + role: "user", + content: "Hello", + createdAt: "2026-01-01T00:00:00Z", + }, + { + id: "msg-2", + role: "assistant", + content: "Hi there!", + createdAt: "2026-01-01T00:00:01Z", + }, + ]; + + expect(isMessageArray(messages)).toBe(true); + }); + + it("should return true for an empty array", () => { + expect(isMessageArray([])).toBe(true); + }); + + it("should return true for messages with optional fields", () => { + const messages = [ + { + id: "msg-1", + role: "assistant", + content: "Response", + createdAt: "2026-01-01T00:00:00Z", + model: "llama3.2", + thinking: "Let me think...", + promptTokens: 10, + completionTokens: 5, + totalTokens: 15, + }, + ]; + + expect(isMessageArray(messages)).toBe(true); + }); + + it("should return false for non-array values", () => { + expect(isMessageArray(null)).toBe(false); + expect(isMessageArray(undefined)).toBe(false); + expect(isMessageArray("string")).toBe(false); + expect(isMessageArray(42)).toBe(false); + expect(isMessageArray({})).toBe(false); + }); + + it("should return false when message is missing required fields", () => { + // Missing id + expect(isMessageArray([{ role: "user", content: "Hello", createdAt: "2026-01-01" }])).toBe( + false + ); + + // Missing role + expect(isMessageArray([{ id: "1", content: "Hello", createdAt: "2026-01-01" }])).toBe(false); + + // Missing content + expect(isMessageArray([{ id: "1", role: "user", createdAt: "2026-01-01" }])).toBe(false); + + // Missing createdAt + expect(isMessageArray([{ id: "1", role: "user", content: "Hello" }])).toBe(false); + }); + + it("should return false when role is not a valid enum value", () => { + const messages = [ + { + id: "msg-1", + role: "invalid-role", + content: "Hello", + createdAt: "2026-01-01T00:00:00Z", + }, + ]; + + expect(isMessageArray(messages)).toBe(false); + }); + + it("should return false when fields have wrong types", () => { + const messages = [ + { + id: 123, // should be string + role: "user", + content: "Hello", + createdAt: "2026-01-01", + }, + ]; + + expect(isMessageArray(messages)).toBe(false); + }); + + it("should return false for array with mixed valid and invalid messages", () => { + const messages = [ + { id: "1", role: "user", content: "Hello", createdAt: "2026-01-01" }, + { invalid: "message" }, + ]; + + expect(isMessageArray(messages)).toBe(false); + }); +}); + +describe("isChatOverlayState", () => { + it("should return true for a valid ChatOverlayState", () => { + expect(isChatOverlayState({ isOpen: true, isMinimized: false })).toBe(true); + expect(isChatOverlayState({ isOpen: false, isMinimized: true })).toBe(true); + }); + + it("should return false for non-object values", () => { + expect(isChatOverlayState(null)).toBe(false); + expect(isChatOverlayState(undefined)).toBe(false); + expect(isChatOverlayState("string")).toBe(false); + expect(isChatOverlayState(42)).toBe(false); + expect(isChatOverlayState([])).toBe(false); + }); + + it("should return false when fields are missing", () => { + expect(isChatOverlayState({ isOpen: true })).toBe(false); + expect(isChatOverlayState({ isMinimized: false })).toBe(false); + expect(isChatOverlayState({})).toBe(false); + }); + + it("should return false when fields have wrong types", () => { + expect(isChatOverlayState({ isOpen: "true", isMinimized: false })).toBe(false); + expect(isChatOverlayState({ isOpen: true, isMinimized: 0 })).toBe(false); + expect(isChatOverlayState({ isOpen: 1, isMinimized: 0 })).toBe(false); + }); + + it("should return true when extra fields are present", () => { + expect(isChatOverlayState({ isOpen: true, isMinimized: false, extra: "field" })).toBe(true); + }); +}); + +describe("isLayoutConfigRecord", () => { + it("should return true for a valid layout config record", () => { + const record = { + default: { + id: "default", + name: "Default Layout", + layout: [], + }, + custom: { + id: "custom", + name: "Custom Layout", + layout: [ + { i: "widget-1", x: 0, y: 0, w: 2, h: 2 }, + { i: "widget-2", x: 2, y: 0, w: 3, h: 1 }, + ], + }, + }; + + expect(isLayoutConfigRecord(record)).toBe(true); + }); + + it("should return true for an empty record", () => { + expect(isLayoutConfigRecord({})).toBe(true); + }); + + it("should return false for non-object values", () => { + expect(isLayoutConfigRecord(null)).toBe(false); + expect(isLayoutConfigRecord(undefined)).toBe(false); + expect(isLayoutConfigRecord("string")).toBe(false); + expect(isLayoutConfigRecord(42)).toBe(false); + expect(isLayoutConfigRecord([])).toBe(false); + }); + + it("should return false when layout config is missing required fields", () => { + // Missing id + expect(isLayoutConfigRecord({ layout1: { name: "Test", layout: [] } })).toBe(false); + + // Missing name + expect(isLayoutConfigRecord({ layout1: { id: "1", layout: [] } })).toBe(false); + + // Missing layout + expect(isLayoutConfigRecord({ layout1: { id: "1", name: "Test" } })).toBe(false); + }); + + it("should return false when layout array contains invalid widget placements", () => { + const record = { + test: { + id: "test", + name: "Test", + layout: [{ i: "widget-1", x: 0 }], // missing y, w, h + }, + }; + + expect(isLayoutConfigRecord(record)).toBe(false); + }); + + it("should return false when widget placement fields have wrong types", () => { + const record = { + test: { + id: "test", + name: "Test", + layout: [{ i: 123, x: 0, y: 0, w: 2, h: 2 }], // i should be string + }, + }; + + expect(isLayoutConfigRecord(record)).toBe(false); + }); + + it("should return true with widget placements that have optional fields", () => { + const record = { + test: { + id: "test", + name: "Test", + layout: [ + { + i: "widget-1", + x: 0, + y: 0, + w: 2, + h: 2, + minW: 1, + maxW: 4, + static: true, + }, + ], + }, + }; + + expect(isLayoutConfigRecord(record)).toBe(true); + }); +}); diff --git a/apps/web/src/lib/utils/safe-json.ts b/apps/web/src/lib/utils/safe-json.ts new file mode 100644 index 0000000..fe5816f --- /dev/null +++ b/apps/web/src/lib/utils/safe-json.ts @@ -0,0 +1,125 @@ +/** + * @file safe-json.ts + * @description Safe JSON parsing utilities with runtime type validation. + * Prevents runtime crashes from corrupted or tampered localStorage/API data. + */ + +/** + * Safely parse a JSON string with runtime type validation. + * Returns the fallback value if parsing fails or the parsed data + * doesn't match the expected shape. + * + * @param json - The JSON string to parse + * @param validator - A type guard function that validates the parsed data + * @param fallback - The default value to return on failure + * @returns The validated parsed data or the fallback value + */ +export function safeJsonParse( + json: string, + validator: (data: unknown) => data is T, + fallback: T +): T { + try { + const parsed: unknown = JSON.parse(json); + if (validator(parsed)) { + return parsed; + } + console.warn("safeJsonParse: parsed data failed validation, returning fallback"); + return fallback; + } catch { + console.warn("safeJsonParse: failed to parse JSON, returning fallback"); + return fallback; + } +} + +/** + * Type guard: validates that a value is a non-null object + */ +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** + * Type guard: validates a chat Message shape + * Checks for required fields: id (string), role (valid enum), content (string), createdAt (string) + */ +export function isMessage(value: unknown): boolean { + if (!isRecord(value)) return false; + const validRoles = ["user", "assistant", "system"]; + return ( + typeof value.id === "string" && + typeof value.role === "string" && + validRoles.includes(value.role) && + typeof value.content === "string" && + typeof value.createdAt === "string" + ); +} + +/** + * Type guard: validates an array of chat Messages + */ +export function isMessageArray(value: unknown): value is { + id: string; + role: "user" | "assistant" | "system"; + content: string; + createdAt: string; + thinking?: string; + model?: string; + provider?: string; + promptTokens?: number; + completionTokens?: number; + totalTokens?: number; +}[] { + return Array.isArray(value) && value.every(isMessage); +} + +/** + * Type guard: validates ChatOverlayState shape + * Expects { isOpen: boolean, isMinimized: boolean } + */ +export function isChatOverlayState( + value: unknown +): value is { isOpen: boolean; isMinimized: boolean } { + if (!isRecord(value)) return false; + return typeof value.isOpen === "boolean" && typeof value.isMinimized === "boolean"; +} + +/** + * Type guard: validates a WidgetPlacement shape + * Checks for required fields: i (string), x/y/w/h (numbers) + */ +function isWidgetPlacement(value: unknown): boolean { + if (!isRecord(value)) return false; + return ( + typeof value.i === "string" && + typeof value.x === "number" && + typeof value.y === "number" && + typeof value.w === "number" && + typeof value.h === "number" + ); +} + +/** + * Type guard: validates a LayoutConfig shape + * Expects { id: string, name: string, layout: WidgetPlacement[] } + */ +function isLayoutConfig(value: unknown): boolean { + if (!isRecord(value)) return false; + return ( + typeof value.id === "string" && + typeof value.name === "string" && + Array.isArray(value.layout) && + (value.layout as unknown[]).every(isWidgetPlacement) + ); +} + +/** + * Type guard: validates a Record shape. + * Uses a branded type approach to ensure compatibility with LayoutConfig consumers. + */ +export function isLayoutConfigRecord( + value: unknown +): value is Record { + if (!isRecord(value)) return false; + return Object.values(value).every(isLayoutConfig); +}