Skip to content

Commit 0010c9a

Browse files
sw-factory-automationsona-agent
sw-factory-automations
and
ona-agent
authored
fix: guard ThemeProvider localStorage access against SecurityError (#1178)
* fix: guard ThemeProvider localStorage access against SecurityError Wrap localStorage.getItem and localStorage.setItem in try-catch in theme.tsx to prevent crashes when storage access is denied (restricted browsers, privacy settings, embedded contexts). Falls back to dark theme on read, silently ignores on write. The inline theme script in layout.tsx already had this protection, but ThemeProvider did not — causing a SecurityError that broke the entire React render tree for affected users. Co-authored-by: Ona <no-reply@ona.com> * chore: re-trigger CI (Vercel status stuck pending) Co-authored-by: Ona <no-reply@ona.com> --------- Co-authored-by: Ona <no-reply@ona.com>
1 parent 788619a commit 0010c9a

4 files changed

Lines changed: 106 additions & 5 deletions

File tree

‎.agents/conventions.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,7 @@ Theme is managed by `src/lib/theme.tsx` which provides `ThemeProvider` and `useT
596596
- **Flash prevention:** Inline `<script>` in `<head>` reads localStorage before React hydrates.
597597
- **System detection:** `prefers-color-scheme` media query listener when preference is `"system"`.
598598
- **Default:** `"dark"` (existing users who haven't set a preference get dark mode).
599+
- **localStorage safety:** All `localStorage` calls must be wrapped in try-catch. Browsers may throw `SecurityError` when storage access is denied (privacy settings, embedded contexts, enterprise policies). Fall back to `"dark"` on read, silently ignore on write.
599600

600601
```typescript
601602
// Reading theme in a client component

‎src/lib/sentry/sentry.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ const BARE_CATCH_PERMANENT_ALLOWLIST = new Set([
2020
"src/components/members/invite-form.tsx", // clipboard writeText — intentionally silent on permission denied
2121
"src/components/members/pending-invite-list.tsx", // clipboard writeText — intentionally silent on permission denied
2222
"src/lib/use-persisted-expanded.ts", // localStorage read/write — intentionally silent in private browsing / SSR
23+
"src/lib/theme.tsx", // localStorage read/write — intentionally silent when storage access is denied (SecurityError)
2324
"src/components/editor/demo-editor.tsx", // URL validation (new URL() throws) — same pattern as editor.tsx
2425
"src/components/editor/local-persistence-plugin.tsx", // sessionStorage read/write — intentionally silent in private browsing
2526
]);

‎src/lib/theme.test.tsx‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
2+
import { renderHook, act } from "@testing-library/react";
3+
import type { ReactNode } from "react";
4+
import { ThemeProvider, useTheme } from "./theme";
5+
6+
function wrapper({ children }: { children: ReactNode }) {
7+
return <ThemeProvider>{children}</ThemeProvider>;
8+
}
9+
10+
describe("ThemeProvider", () => {
11+
beforeEach(() => {
12+
localStorage.clear();
13+
document.documentElement.removeAttribute("data-theme");
14+
document.documentElement.classList.remove("dark");
15+
});
16+
17+
it("defaults to dark when localStorage is empty", () => {
18+
const { result } = renderHook(() => useTheme(), { wrapper });
19+
expect(result.current.preference).toBe("dark");
20+
expect(result.current.resolved).toBe("dark");
21+
});
22+
23+
it("reads stored preference from localStorage", () => {
24+
localStorage.setItem("memo-theme", "light");
25+
const { result } = renderHook(() => useTheme(), { wrapper });
26+
expect(result.current.preference).toBe("light");
27+
expect(result.current.resolved).toBe("light");
28+
});
29+
30+
it("ignores invalid stored values", () => {
31+
localStorage.setItem("memo-theme", "invalid-value");
32+
const { result } = renderHook(() => useTheme(), { wrapper });
33+
expect(result.current.preference).toBe("dark");
34+
});
35+
36+
it("persists preference changes to localStorage", () => {
37+
const { result } = renderHook(() => useTheme(), { wrapper });
38+
act(() => result.current.setPreference("light"));
39+
expect(localStorage.getItem("memo-theme")).toBe("light");
40+
expect(result.current.preference).toBe("light");
41+
});
42+
43+
/**
44+
* Regression test for Sentry MEMO-2H: SecurityError when localStorage is
45+
* blocked (restricted browsers, privacy settings, embedded contexts).
46+
* ThemeProvider must fall back to dark theme instead of crashing.
47+
*/
48+
describe("localStorage SecurityError handling", () => {
49+
let getItemSpy: ReturnType<typeof vi.spyOn>;
50+
let setItemSpy: ReturnType<typeof vi.spyOn>;
51+
52+
beforeEach(() => {
53+
getItemSpy = vi.spyOn(Storage.prototype, "getItem").mockImplementation(
54+
() => {
55+
throw new DOMException(
56+
"Failed to read the 'localStorage' property from 'Window': Access is denied for this document.",
57+
"SecurityError",
58+
);
59+
},
60+
);
61+
setItemSpy = vi.spyOn(Storage.prototype, "setItem").mockImplementation(
62+
() => {
63+
throw new DOMException(
64+
"Failed to set the 'localStorage' property on 'Window': Access is denied for this document.",
65+
"SecurityError",
66+
);
67+
},
68+
);
69+
});
70+
71+
afterEach(() => {
72+
getItemSpy.mockRestore();
73+
setItemSpy.mockRestore();
74+
});
75+
76+
it("falls back to dark theme when getItem throws SecurityError", () => {
77+
const { result } = renderHook(() => useTheme(), { wrapper });
78+
expect(result.current.preference).toBe("dark");
79+
expect(result.current.resolved).toBe("dark");
80+
});
81+
82+
it("does not crash when setItem throws SecurityError", () => {
83+
const { result } = renderHook(() => useTheme(), { wrapper });
84+
expect(() => {
85+
act(() => result.current.setPreference("light"));
86+
}).not.toThrow();
87+
expect(result.current.preference).toBe("light");
88+
expect(result.current.resolved).toBe("light");
89+
});
90+
});
91+
});

‎src/lib/theme.tsx‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,25 @@ function applyTheme(theme: ResolvedTheme) {
2828
export function ThemeProvider({ children }: { children: React.ReactNode }) {
2929
const [preference, setPreferenceState] = useState<ThemePreference>(() => {
3030
if (typeof window === "undefined") return "dark";
31-
const stored = localStorage.getItem(STORAGE_KEY);
32-
return stored === "light" || stored === "dark" || stored === "system"
33-
? stored
34-
: "dark";
31+
try {
32+
const stored = localStorage.getItem(STORAGE_KEY);
33+
return stored === "light" || stored === "dark" || stored === "system"
34+
? stored
35+
: "dark";
36+
} catch {
37+
return "dark";
38+
}
3539
});
3640
const [resolved, setResolved] = useState<ResolvedTheme>(() =>
3741
preference === "system" ? getSystemTheme() : preference,
3842
);
3943

4044
function setPreference(pref: ThemePreference) {
41-
localStorage.setItem(STORAGE_KEY, pref);
45+
try {
46+
localStorage.setItem(STORAGE_KEY, pref);
47+
} catch {
48+
// Storage unavailable (SecurityError in restricted browsers)
49+
}
4250
setPreferenceState(pref);
4351
const next = pref === "system" ? getSystemTheme() : pref;
4452
setResolved(next);

0 commit comments

Comments
 (0)