WIP! A BB-style forum, on the ATmosphere! We're still working... we'll be back soon when we have something to show off!
node typescript hono htmx atproto

fix: address PR review feedback on user theme preferences

Security:
- Add CSS injection guard to ThemeSwatchPreview (rejects values containing
; < }) matching the pattern already used in admin-themes.tsx

Error handling:
- Split network/JSON try blocks in preview, GET themes list, POST policy
fetch — SyntaxError from res.json() is a data error, not a code bug,
and must not be re-thrown via isProgrammingError
- Promote logger.warn → logger.error for themes list fetch failure
- Add logger.warn to preview endpoint catch block (was silently swallowing
AppView failures)

User-facing:
- Map ?error= codes to friendly messages; drop unknown codes (phishing
vector for crafted URLs showing raw internal codes like "invalid-theme")

Tests:
- Add getSetCookie absence assertions to allowUserChoice:false and
invalid-theme POST rejection tests
- Update ?error=invalid-theme GET test to verify friendly message in
settings-banner--error element
- Add tests for themes list non-ok response and network throw paths
- Add test for unknown ?error= code producing no banner

Docs:
- Align theme-resolution.ts internal section comments to use descriptive
headings instead of "Step N" (conflicted with JSDoc 5-step waterfall)
- CLAUDE.md: clarify settings routes bypass ThemeCache intentionally

