Skip to content

fix: inline the public Sentry DSN into the client bundle - #95

Merged
Skeyelab merged 1 commit into
masterfrom
fix/client-sentry-dsn-inlining
Sep 23, 2026
Merged

Skeyelab merged 1 commit into
masterfrom
fix/client-sentry-dsn-inlining

Conversation

@Skeyelab

Copy link
Copy Markdown
Member

Follow-up to #94, which shipped broken client-side error reporting.

What went wrong

buildSentryOptions reads env values through env[key]. Next only substitutes literal process.env.NEXT_PUBLIC_* expressions into the client bundle, so a dynamic read is left as-is and evaluates to undefined in the browser. Sentry then disables itself silently — no error, no warning.

Verified against the deployed site after #94: the Sentry SDK was present in two chunks, the DSN string in none, while PostHog's phc_… token was inlined because instrumentation-client.ts references it literally.

Server and edge runtimes were never affected: they read process.env at runtime, where dynamic access is fine.

Fix

The client now passes its values as literal process.env.X expressions. Client-side environment and release come from NEXT_PUBLIC_-prefixed names, since SOURCE_COMMIT is not available in the browser.

Test

A unit test cannot observe bundler substitution, so the guard is a static one: it asserts instrumentation-client.ts contains the literal reference and does not use the no-argument buildSentryOptions('client') form. Both tests fail against the code as merged in #94.

Also confirmed the real way — built with NEXT_PUBLIC_SENTRY_DSN set to a probe value and grepped the output: the probe now appears in a client chunk, where before it appeared in none.

Verification

  • yarn lint clean
  • yarn test:run — 304 tests, 70 files, all passing
  • yarn build passes

PR #94 read the DSN through a helper that indexes process.env dynamically.
Next only substitutes literal `process.env.NEXT_PUBLIC_*` expressions when
building the client bundle, so the dynamic read survived into the browser as
undefined and Sentry disabled itself with no error. Verified against the
deployed site: the SDK shipped but the DSN string was absent, while PostHog's
token was inlined because it is referenced literally.

Server and edge runtimes were unaffected — they read process.env at runtime.

The client now passes literal references explicitly, and a guard test asserts
the literal is present and that the no-argument form is not used, since a unit
test cannot observe bundler substitution. Confirmed with a real build: a probe
DSN now appears in a client chunk.
@strix-security

strix-security Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Strix Security Review

No security issues found.

Review summary

Reviewed both changed files: instrumentation-client.ts and the new tests/observability/client-dsn-inlining.test.ts. The change replaces the dynamic buildSentryOptions('client') call (which left env lookups unresolved in the browser) with a literal-mapping form that Next.js will substitute at build time. The inlined values are NEXT_PUBLIC_SENTRY_DSN, NODE_ENV, NEXT_PUBLIC_SENTRY_ENVIRONMENT, and NEXT_PUBLIC_SENTRY_RELEASE. The buildSentryOptions helper's readDsn path for the client runtime only reads NEXT_PUBLIC_SENTRY_DSN and never the server-only SENTRY_DSN, so only the intended public DSN is bundled; the release/environment values are non-secret identifiers and the commit hash (SOURCE_COMMIT) is deliberately not exposed. No secrets, injection sinks, auth/authorization gaps, or other vulnerabilities were introduced by this diff. The change is in fact a hardening improvement that keeps server-only Sentry configuration out of browser code.

Updated for 78b542d.


Reviewed by Strix
Re-run review · Configure security review settings

@Skeyelab
Skeyelab merged commit 1d3048c into master Sep 23, 2026
3 checks passed
@Skeyelab
Skeyelab deleted the fix/client-sentry-dsn-inlining branch September 23, 2026 20:26
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.

1 participant