fix(astro,tanstack-react-start): Fix XSS via user-controlled JSON#9166
fix(astro,tanstack-react-start): Fix XSS via user-controlled JSON#9166dominic-clerk wants to merge 2 commits into
Conversation
The JSON injected in the page may contain user-controlled data in certain circumstances. This PR introduces a helper that ensures the JSON cannot be broken out of to cause XSS.
🦋 Changeset detectedLatest commit: 7f76f3b The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughClerk adds ChangesSSR script serialization
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/shared/src/__tests__/htmlSafeJson.spec.ts (1)
5-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd or confirm integration coverage for the SSR script boundaries.
These tests validate
htmlSafeJson, but not the generated Astro<script>markup or TanStackScriptOnceoutput. Add focused integration tests with</script>and U+2028/U+2029 payloads, or confirm equivalent tests already exist, so either integration cannot regress to rawJSON.stringify.As per coding guidelines, new functionality should include integration coverage and edge-case verification.
🤖 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 `@packages/shared/src/__tests__/htmlSafeJson.spec.ts` around lines 5 - 37, Add integration coverage for the SSR script-rendering paths that consume htmlSafeJson, including Astro-generated script markup and TanStack ScriptOnce output. Use payloads containing </script> and U+2028/U+2029, assert the rendered markup remains safely escaped rather than raw JSON.stringify output, and verify the payload still round-trips when parsed.Source: Coding guidelines
packages/shared/src/htmlSafeJson.ts (1)
5-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComplete the public API JSDoc.
This exported, cross-package helper is missing the required
@param,@returns,@throws, and@exampletags. Document the"undefined"result and serialization errors; the Docs team should also review the generated reference documentation.As per coding guidelines, public APIs require complete JSDoc tags. As per path instructions, exported reference-facing APIs may appear in generated Clerk Docs.
Suggested documentation addition
/** * `JSON.stringify` that is safe to embed directly inside an HTML `<script>` element. + * + * `@param` value - The value to serialize. + * `@returns` HTML-safe JSON, or `"undefined"` when serialization produces no value. + * `@throws` {TypeError} If the value cannot be serialized. + * `@example` + * JSON.parse(htmlSafeJson({ html: '</script>' })); */🤖 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 `@packages/shared/src/htmlSafeJson.ts` around lines 5 - 13, Complete the JSDoc for the exported htmlSafeJson function with `@param`, `@returns`, `@throws`, and `@example` tags. Document that undefined produces the string "undefined" and that serialization errors may be thrown, and include a representative safe-HTML serialization example without changing the implementation.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@packages/shared/src/__tests__/htmlSafeJson.spec.ts`:
- Around line 5-37: Add integration coverage for the SSR script-rendering paths
that consume htmlSafeJson, including Astro-generated script markup and TanStack
ScriptOnce output. Use payloads containing </script> and U+2028/U+2029, assert
the rendered markup remains safely escaped rather than raw JSON.stringify
output, and verify the payload still round-trips when parsed.
In `@packages/shared/src/htmlSafeJson.ts`:
- Around line 5-13: Complete the JSDoc for the exported htmlSafeJson function
with `@param`, `@returns`, `@throws`, and `@example` tags. Document that undefined
produces the string "undefined" and that serialization errors may be thrown, and
include a representative safe-HTML serialization example without changing the
implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: d69a6635-cccd-4bb5-b0c2-db08ff477183
📒 Files selected for processing (5)
.changeset/open-buckets-bake.mdpackages/astro/src/server/clerk-middleware.tspackages/shared/src/__tests__/htmlSafeJson.spec.tspackages/shared/src/htmlSafeJson.tspackages/tanstack-react-start/src/client/ClerkProvider.tsx
API Changes Report
Summary
@clerk/sharedCurrent version: 4.25.3 Subpath
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
1 participant
Description
The JSON injected in the page may contain user-controlled data in certain circumstances. This PR introduces a helper that ensures the JSON cannot be broken out of to cause XSS.
Fixes SDK-57
Fixes SDK-58
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Security
Bug Fixes
Tests