fix(SEC-WEB-33+35): Fix Mermaid error display + useWorkspaceId error logging
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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(<MermaidViewer diagram={sensitiveSource} />);
|
||||||
|
|
||||||
|
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(<MermaidViewer diagram={diagram} />);
|
||||||
|
|
||||||
|
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(<MermaidViewer diagram={diagram} />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
// There should be no <pre> 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(<MermaidViewer diagram={diagram} />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(consoleErrorSpy).toHaveBeenCalledWith("Mermaid rendering failed:", originalError);
|
||||||
|
});
|
||||||
|
|
||||||
|
consoleErrorSpy.mockRestore();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("DOMPurify integration", () => {
|
describe("DOMPurify integration", () => {
|
||||||
it("should sanitize rendered SVG output", async () => {
|
it("should sanitize rendered SVG output", async () => {
|
||||||
const diagram = `graph TD
|
const diagram = `graph TD
|
||||||
|
|||||||
@@ -86,7 +86,9 @@ export function MermaidViewer({
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
} catch (err) {
|
} 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 {
|
} finally {
|
||||||
setIsLoading(false);
|
setIsLoading(false);
|
||||||
}
|
}
|
||||||
@@ -124,11 +126,8 @@ export function MermaidViewer({
|
|||||||
if (error) {
|
if (error) {
|
||||||
return (
|
return (
|
||||||
<div className={`flex flex-col items-center justify-center p-8 ${className}`}>
|
<div className={`flex flex-col items-center justify-center p-8 ${className}`}>
|
||||||
<div className="text-red-500 mb-2">Failed to render diagram</div>
|
<div className="text-red-500 mb-2">Diagram rendering failed</div>
|
||||||
<div className="text-sm text-gray-500">{error}</div>
|
<div className="text-sm text-gray-500">Please check the diagram syntax and try again.</div>
|
||||||
<pre className="mt-4 p-4 bg-gray-100 dark:bg-gray-800 rounded text-xs overflow-auto max-w-full">
|
|
||||||
{diagram}
|
|
||||||
</pre>
|
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -232,6 +232,11 @@ export function useWorkspaceId(): string | null {
|
|||||||
const stored = localStorage.getItem(WORKSPACE_KEY);
|
const stored = localStorage.getItem(WORKSPACE_KEY);
|
||||||
if (stored) {
|
if (stored) {
|
||||||
setWorkspaceId(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) {
|
} catch (error) {
|
||||||
console.error("Failed to load workspace ID from localStorage:", error);
|
console.error("Failed to load workspace ID from localStorage:", error);
|
||||||
|
|||||||
109
apps/web/src/lib/hooks/useWorkspaceId.test.ts
Normal file
109
apps/web/src/lib/hooks/useWorkspaceId.test.ts
Normal file
@@ -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<typeof vi.fn>;
|
||||||
|
setItem: ReturnType<typeof vi.fn>;
|
||||||
|
removeItem: ReturnType<typeof vi.fn>;
|
||||||
|
clear: ReturnType<typeof vi.fn>;
|
||||||
|
readonly length: number;
|
||||||
|
key: ReturnType<typeof vi.fn>;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Mock localStorage
|
||||||
|
const localStorageMock = ((): MockLocalStorage => {
|
||||||
|
let store: Record<string, string> = {};
|
||||||
|
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<typeof vi.spyOn>;
|
||||||
|
let consoleErrorSpy: ReturnType<typeof vi.spyOn>;
|
||||||
|
|
||||||
|
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();
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user