Repository navigation
Conversation
javascript-sdk-gitflow
Bot
requested review from
andreiborza,
nicohrubec and
s1gr1d
November 12, 2026 10:22
Contributor
size-limit report 📦Compared: 3562cb3 (PR test merge using develop at c9e61e3) Against: f49009b (develop) · base run #65896
|
Contributor
|
One or more gzipped bundles increased by more than 500 bytes. If this increase is intentional, add the Accept Bundlesize Increase label to this PR to rerun and accept the check. |
Contributor
Optional E2E testsSome optional checks were not successful. They do not block the merge. 4 not successful, 55 successful. See the run.
Successful checks
|
isaacs
force-pushed
the
feat/escape-json-stringify-for-js-injection
branch
from
November 12, 2026 11:19
1d4e4d8 to
97cab5a
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 97cab5a. Configure here.
| ): string { | ||
| // `JSON.stringify` returns `undefined` for `undefined` and functions, although its type says `string`. | ||
| // oxlint-disable-next-line typescript/no-explicit-any | ||
| return JSON.stringify(value, replacer as any, space)?.replace(UNSAFE_CHARS, char => ESCAPED_CHARS[char] as string); |
There was a problem hiding this comment.
Uncommented any casts in helper
Low Severity
This was flagged because the review rules require a comment on new any and as any in SDK source explaining why a safer type is not possible. The replacer parameter and the replacer as any pass-through have oxlint disables, but those comments do not explain the unsafe typing.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 97cab5a. Configure here.
JPeer264
approved these changes
Nov 12, 2026
|
|
||
| - Follow existing conventions — check neighboring files | ||
| - Reach for existing utils before writing a new one. Most shared helpers live in `@sentry/core` (`packages/core/src/utils/`), with browser helpers in `packages/browser-utils/`. Search first (LSP `workspaceSymbol` or grep) for common needs (type guards in `is.ts`, object/array helpers, `normalize`, `dsn`, `merge`, string/url helpers). Reuse or extend the existing util rather than adding a near-duplicate; only introduce a new util when nothing fits. | ||
| - When `JSON.stringify` output goes into generated code (bundler plugins, loaders, virtual modules, `define`), use `stringifyForCodeInjection` from `@sentry/core` instead. It escapes `<`, `>`, U+2028 and U+2029, so the code stays safe when inlined in a `<script>` element. |
Member
There was a problem hiding this comment.
l: Even though we might retire it, but adding it to BUGBOT.md wouldn't harm. Especially if we at some point migrate that somehow to Warden
JPeer264
force-pushed
the
feat/escape-json-stringify-for-js-injection
branch
from
November 12, 2026 13:28
97cab5a to
ecc2738
Compare
`JSON.stringify` output is a valid JavaScript expression, but it does not escape `<` and `>`. When a value with `</script>` or `<!--` goes into generated code and the bundle is inlined in HTML, it breaks out of the `<script>` element. CodeQL flags this pattern (js/bad-code-sanitization). This helper avoids the issue. Two packages had their own private escape helpers, and they did different things: `bundler-plugins` escaped `<`, and Remix escaped `</` and `<!--`. The other SDKs did not escape at all. Now `stringifyForCodeInjection` in `@sentry/core` is the one helper. It is a drop-in replacement for `JSON.stringify` that also escapes `<`, `>`, U+2028 and U+2029. These characters only occur inside JSON strings, where their `\u` escapes have the same value. `JSON.stringify` already escapes the other characters in the CodeQL example. Build-time code in astro, bundler-plugins, cloudflare, nextjs, nuxt, remix, server-utils, solidstart, svelte, sveltekit and tanstackstart-react now uses it for code, virtual modules, loaders and Vite `define` values. `AGENTS.md` points to it. These uses of `JSON.stringify` stay as they are: - log and warning messages - boolean `define` and `DefinePlugin` values, and npm package names in `__SENTRY_SERVER_MODULES__` - the `node` local variables worker, which sends code to the V8 inspector, not to HTML - the Node ESM loader in `server-runtime-injection`, which is URL-encoded into a `data:` URL - base64-encoded source maps Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
JPeer264
force-pushed
the
feat/escape-json-stringify-for-js-injection
branch
from
November 13, 2026 06:40
ecc2738 to
4d40b75
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JSON.stringifyoutput is a valid JavaScript expression, but it does not escape<and>. When a value with</script>or<!--goes into generated code and the bundle is inlined in HTML, it breaks out of the<script>element.CodeQL flags this pattern (js/bad-code-sanitization). This helper avoids the issue.
Two packages had their own private escape helpers, and they did different things:
bundler-pluginsescaped<, and Remix escaped</and<!--. The other SDKs did not escape at all. NowstringifyForCodeInjectionin@sentry/coreis the one helper. It is a drop-in replacement forJSON.stringifythat also escapes<,>, U+2028 and U+2029. These characters only occur inside JSON strings, where their\uescapes have the same value.JSON.stringifyalready escapes the other characters in the CodeQL example.Build-time code in astro, bundler-plugins, cloudflare, nextjs, nuxt, remix, server-utils, solidstart, svelte, sveltekit and tanstackstart-react now uses it for code, virtual modules, loaders and Vite
definevalues.AGENTS.mdpoints to it.These uses of
JSON.stringifystay as they are:defineandDefinePluginvalues, and npm package names in__SENTRY_SERVER_MODULES__nodelocal variables worker, which sends code to the V8 inspector, not to HTMLserver-runtime-injection, which is URL-encoded into adata:URL