src/app/notifications/NotificationsPage.tsx (1)

40-46: 💤 Low value

Simplify parseSelectedCategory by checking enum membership directly.

The four-way if-chain duplicates every enum member by name and will silently rot if a new NotificationCategory variant is added — the new value would be coerced to 'all' instead of itself. A single membership check against the enum's values keeps it data-driven:

♻️ Proposed simplification

-function parseSelectedCategory(value: string): NotificationCategory | 'all' {
-  if (value === NotificationCategory.ENGAGEMENT) return NotificationCategory.ENGAGEMENT
-  if (value === NotificationCategory.CONTENT) return NotificationCategory.CONTENT
-  if (value === NotificationCategory.SYSTEM) return NotificationCategory.SYSTEM
-  if (value === NotificationCategory.MODERATION) return NotificationCategory.MODERATION
-  return 'all'
-}
+const CATEGORY_VALUES = Object.values(NotificationCategory) as NotificationCategory[]
+
+function parseSelectedCategory(value: string): NotificationCategory | 'all' {
+  return (CATEGORY_VALUES as string[]).includes(value) ? (value as NotificationCategory) : 'all'
+}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/app/notifications/NotificationsPage.tsx` around lines 40 - 46, The
function parseSelectedCategory repeats each NotificationCategory member and will
miss future enum values; change it to perform a membership check against the
enum values (e.g. use Object.values(NotificationCategory).includes(value)) and,
if present, return the value cast to NotificationCategory, otherwise return
'all'—update parseSelectedCategory to rely on enum membership rather than
hard-coded branches.

src/app/docs/api/swagger/SwaggerPage.tsx (1)

8-8: 💤 Low value

Bundle the 563 KB OpenAPI spec only when needed, not in the initial page chunk.

Two concerns with this static import:

1. Non-idiomatic path traversal: Importing from public/ via ../../../../../ is brittle (any file move breaks it) and violates the intended use of public/ for static asset serving, not module resolution.

2. Deferred loading lost: Switching from url to configuration.content = openApiDocument bundles the ~563 KB spec into the page's client JavaScript chunk, downloaded by every visitor. The component itself is lazy-loaded (line 13), but the spec payload is not, negating the performance benefit of deferred loading.

Suggested fix: Move mobile-openapi.json to src/ with a path alias (e.g., @/server/api), or restore url-based loading so Scalar fetches the spec on demand—especially given the file size.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/app/docs/api/swagger/SwaggerPage.tsx` at line 8, The static import of
mobile-openapi.json in SwaggerPage.tsx (openApiDocument) pulls a 563KB spec into
the initial client bundle and uses a brittle relative path into public/;
instead, stop the direct import and either (A) move the JSON into src/ and
import it via your project path alias (e.g., from `@/server/api`) so it becomes a
proper module you can lazy-load, or (B) restore URL-based loading so the
component sets configuration.content by fetching the JSON at runtime (keep the
lazy-loaded component intact) and assign configuration.content only after the
fetch resolves; also remove the ../../../../../public import and update any
references to openApiDocument and configuration.content accordingly.

src/utils/markdown.ts (1)

8-10: ⚖️ Poor tradeoff

The workaround is partially redundant; jsdom is already marked as server-external in next.config.js.

