fix: address review findings -- caching bug, tests, comment clarity

Aureliolo · claude · Aureliolo · commit b003f65ea584 · 2026-03-30T11:33:49.000+02:00
Fix getCspNonce caching: use Symbol sentinel so absent-nonce results are
also cached (previously re-queried DOM on every call). Trim whitespace
from nonce content. Add tests for whitespace-only content and DOM
re-query prevention. Improve index.html comment with cross-references
and include rule name in ESLint escape hatch message.

Pre-reviewed by 5 agents, 4 findings addressed

Co-Authored-By: Claude Opus 4.6 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/web/eslint.config.js b/web/eslint.config.js
@@ -29,7 +29,7 @@ export default tseslint.config(
           selector: 'JSXAttribute[name.name="dangerouslySetInnerHTML"]',
           message:
             'dangerouslySetInnerHTML is banned -- use text content or a sanitization library. ' +
-            'If absolutely necessary, add // eslint-disable-next-line with a justification comment.',
+            'If absolutely necessary, add // eslint-disable-next-line no-restricted-syntax with a justification comment.',
         },
       ],
       // Rule flags every obj[var] with no data-flow analysis -- too many false
diff --git a/web/index.html b/web/index.html
@@ -5,7 +5,8 @@
     <meta name="viewport" content="width=device-width, initial-scale=1.0" />
     <meta name="description" content="SynthOrg -- monitoring and management dashboard for synthetic organizations" />
     <!-- CSP nonce: nginx sub_filter replaces __CSP_NONCE__ on each request.
-         Uncomment when CSP is tightened to use nonce-based style-src. -->
+         Uncomment when style-src 'unsafe-inline' in security-headers.conf is
+         replaced with nonce-based policy. See App.tsx MotionConfig + lib/csp.ts. -->
     <!-- <meta name="csp-nonce" content="__CSP_NONCE__" /> -->
     <link rel="icon" type="image/svg+xml" href="/favicon.svg" />
     <title>SynthOrg Dashboard</title>
diff --git a/web/src/__tests__/lib/csp.test.ts b/web/src/__tests__/lib/csp.test.ts
@@ -42,4 +42,31 @@ describe('getCspNonce', () => {
     const { getCspNonce } = await import('@/lib/csp')
     expect(getCspNonce()).toBeUndefined()
   })
+
+  it('returns undefined for whitespace-only content', async () => {
+    const meta = document.createElement('meta')
+    meta.name = 'csp-nonce'
+    meta.content = '   '
+    document.head.appendChild(meta)
+
+    const { getCspNonce } = await import('@/lib/csp')
+    expect(getCspNonce()).toBeUndefined()
+  })
+
+  it('caches absent result and does not re-query DOM', async () => {
+    const spy = vi.spyOn(document, 'querySelector')
+
+    const { getCspNonce } = await import('@/lib/csp')
+    expect(getCspNonce()).toBeUndefined()
+    expect(getCspNonce()).toBeUndefined()
+
+    // querySelector called once during import-time init + once for first call
+    // Second call should hit cache, not query DOM again
+    const cspCalls = spy.mock.calls.filter(
+      ([sel]) => sel === 'meta[name="csp-nonce"]',
+    )
+    expect(cspCalls).toHaveLength(1)
+
+    spy.mockRestore()
+  })
 })
diff --git a/web/src/lib/csp.ts b/web/src/lib/csp.ts
@@ -5,17 +5,19 @@
  * infrastructure (e.g. nginx `sub_filter`). If no meta tag is present
  * (local dev, environments without CSP nonce injection), returns `undefined`.
  *
- * The value is read once and cached for the lifetime of the page.
+ * The value is read once on first call and cached for the lifetime of the
+ * page -- both present and absent results are cached.
  */
 
-let cached: string | undefined
+const UNREAD: unique symbol = Symbol('unread')
+let cached: string | undefined | typeof UNREAD = UNREAD
 
 export function getCspNonce(): string | undefined {
-  if (cached !== undefined) return cached
+  if (cached !== UNREAD) return cached as string | undefined
 
   const meta = document.querySelector<HTMLMetaElement>(
     'meta[name="csp-nonce"]',
   )
-  cached = meta?.content || undefined
+  cached = meta?.content?.trim() || undefined
   return cached
 }
