From 12fa093f5843d558aeddd82c319cb3a8a7c97174 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Fri, 6 Feb 2026 18:16:07 -0600 Subject: [PATCH] fix(SEC-WEB-33+35): Fix Mermaid error display + useWorkspaceId error logging SEC-WEB-33: Replace raw diagram source and detailed error messages in MermaidViewer error UI with a generic "Diagram rendering failed" message. Detailed errors are logged to console.error for debugging only. SEC-WEB-35: Add console.warn in useWorkspaceId when no workspace ID is found in localStorage, making it easier to distinguish "no workspace selected" from silent hook failure. Co-Authored-By: Claude Opus 4.6 --- .../components/mindmap/MermaidViewer.test.tsx | 78 +++++++++++++ .../src/components/mindmap/MermaidViewer.tsx | 11 +- apps/web/src/lib/hooks/useLayout.ts | 5 + apps/web/src/lib/hooks/useWorkspaceId.test.ts | 109 ++++++++++++++++++ 4 files changed, 197 insertions(+), 6 deletions(-) create mode 100644 apps/web/src/lib/hooks/useWorkspaceId.test.ts diff --git a/apps/web/src/components/mindmap/MermaidViewer.test.tsx b/apps/web/src/components/mindmap/MermaidViewer.test.tsx index 20f2d78..c4762bd 100644 --- a/apps/web/src/components/mindmap/MermaidViewer.test.tsx +++ b/apps/web/src/components/mindmap/MermaidViewer.test.tsx @@ -209,6 +209,84 @@ describe("MermaidViewer XSS Protection", () => { }); }); + describe("Error display (SEC-WEB-33)", () => { + it("should not display raw diagram source when rendering fails", async () => { + const sensitiveSource = `graph TD + A["SECRET_API_KEY=abc123"] + B["password: hunter2"]`; + + // Mock mermaid to throw an error containing the diagram source + const mermaid = await import("mermaid"); + vi.mocked(mermaid.default.render).mockRejectedValue( + new Error(`Parse error in diagram: ${sensitiveSource}`) + ); + + const { container } = render(); + + await waitFor(() => { + const content = container.innerHTML; + // Should show generic error message, not raw source or detailed error + expect(content).toContain("Diagram rendering failed"); + expect(content).not.toContain("SECRET_API_KEY"); + expect(content).not.toContain("password: hunter2"); + expect(content).not.toContain("Parse error in diagram"); + }); + }); + + it("should not expose detailed error messages in the UI", async () => { + const diagram = `graph TD + A["Test"]`; + + const mermaid = await import("mermaid"); + vi.mocked(mermaid.default.render).mockRejectedValue( + new Error("Lexical error on line 2. Unrecognized text at /internal/path/file.ts") + ); + + const { container } = render(); + + await waitFor(() => { + const content = container.innerHTML; + expect(content).toContain("Diagram rendering failed"); + expect(content).not.toContain("Lexical error"); + expect(content).not.toContain("/internal/path/file.ts"); + }); + }); + + it("should not display a pre tag with raw diagram source on error", async () => { + const diagram = `graph TD + A["Node A"]`; + + const mermaid = await import("mermaid"); + vi.mocked(mermaid.default.render).mockRejectedValue(new Error("render failed")); + + const { container } = render(); + + await waitFor(() => { + // There should be no 
 element showing raw diagram source
+        const preElements = container.querySelectorAll("pre");
+        expect(preElements.length).toBe(0);
+      });
+    });
+
+    it("should log the detailed error to console.error", async () => {
+      const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
+      const diagram = `graph TD
+        A["Test"]`;
+      const originalError = new Error("Detailed parse error at line 2");
+
+      const mermaid = await import("mermaid");
+      vi.mocked(mermaid.default.render).mockRejectedValue(originalError);
+
+      render();
+
+      await waitFor(() => {
+        expect(consoleErrorSpy).toHaveBeenCalledWith("Mermaid rendering failed:", originalError);
+      });
+
+      consoleErrorSpy.mockRestore();
+    });
+  });
+
   describe("DOMPurify integration", () => {
     it("should sanitize rendered SVG output", async () => {
       const diagram = `graph TD
diff --git a/apps/web/src/components/mindmap/MermaidViewer.tsx b/apps/web/src/components/mindmap/MermaidViewer.tsx
index efba554..73037ef 100644
--- a/apps/web/src/components/mindmap/MermaidViewer.tsx
+++ b/apps/web/src/components/mindmap/MermaidViewer.tsx
@@ -86,7 +86,9 @@ export function MermaidViewer({
         }
       }
     } catch (err) {
-      setError(err instanceof Error ? err.message : "Failed to render diagram");
+      // Log detailed error for debugging but don't expose raw source/messages to the UI
+      console.error("Mermaid rendering failed:", err);
+      setError("Diagram rendering failed. Please check the diagram syntax and try again.");
     } finally {
       setIsLoading(false);
     }
@@ -124,11 +126,8 @@ export function MermaidViewer({
   if (error) {
     return (
       

-        
Failed to render diagram

-        
{error}

-        
-          {diagram}
-        

+        
Diagram rendering failed

+        
Please check the diagram syntax and try again.

       

     );
   }
diff --git a/apps/web/src/lib/hooks/useLayout.ts b/apps/web/src/lib/hooks/useLayout.ts
index cb86198..ef1d903 100644
--- a/apps/web/src/lib/hooks/useLayout.ts
+++ b/apps/web/src/lib/hooks/useLayout.ts
@@ -232,6 +232,11 @@ export function useWorkspaceId(): string | null {
       const stored = localStorage.getItem(WORKSPACE_KEY);
       if (stored) {
         setWorkspaceId(stored);
+      } else {
+        console.warn(
+          `useWorkspaceId: No workspace ID found in localStorage (key: "${WORKSPACE_KEY}"). ` +
+            "This may indicate no workspace has been selected yet."
+        );
       }
     } catch (error) {
       console.error("Failed to load workspace ID from localStorage:", error);
diff --git a/apps/web/src/lib/hooks/useWorkspaceId.test.ts b/apps/web/src/lib/hooks/useWorkspaceId.test.ts
new file mode 100644
index 0000000..5c0fed8
--- /dev/null
+++ b/apps/web/src/lib/hooks/useWorkspaceId.test.ts
@@ -0,0 +1,109 @@
+/**
+ * useWorkspaceId Hook Tests
+ * Tests for SEC-WEB-35: warning logging when workspace ID is not found
+ */
+
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import { renderHook } from "@testing-library/react";
+import { useWorkspaceId } from "./useLayout";
+
+interface MockLocalStorage {
+  getItem: ReturnType;
+  setItem: ReturnType;
+  removeItem: ReturnType;
+  clear: ReturnType;
+  readonly length: number;
+  key: ReturnType;
+}
+
+// Mock localStorage
+const localStorageMock = ((): MockLocalStorage => {
+  let store: Record = {};
+  return {
+    getItem: vi.fn((key: string): string | null => store[key] ?? null),
+    setItem: vi.fn((key: string, value: string): void => {
+      store[key] = value;
+    }),
+    removeItem: vi.fn((key: string): void => {
+      store = Object.fromEntries(Object.entries(store).filter(([k]) => k !== key));
+    }),
+    clear: vi.fn((): void => {
+      store = {};
+    }),
+    get length(): number {
+      return Object.keys(store).length;
+    },
+    key: vi.fn((_index: number): string | null => null),
+  };
+})();
+
+Object.defineProperty(window, "localStorage", {
+  value: localStorageMock,
+  writable: true,
+});
+
+describe("useWorkspaceId", (): void => {
+  let consoleWarnSpy: ReturnType;
+  let consoleErrorSpy: ReturnType;
+
+  beforeEach((): void => {
+    vi.clearAllMocks();
+    localStorageMock.clear();
+    consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined);
+    consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
+  });
+
+  afterEach((): void => {
+    consoleWarnSpy.mockRestore();
+    consoleErrorSpy.mockRestore();
+    vi.resetAllMocks();
+  });
+
+  it("should return workspace ID when stored in localStorage", (): void => {
+    localStorageMock.setItem("mosaic-workspace-id", "ws-123");
+
+    const { result } = renderHook(() => useWorkspaceId());
+
+    expect(result.current).toBe("ws-123");
+    expect(consoleWarnSpy).not.toHaveBeenCalled();
+  });
+
+  it("should return null and log warning when no workspace ID in localStorage", (): void => {
+    const { result } = renderHook(() => useWorkspaceId());
+
+    expect(result.current).toBeNull();
+    expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
+    expect(consoleWarnSpy).toHaveBeenCalledWith(
+      expect.stringContaining("No workspace ID found in localStorage")
+    );
+  });
+
+  it("should include the storage key in the warning message", (): void => {
+    renderHook(() => useWorkspaceId());
+
+    expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("mosaic-workspace-id"));
+  });
+
+  it("should log console.error when localStorage throws", (): void => {
+    localStorageMock.getItem.mockImplementation(() => {
+      throw new Error("localStorage is disabled");
+    });
+
+    const { result } = renderHook(() => useWorkspaceId());
+
+    expect(result.current).toBeNull();
+    expect(consoleErrorSpy).toHaveBeenCalledWith(
+      "Failed to load workspace ID from localStorage:",
+      expect.any(Error)
+    );
+  });
+
+  it("should not log warning when workspace ID exists", (): void => {
+    localStorageMock.setItem("mosaic-workspace-id", "ws-abc-def");
+
+    renderHook(() => useWorkspaceId());
+
+    expect(consoleWarnSpy).not.toHaveBeenCalled();
+    expect(consoleErrorSpy).not.toHaveBeenCalled();
+  });
+});