serverExternalPackages: ['jsdom'] is already configured, which mitigates the bundling risk. However, the fragile 'js' + 'dom' concatenation and __non_webpack_require__ workaround remain unnecessary. Simplify to a plain import { JSDOM } from 'jsdom' at the top of the file, since jsdom is already guaranteed to be server-external.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/markdown.ts` around lines 8 - 10, Remove the fragile runtime
workaround using __non_webpack_require__ and the JSDOM_PACKAGE_NAME
concatenation and replace it with a direct top-level import; specifically delete
the declare __non_webpack_require__ and const JSDOM_PACKAGE_NAME = 'js' + 'dom'
lines and add a plain import { JSDOM } from 'jsdom' at the top so the file uses
the JSDOM symbol directly (serverExternalPackages already prevents bundling).

src/utils/markdown.test.ts (2)

153-158: 💤 Low value

Consider tightening the data: assertion to avoid false negatives.

expect(result).not.toContain('data:') will pass if the sanitizer simply HTML-encodes the colon (e.g., data:) or any other encoding form, even though the underlying URI would still resolve in some contexts. Since the input also contains a base64-encoded <script> payload (PHNjcmlwdD4=), it might be worth additionally asserting not.toContain('PHNjcmlwdD4=') (or the decoded <script>) to ensure the full payload is dropped rather than just the scheme literal.

♻️ Suggested additional assertion

     it('should strip unsafe data URI markdown links', () => {
       const result = parseMarkdown('[bad](data:text/html;base64,PHNjcmlwdD4=)')

       expect(result).not.toContain('data:')
       expect(result).not.toContain('href=')
+      expect(result).not.toContain('PHNjcmlwdD4=')
     })

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/markdown.test.ts` around lines 153 - 158, The test "should strip
unsafe data URI markdown links" currently only asserts absence of the literal
"data:" which can be evaded by encoding; update the test around
parseMarkdown('[bad](data:text/html;base64,PHNjcmlwdD4=)') to also assert that
the base64 payload and decoded tag are removed — e.g., add negative assertions
for the base64 chunk "PHNjcmlwdD4=" and for the decoded "<script>" token — so
parseMarkdown is verified to drop the actual payload, not just the scheme
literal.

---

169-178: ⚡ Quick win

Wall-clock ReDoS assertion is likely to flake on shared CI runners.

expect(duration).toBeLessThan(1000) is sensitive to runner load, GC pauses, and noisy neighbors. A 30k-char linkify input that normally completes in tens of milliseconds locally can occasionally cross 1s on a contended CI VM, leading to false positives that look like security regressions. A few options to make this more robust:

• Raise the budget to something clearly catastrophic (e.g., 3–5s) so only true ReDoS regressions trip it, while still failing fast on quadratic/exponential blowups.
• Combine the time bound with a process-level timeout (Vitest { timeout }) and prefer performance.now() over Date.now() for sub-ms precision.
• Optionally gate strict timing in CI via an env flag, or mark the test as it.concurrent only when timing is loose.

♻️ Suggested adjustment

-    it('should handle markdown-it linkify ReDoS input quickly', () => {
-      const redosInput = `https://example.com/${'*'.repeat(30000)}!`
-      const start = Date.now()
-
-      const result = parseMarkdown(redosInput)
-      const duration = Date.now() - start
-
-      expect(typeof result).toBe('string')
-      expect(duration).toBeLessThan(1000)
-    })
+    it(
+      'should handle markdown-it linkify ReDoS input quickly',
+      () => {
+        const redosInput = `https://example.com/${'*'.repeat(30000)}!`
+        const start = performance.now()
+
+        const result = parseMarkdown(redosInput)
+        const duration = performance.now() - start
+
+        expect(typeof result).toBe('string')
+        // Generous bound: catches catastrophic backtracking without
+        // flaking on contended CI runners.
+        expect(duration).toBeLessThan(3000)
+      },
+      5000,
+    )

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/markdown.test.ts` around lines 169 - 178, The wall-clock assertion
in the test for parseMarkdown is flaky; switch timing to use performance.now()
instead of Date.now(), raise the allowed duration to a safer threshold (e.g.,
3000 ms) to avoid CI noise, and add a Vitest per-test timeout (e.g., set the
it(...) timeout to 5000 ms) so the test fails by the test runner on catastrophic
hangs rather than flaky assertions; update the test case surrounding
parseMarkdown(redosInput) to compute duration via performance.now(), assert
duration < 3000, and pass a timeout option to the it call.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/utils/markdown.ts (3)

10-10: 💤 Low value

Add a brief comment explaining the 'js' + 'dom' concatenation.

The string concatenation is a deliberate trick to prevent webpack from statically analyzing and bundling jsdom into client output, but this intent is non-obvious. A short comment will save future maintainers from "simplifying" it back to a literal.

📝 Proposed comment

-const JSDOM_PACKAGE_NAME = 'js' + 'dom'
+// Concatenated to hide the literal from webpack's static analyzer so `jsdom`
+// is never bundled into client output; it must only be resolved at runtime on the server.
+const JSDOM_PACKAGE_NAME = 'js' + 'dom'

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/markdown.ts` at line 10, Add a brief inline comment above the
JSDOM_PACKAGE_NAME constant explaining that the 'js' + 'dom' concatenation is
intentional to avoid static bundlers (e.g., webpack) from detecting and bundling
the "jsdom" package into client builds; reference the constant name
JSDOM_PACKAGE_NAME and keep the comment short and explicit about the bundling
avoidance intent.