+139 -37
+1
CLAUDE.md
··· 205 205 - User preference cookies are always validated against the current `policy.availableThemes` on read. Stale or removed theme URIs silently fall through to the forum default. 206 206 - `resolveTheme()` never throws -- it always returns a usable `ResolvedTheme`. 207 207 - Settings routes (`/settings`, `/settings/appearance`) require authentication. The preview endpoint (`/settings/preview`) is unauthenticated — it returns only public theme data (color swatches). The preference form is only rendered when `policy.allowUserChoice` is true. 208 + - **Settings routes bypass ThemeCache intentionally.** `GET /settings` and `POST /settings/appearance` call `fetch()` directly (not via `resolveTheme()`), so they always get fresh policy data. This is by design: preference saves must validate against the current policy, not a potentially stale cached copy. 208 209 209 210 ## Middleware Patterns 210 211
+6 -6
apps/web/src/lib/theme-resolution.ts
··· 108 108 ): Promise<ResolvedTheme> { 109 109 const colorScheme = detectColorScheme(cookieHeader, colorSchemeHint); 110 110 111 - // ── Step 1: Get theme policy (from cache or AppView) ─────────────────────── 111 + // ── Policy: fetch or restore from cache ──────────────────────────────────── 112 112 let policy: CachedPolicy | null = cache?.getPolicy() ?? null; 113 113 114 114 if (!policy) { ··· 145 145 } 146 146 } 147 147 148 - // ── Step 2: User preference or forum default URI ─────────────────────────── 148 + // ── URI resolution: user preference → forum default ─────────────────────── 149 149 const userPreferenceUri = resolveUserThemePreference( 150 150 cookieHeader, 151 151 colorScheme, ··· 170 170 // cid may be absent for live refs (e.g. canonical atbb.space presets) — that is expected 171 171 const expectedCid = matchingTheme?.cid ?? null; 172 172 173 - // ── Step 3: Get theme (from cache or AppView) ────────────────────────────── 173 + // ── Theme data: restore from cache if available ──────────────────────────── 174 174 const cachedTheme: CachedTheme | null = cache?.getTheme(defaultUri, colorScheme) ?? null; 175 175 176 176 if (cachedTheme !== null) { ··· 198 198 } 199 199 } 200 200 201 - // ── Step 4: Fetch theme from AppView ────────────────────────────────────── 201 + // ── Theme data: fetch from AppView ──────────────────────────────────────── 202 202 let themeRes: Response; 203 203 try { 204 204 themeRes = await fetch(`${appviewUrl}/api/themes/${rkey}`); ··· 221 221 return fallbackForScheme(colorScheme); 222 222 } 223 223 224 - // ── Step 5: Parse theme JSON ─────────────────────────────────────────────── 224 + // ── Theme data: parse response JSON ─────────────────────────────────────── 225 225 let theme: CachedTheme; 226 226 try { 227 227 theme = (await themeRes.json()) as CachedTheme; ··· 234 234 return fallbackForScheme(colorScheme); 235 235 } 236 236 237 - // ── Step 6: CID integrity check ──────────────────────────────────────────── 237 + // ── CID integrity check ──────────────────────────────────────────────────── 238 238 if (expectedCid && theme.cid !== expectedCid) { 239 239 logger.warn("Theme CID mismatch — using hardcoded fallback", { 240 240 operation: "resolveTheme",
+74 -2
apps/web/src/routes/__tests__/settings.test.tsx
··· 131 131 expect(html).not.toContain("<select"); 132 132 }); 133 133 134 + // ── GET /settings — Themes list fetch failure ──────────────────────── 135 + 136 + it("GET /settings renders form with empty selects when themes list returns non-ok", async () => { 137 + // Auth + policy succeed; themes fetch returns 500 138 + mockFetch.mockResolvedValueOnce( 139 + mockResponse({ authenticated: true, did: "did:plc:user", handle: "alice.bsky.social" }) 140 + ); 141 + mockFetch.mockResolvedValueOnce( 142 + mockResponse({ 143 + allowUserChoice: true, 144 + defaultLightThemeUri: null, 145 + defaultDarkThemeUri: null, 146 + availableThemes: [], 147 + }) 148 + ); 149 + mockFetch.mockResolvedValueOnce(mockResponse({}, false, 500)); // themes fetch fails 150 + const routes = await loadSettingsRoutes(); 151 + const res = await routes.request("/settings", { 152 + headers: { cookie: "atbb_session=token" }, 153 + }); 154 + expect(res.status).toBe(200); 155 + const html = await res.text(); 156 + // Form renders (no crash), but no theme options 157 + expect(html).toContain("id=\"lightThemeUri\""); 158 + expect(html).not.toContain("<option"); 159 + }); 160 + 161 + it("GET /settings renders form with empty selects when themes list fetch throws", async () => { 162 + // Auth + policy succeed; themes fetch throws network error 163 + mockFetch.mockResolvedValueOnce( 164 + mockResponse({ authenticated: true, did: "did:plc:user", handle: "alice.bsky.social" }) 165 + ); 166 + mockFetch.mockResolvedValueOnce( 167 + mockResponse({ 168 + allowUserChoice: true, 169 + defaultLightThemeUri: null, 170 + defaultDarkThemeUri: null, 171 + availableThemes: [], 172 + }) 173 + ); 174 + mockFetch.mockRejectedValueOnce(new Error("Network error")); // themes fetch throws 175 + const routes = await loadSettingsRoutes(); 176 + const res = await routes.request("/settings", { 177 + headers: { cookie: "atbb_session=token" }, 178 + }); 179 + expect(res.status).toBe(200); 180 + const html = await res.text(); 181 + // Form renders (no crash), but no theme options 182 + expect(html).toContain("id=\"lightThemeUri\""); 183 + expect(html).not.toContain("<option"); 184 + }); 185 + 134 186 // ── GET /settings — allowUserChoice: false ─────────────────────────── 135 187 136 188 it("GET /settings shows informational banner when allowUserChoice: false", async () => { ··· 201 253 ); 202 254 }); 203 255 204 - it("GET /settings with ?error=invalid-theme shows error banner", async () => { 256 + it("GET /settings with ?error=invalid-theme shows friendly message in error banner", async () => { 205 257 setupAuthenticatedSessionGet(); 206 258 const routes = await loadSettingsRoutes(); 207 259 const res = await routes.request("/settings?error=invalid-theme", { ··· 209 261 }); 210 262 expect(res.status).toBe(200); 211 263 const html = await res.text(); 212 - expect(html).toContain("invalid-theme"); 264 + expect(html).toContain("settings-banner--error"); 265 + expect(html).toContain("no longer available"); 266 + }); 267 + 268 + it("GET /settings with unknown ?error= code shows no error banner", async () => { 269 + setupAuthenticatedSessionGet(); 270 + const routes = await loadSettingsRoutes(); 271 + const res = await routes.request("/settings?error=some-unknown-code", { 272 + headers: { cookie: "atbb_session=token" }, 273 + }); 274 + expect(res.status).toBe(200); 275 + const html = await res.text(); 276 + expect(html).not.toContain("settings-banner--error"); 277 + expect(html).not.toContain("some-unknown-code"); 213 278 }); 214 279 215 280 // ── POST /settings/appearance — Unauthenticated ────────────────────── ··· 294 359 }); 295 360 expect(res.status).toBe(302); 296 361 expect(res.headers.get("location")).toBe("/settings?error=invalid-theme"); 362 + const cookies = res.headers.getSetCookie?.() ?? []; 363 + expect(cookies.length).toBe(0); 297 364 }); 298 365 299 366 it("POST /settings/appearance rejects invalid dark theme URI with ?error=invalid-theme", async () => { ··· 309 376 }); 310 377 expect(res.status).toBe(302); 311 378 expect(res.headers.get("location")).toBe("/settings?error=invalid-theme"); 379 + const cookies = res.headers.getSetCookie?.() ?? []; 380 + expect(cookies.length).toBe(0); 312 381 }); 313 382 314 383 // ── POST /settings/appearance — Policy fetch failure ────────────────── ··· 352 421 }); 353 422 expect(res.status).toBe(302); 354 423 expect(res.headers.get("location")).toBe("/settings?error=not-allowed"); 424 + // Verify no cookies are set (reject before reaching cookie-write code) 425 + const cookies = res.headers.getSetCookie?.() ?? []; 426 + expect(cookies.length).toBe(0); 355 427 }); 356 428 357 429 // ── POST /settings/appearance — Happy path ──────────────────────────
+58 -29
apps/web/src/routes/settings.tsx
··· 30 30 31 31 const saved = c.req.query("saved") === "1"; 32 32 const rawError = c.req.query("error"); 33 - // decodeURIComponent throws URIError on malformed percent-encoding (e.g. %ZZ) 34 - const errorMessage = rawError 35 - ? (() => { 36 - try { 37 - return decodeURIComponent(rawError); 38 - } catch { 39 - return rawError; 40 - } 41 - })() 42 - : undefined; 33 + const ERROR_MESSAGES: Record<string, string> = { 34 + invalid: "Please fill in all required fields.", 35 + "invalid-theme": "The selected theme is no longer available. Please choose another.", 36 + "not-allowed": "Theme selection is managed by the forum administrator.", 37 + unavailable: "Theme settings are temporarily unavailable. Please try again later.", 38 + }; 39 + const errorMessage = rawError ? (ERROR_MESSAGES[rawError] ?? undefined) : undefined; 43 40 44 41 // Fetch theme policy 45 42 let policy: Policy | null = null; ··· 71 68 72 69 // Fetch available themes list (already filtered by policy server-side in AppView) 73 70 let allThemes: Array<ThemeSummary> = []; 71 + let themesRes: Response | undefined; 74 72 try { 75 - const themesRes = await fetch(`${appviewUrl}/api/themes`); 76 - if (themesRes.ok) { 77 - const data = (await themesRes.json()) as { themes: Array<ThemeSummary> }; 78 - allThemes = data.themes ?? []; 79 - } 73 + themesRes = await fetch(`${appviewUrl}/api/themes`); 80 74 } catch (err) { 81 75 if (isProgrammingError(err)) throw err; 82 - logger.warn("Failed to fetch themes list for settings page", { 76 + logger.error("Failed to fetch themes list for settings page", { 83 77 operation: "GET /settings", 84 78 error: err instanceof Error ? err.message : String(err), 85 79 }); 86 80 } 81 + if (themesRes?.ok) { 82 + try { 83 + const data = (await themesRes.json()) as { themes: Array<ThemeSummary> }; 84 + allThemes = data.themes ?? []; 85 + } catch { 86 + logger.error("Themes list response contained invalid JSON", { 87 + operation: "GET /settings", 88 + }); 89 + } 90 + } 87 91 88 92 const lightThemes = allThemes.filter((t) => t.colorScheme === "light"); 89 93 const darkThemes = allThemes.filter((t) => t.colorScheme === "dark"); 90 94 91 - // Pre-select current preference cookie (Phase 1), falling back to forum default 95 + // Pre-select current preference cookie, falling back to forum default 92 96 const currentLightUri = 93 97 resolveUserThemePreference(cookieHeader, "light", allThemes, policy.allowUserChoice) ?? 94 98 policy.defaultLightThemeUri; ··· 185 189 } 186 190 187 191 // Fetch FRESH policy (bypass cache) so recently-removed themes can't be saved 188 - let policy: { availableThemes: Array<{ uri: string }>; allowUserChoice: boolean } | null = 189 - null; 192 + type PostPolicy = { availableThemes: Array<{ uri: string }>; allowUserChoice: boolean }; 193 + let policy: PostPolicy | null = null; 194 + let postPolicyRes: Response | undefined; 190 195 try { 191 - const policyRes = await fetch(`${appviewUrl}/api/theme-policy`); 192 - if (policyRes.ok) { 193 - policy = (await policyRes.json()) as { availableThemes: Array<{ uri: string }>; allowUserChoice: boolean }; 194 - } 196 + postPolicyRes = await fetch(`${appviewUrl}/api/theme-policy`); 195 197 } catch (err) { 196 198 if (isProgrammingError(err)) throw err; 197 199 logger.error("Failed to fetch theme policy during preference save", { 198 200 operation: "POST /settings/appearance", 199 201 error: err instanceof Error ? err.message : String(err), 200 202 }); 203 + } 204 + if (postPolicyRes?.ok) { 205 + try { 206 + policy = (await postPolicyRes.json()) as PostPolicy; 207 + } catch { 208 + logger.error("Theme policy response contained invalid JSON during preference save", { 209 + operation: "POST /settings/appearance", 210 + }); 211 + } 201 212 } 202 213 203 214 if (!policy) { ··· 249 260 <div class="theme-preview__swatches"> 250 261 {swatchTokens.map((token) => { 251 262 const color = tokens[token]; 252 - if (!color) return null; 263 + // Reject values that could escape the style attribute via CSS injection 264 + if (!color || color.includes(";") || color.includes("<") || color.includes("}")) { 265 + return null; 266 + } 253 267 return ( 254 268 <span 255 269 class="theme-preview__swatch" ··· 272 286 const rkey = parseRkeyFromUri(themeUri); 273 287 if (!rkey || !/^[a-z0-9-]+$/i.test(rkey)) return c.html(emptyFragment); 274 288 289 + let previewRes: Response; 275 290 try { 276 - const res = await fetch(`${appviewUrl}/api/themes/${rkey}`); 277 - if (!res.ok) return c.html(emptyFragment); 278 - const theme = (await res.json()) as { 291 + previewRes = await fetch(`${appviewUrl}/api/themes/${rkey}`); 292 + } catch (err) { 293 + if (isProgrammingError(err)) throw err; 294 + logger.warn("Failed to fetch theme for preview", { 295 + operation: "GET /settings/preview", 296 + rkey, 297 + error: err instanceof Error ? err.message : String(err), 298 + }); 299 + return c.html(emptyFragment); 300 + } 301 + if (!previewRes.ok) return c.html(emptyFragment); 302 + 303 + try { 304 + const theme = (await previewRes.json()) as { 279 305 name: string; 280 306 tokens: Record<string, string>; 281 307 }; 282 308 return c.html( 283 309 <ThemeSwatchPreview name={theme.name ?? ""} tokens={theme.tokens ?? {}} /> 284 310 ); 285 - } catch (err) { 286 - if (isProgrammingError(err)) throw err; 311 + } catch { 312 + logger.warn("Theme preview response contained invalid JSON", { 313 + operation: "GET /settings/preview", 314 + rkey, 315 + }); 287 316 return c.html(emptyFragment); 288 317 } 289 318 });