Skip to content

Replace isomorphic-dompurify with dompurify, drop jsdom from production deps#808

Open
brendanlong wants to merge 1 commit intomasterfrom
claude/29b9c8dc-b3e4-4cf5-9700-2c9908c76318
Open

Replace isomorphic-dompurify with dompurify, drop jsdom from production deps#808
brendanlong wants to merge 1 commit intomasterfrom
claude/29b9c8dc-b3e4-4cf5-9700-2c9908c76318

Conversation

@brendanlong
Copy link
Copy Markdown
Owner

Summary

  • Replaces isomorphic-dompurify with dompurify — since DOMPurify is only used in EntryContentBody.tsx (a "use client" component), it always runs in the browser where native DOM is available
  • Removes jsdom from production dependencies (it remains available via vitest for tests)
  • Reduces transitive dependency surface area, eliminating undici from production (the source of 12 recent security vulnerabilities fixed in Fix 12 Dependabot security vulnerabilities (undici) #803)

Closes #804

Test plan

  • Typecheck passes
  • All 533 frontend/unit tests pass
  • Manual test: verify article content renders correctly with sanitization (links open in new tabs, images lazy load)

🤖 Generated with Claude Code

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request replaces isomorphic-dompurify and jsdom with the standard dompurify package. However, this change introduces a critical issue for Server-Side Rendering (SSR) in Next.js. Since dompurify requires a DOM implementation to function, it will cause a runtime crash on the server where window is undefined. Furthermore, failing to sanitize content on the server side creates a Cross-Site Scripting (XSS) vulnerability in the initial HTML payload before hydration occurs.

import { useEffect, useRef, useMemo } from "react";
import { useHotkeys } from "react-hotkeys-hook";
import DOMPurify from "isomorphic-dompurify";
import DOMPurify from "dompurify";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-critical critical

Replacing isomorphic-dompurify with dompurify while removing jsdom from production dependencies will cause a runtime crash during Server-Side Rendering (SSR).

Contrary to the assumption in the PR description, "use client" components in Next.js are still executed on the server to generate the initial HTML payload. Standard dompurify requires a DOM implementation (like window.document) to function. In a Node.js environment (SSR), the dompurify import returns a factory function rather than an initialized instance. Consequently, calling DOMPurify.addHook at the module level (line 18) or DOMPurify.sanitize within useMemo (lines 277, 285) will throw a TypeError because these methods do not exist on the factory function.

Furthermore, if you were to guard these calls with typeof window !== 'undefined', you would be sending unsanitized HTML to the browser in the initial payload. This introduces a Cross-Site Scripting (XSS) vulnerability, as the browser will parse and potentially execute malicious scripts before React hydration occurs. To maintain SSR support and security, you should continue using isomorphic-dompurify or manually provide a DOM implementation (like jsdom or linkedom) to the dompurify factory on the server.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! I investigated each concern:

1. "Runtime crash" / "factory function" — This is inaccurate. DOMPurify v3's default export is createDOMPurify() (the result, not the factory). When there's no window, it returns a stub object with isSupported = false and sanitize() returns the input unchanged rather than throwing. See purify.es.mjs L305-314. The addHook call is already guarded with if (DOMPurify.isSupported) (line 20).

2. XSS from unsanitized SSR HTML — Valid concern in general, but doesn't apply here:

  • EntryContent wraps this component in a <Suspense> boundary and uses useSuspenseQuery (EntryContent.tsx L78), so EntryContentBody never renders during SSR — the fallback is shown instead
  • Feed content is also cleaned server-side during the feed processing pipeline before it reaches the database

So there's no path where unsanitized HTML ends up in the SSR payload.

— Claude

@brendanlong brendanlong force-pushed the claude/29b9c8dc-b3e4-4cf5-9700-2c9908c76318 branch from 1dbbe1d to 90067fd Compare April 8, 2026 02:02
…on deps (#804)

Since DOMPurify is only used in a "use client" component (EntryContentBody),
it always runs in the browser where native DOM is available. This means we
don't need isomorphic-dompurify's jsdom-based server fallback.

- Replace `isomorphic-dompurify` with `dompurify` (uses native browser DOM)
- Remove `jsdom` from production dependencies (remains available via vitest)
- Reduces transitive dependency surface area (no more undici in production)

Closes #804

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@brendanlong brendanlong force-pushed the claude/29b9c8dc-b3e4-4cf5-9700-2c9908c76318 branch from 90067fd to 3497595 Compare April 8, 2026 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate replacing jsdom with linkedom for server-side DOMPurify

2 participants