---

101-112: 💤 Low value

Redundant guard, but functionally correct.

The typeof process.getBuiltinModule !== 'function' check at line 102 and the if (!moduleBuiltin) check at line 107 throw the same error. In practice, when getBuiltinModule exists, calling it with 'module' will always return the builtin (or throw ERR_UNKNOWN_BUILTIN_MODULE), so the second guard is unreachable defensive coding. Not worth changing on its own, but consider consolidating if you touch this again.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/markdown.ts` around lines 101 - 112, The getServerRequire function
contains a redundant second guard that checks moduleBuiltin after verifying
process.getBuiltinModule exists; consolidate the checks by either using a single
try/catch around calling process.getBuiltinModule('module') or keep the initial
typeof check and remove the unreachable if (!moduleBuiltin) branch. Update
getServerRequire to call process.getBuiltinModule('module') inside a try block
(or after the typeof check) and throw the same Error('Node module loader is
unavailable for server-side markdown sanitization') if obtaining the builtin
fails, then return moduleBuiltin.createRequire(`${process.cwd()}/package.json`)
as before.

---

122-165: 💤 Low value

Fallback in catch reuses the same DOMPurify instance that may have caused the failure.

If getPurify() ever throws during initialization (e.g., jsdom resolution fails at runtime on the server path), sanitizeHtml(html) throws on line 128, then the catch invokes sanitizePlainText(markdownText) which also calls getPurify() — re-throwing the exact same error and propagating out of parseMarkdown. Same applies to stripMarkdown at line 163.

This is a narrow edge case (only matters when DOMPurify init itself fails, not when markdown-it fails), so it's a defense-in-depth concern rather than a blocker. If you want a truly safe terminal fallback, escape with a tiny inline helper instead of routing back through DOMPurify:

♻️ Optional safer fallback

+function escapeHtml(text: string): string {
+  return text
+    .replace(/&/g, '&')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&#x27;')
+}
+
 export function parseMarkdown(markdownText: string): string {
   if (!markdownText) return ''
 
   try {
     const html = md.render(markdownText)
     return sanitizeHtml(html)
   } catch (error) {
     console.warn('Markdown parsing failed:', error)
-    return sanitizePlainText(markdownText)
+    return escapeHtml(markdownText)
   }
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/markdown.ts` around lines 122 - 165, parseMarkdown and
stripMarkdown currently call sanitizePlainText in their catch blocks which in
turn invokes getPurify(), so if DOMPurify/init fails the fallback will re-throw
the same error; replace those fallbacks with a safe, pure-JS escape that does
not call getPurify() (e.g., a small inline escapeHtml helper that replaces &, <,
>, ", ' and collapses whitespace/newlines to single spaces) and return that
escaped string from the catch in both parseMarkdown and stripMarkdown; keep
references to md.render usage intact but ensure the catch never calls
sanitizePlainText or any function that calls getPurify().