Uh oh!
There was an error while loading. Please reload this page.
console/app-shell: stop shipping an un-disableable Sentry DSN, and make sendDefaultPii opt-in (#5522) - #5559
Merged
Conversation
…ed bundle `@object-ui/console` publishes a PRE-BUILT SPA, so one artifact built from `apps/console/.env.production` is what the hosted SaaS console and the on-premises / air-gapped EE images all embed. Vite inlines every `VITE_*` from that file into the bundle as a frozen object literal, which made the committed `VITE_SENTRY_DSN` a live third-party telemetry endpoint compiled into artifacts that land inside customer networks — and un-disableable, because the `VITE_SENTRY_ENABLED` kill switch is read off that same frozen literal and is `undefined` forever on a build that never defined it. - drop the DSN, environment and `SEND_DEFAULT_PII=true` from `.env.production`; builds that want reporting inject the DSN from their own deploy environment - `sendDefaultPii` becomes opt-in (`=== 'true'`), so IP + User-Agent are never the inherited default of a build that did not ask for them - make the gate fail CLOSED and document why the direction is inverted here - ratchet test on `.env.production`, plus gate tests with a counter-probe Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014zHsbJoTkTZeJQ5DLbRXrE
The gate's decision was unreachable from tests: this repo's Vitest exposes only BASE_URL/DEV/MODE/PROD/SSR on `import.meta.env`, and `vi.stubEnv` writes to `process.env` without reaching `import.meta.env` — measured, after a first draft whose five positive cases all failed while its three absence cases "passed". An untestable gate is how the previous one stayed broken. Also moves the `.env` ratchet next to `sentry.ts`: Vite's `server.fs.deny` blocks `.env*` from `?raw` imports, and the console's tsconfig is browser-only so it cannot use `node:fs` either — app-shell's carries `types: [node, ...]`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014zHsbJoTkTZeJQ5DLbRXrE
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Part of #5522 — not
Fixes. See "What this does not close" at the end.@object-ui/consolepublishes a pre-built SPA, so ONE artifact — built once fromapps/console/.env.production— is what the hosted SaaS console and the on-premises /air-gapped EE images all embed. Vite inlines every
VITE_*from that file into the bundleas a frozen object literal, so the DSN committed there was a live third-party telemetry
endpoint compiled into artifacts that land inside customer networks.
It could not be switched off afterwards either: the
VITE_SENTRY_ENABLEDkill switch isread off that same frozen literal, so on a shipped bundle it is
undefinedforever andediting env vars on the deployed host does nothing. Upstream (objectstack-ai/cloud#1508,
graded p0/security) an air-gapped deployment was measured sending 14 envelopes per session
to sentry.io carrying IP + User-Agent PII, unstoppable by the customer.
What changed
apps/console/.env.productionno longer definesVITE_SENTRY_DSN,VITE_SENTRY_ENVIRONMENTorVITE_SENTRY_SEND_DEFAULT_PII. A build with no DSN neverimports
@sentry/react, so thevendor-sentrychunk is not even fetched.sendDefaultPiichanged from opt-out (!== 'false') to opt-in (=== 'true'), so IPaddress and User-Agent are never the inherited default of a build that did not ask.
resolveSentryGate(env)and now fails closed: absent,empty, whitespace-only or non-string DSN, and a null/undefined env object, all return
enabled: falsebefore the dynamic import. Every withheld verdict also carriessendDefaultPii: false, so no disabled branch is one refactor away from leaking.The fail direction is deliberately inverted from the usual: an unreported error is
recoverable, PII leaving an air-gapped deployment is not.
Why the gate became a pure function, and why that is not incidental
The first draft stubbed environment variables and silently tested nothing. This repo's
Vitest exposes only
BASE_URL/DEV/MODE/PROD/SSRonimport.meta.env, andvi.stubEnvwrites toprocess.envwithout reachingimport.meta.env— so every caselanded on the no-DSN branch, which is exactly why the three "absence" cases appeared to
pass. Making the decision reachable is the fix; no assertion was relaxed.
Verification (all on
f751c0288, the commit this branch points at)main: the live DSN appears 6 times across 3 chunks,and the compiled gate reads
e.VITE_SENTRY_ENABLEDfrom a literal ending…VITE_SENTRY_SEND_DEFAULT_PII:"true",VITE_SERVER_URL:"",VITE_USE_MOCK_SERVER:"false"}—the key is provably absent from the literal it is read from.
sentry.io host 0.
this same tree, the injected DSN inlines 6 times and the
vendor-sentrychunk isemitted — identical in shape to the defect. So the zero is a gate, not a build that wired
nothing up. The SaaS path is intact.
the DSN turns the ratchet red on that file only (
Tests 1 failed | 6 passed);reverting
sendDefaultPiito the opt-out spelling turns exactly the two PII-default casesred (
Tests 2 failed | 12 passed). Each mutation was proved on disk by grep counts beforeany output was read, under a restoring
trap.'TRUE','True','1','yes','on',''.The ratchet test reads
.env.productionthrough Node'sfs, not a Vite?rawimport —Vite's own
server.fs.denycovers.env*by design, and that protection was not weakened tomake a test pass. It lives in
packages/app-shellbecauseapps/console's tsconfig isbrowser-only while app-shell's carries
["node", "vite/client"].The hosted SaaS/demo console must inject
VITE_SENTRY_DSNfrom its Vercel projectenvironment, the same way
VITE_SERVER_URLalready is, or it builds with error reportingoff. Add
VITE_SENTRY_SEND_DEFAULT_PII=trueif IP/User-Agent on events is still wanted —that one is a genuine behaviour change, opt-out became opt-in. It is a deploy-dashboard
action, outside this repo.
What this does not close
An opted-in build still has no post-build off switch, which is the literal wording of the
card's invariant. A runtime-configurable gate needs a new key on
/api/v1/runtime/config—an objectstack contract change, filed upstream with the measurement. The existing payload was
read key by key first and carries nothing usable: reading
cloudUrl === ''as "air-gapped"would be inferring a posture from an unrelated signal, which is the anti-pattern
runtime-config.tswarns against inisMarketplaceEnabled()'s own doc.So #5522 stays open after this merges; the shipped artifact no longer carrying an endpoint
at all is the stop-the-bleed.
Generated by Claude Code