Uh oh!
There was an error while loading. Please reload this page.
feat(app-shell,console): gate Sentry on the runtime's client-telemetry permission - #5982
Merged
Merged
Conversation
…y permission `/api/v1/runtime/config` now carries `telemetry.allowClientErrorReporting` (objectstack#11382), so the Console can finally be silenced by the deployment it lands in rather than only by the build it came from. The shipped decision becomes a conjunction of two independent grants — a DSN injected at build time AND a positive permission from the runtime — and either one denies alone. That is what lets the single pre-built SPA that both the hosted SaaS console and the on-premises / air-gapped EE images embed serve every posture: the identical bundle meets a runtime that grants nothing and stays quiet, with no rebuild and without editing files inside a published SPA. - `runtime-config.ts` parses the key through `grantsClientErrorReporting`, a deliberate line-for-line mirror of `isClientErrorReportingAllowed` from `@objectstack/cloud-connection`. Nothing here depends on that package, so the shape reaches us only by being retyped, exactly as `branding` and `features` already are; the mirror is pinned in both directions. - The permission is REPLACED per payload, never merged like `features` / `branding`, so a grant cannot outlive the response that carried it. - `resolveSentryGate` takes the permission as a REQUIRED parameter. Both spellings fail closed, but only a required one makes the compiler refuse a caller that never considered the question, which is this card's defect class. - The console's boot kick moves after `initRuntimeConfig()` settles. Reading a server value requires waiting for the server; from module-eval time the fail-closed permission read DENIED on every boot and memoized it, which would have turned the switch into a permanent removal. Fails closed on absent key, absent block, malformed payload, failed fetch and runtimes predating the key — precisely the set that is leaking today. Only a real boolean `true` grants. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
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
|
yinlianghui
marked this pull request as ready for review
August 24, 2026 10:36
Uh oh!
There was an error while loading. Please reload this page.
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
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
Reads the runtime's client-telemetry permission off
/api/v1/runtime/configand gatesinitSentryon it, fail-closed. This is the consumer half; the objectui half landed in #5559 and the upstream contract half landed as objectstack-ai/objectstack#11382.Part ofrather than a closing keyword — deliberately, see What would close the card below. The invariant is restored in code, but a live credential named by the card is still outstanding and that is a maintainer action. Flipping this to a closing keyword is a one-line change if the reviewer disagrees; a wrongly-closed p0 is not as cheap to undo.The gate is now a conjunction
Both are opt-in and either one denies alone. That is what finally lets one artifact serve every posture:
@object-ui/consolepublishes a single pre-built SPA that the hosted SaaS console and the on-prem / air-gapped EE images all embed, so the bundle cannot tell those deployments apart — only the server can. An air-gapped EE Console was measured sending 14 Sentry envelopes per session tosentry.iocarrying IP + User-Agent PII with no way for the customer to stop it (objectstack-ai/cloud#1508).The server half is a permission, never a source: it supplies no DSN and cannot switch telemetry on for a build that carries none.
PM assumption 1 was falsified — there is no pin, and that is load-bearing
The dispatch asked whether objectui's pin resolves to a version carrying the new key. It cannot, in either direction. Measured, not assumed:
package.jsonin this repo names@objectstack/cloud-connection— the package that serves the payload and ownsRuntimeTelemetryPosture.@objectstack/specnor@objectstack/client, which we do pin at^17.0.0, depends on or re-exports it.The payload shape reaches this repo only by being retyped, exactly as
brandingandfeaturesalready are inruntime-config.ts. So no version bump can hand us the key and no pin lag can withhold it. No pin was bumped.That changes what the risk actually is: not pin lag but mirror drift.
grantsClientErrorReportingis therefore a deliberate line-for-line mirror ofisClientErrorReportingAllowedfrom@objectstack/cloud-connection/telemetry-posture, named as such at its definition, and pinned in both directions by test.Fail-closed, and why each state is pinned separately
Absent key,
telemetryblock absent, malformed payload, failed fetch, and runtimes predating the key all read do not send — precisely the set of runtimes leaking today. They arrive through four different code paths (early return, absent key, absent block,catch) and share only their answer, so each is pinned on its own rather than represented by one case.Only a real boolean
truegrants;'true',1,'yes'do not. The permission is replaced per payload, never merged likefeatures/branding, so a grant cannot outlive the response that carried it.resolveSentryGatetakes the permission as a required parameter. Both spellings fail closed, but only a required one makes the compiler refuse a caller that never considered the question — and that is this card's entire defect class. It is package-internal (not exported frompackages/app-shell/src/index.ts), so no published signature changed.apps/console/src/main.tsxThe dispatch fenced this to
packages/app-shell/src/observability/**+ the runtime-config reader + a changeset. One line outside that fence was load-bearing and I took it, declaring it here rather than shipping a vacuous gate.void initSentry()ran at module-eval time, beforeinitRuntimeConfig()was even started. Reading a server value requires waiting for the server: from there the fail-closed permission would read DENIED on every boot and memoize that verdict — turning the switch this card asks for into a permanent removal of telemetry, silently, including for the hosted console. The call now sits inside the existing.finally()that already awaits the config before first paint..finally()(not.then()) preserves the pre-existing guarantee that a failed config fetch never blocks boot — and on that path the permission is denied, so the failure direction is silence.initSentryhas exactly one call site repo-wide, so this is the whole ordering surface. The static import set ofmain.tsxis byte-identical before and after (only a call expression moved), which is why the eager-closure budget cannot move — see verification.Verification
Union re-run after the final commit, at
ca91e85ef:vitest run(5 files, path-filtered)Test Files 5 passed (5)/Tests 93 passed (93)type-checkapp-shell + consolepackages/app-shell type-check: Done·apps/console type-check: Donecheck:control-bytes✅ OK (scanned 4950 tracked text file(s))check:phantom-deps✅ Every in-scope import is declared by the package that publishes it.check:self-import✅ No package names itself inside its own src/.check:spec-symbols✅ spec alignment claims: 2 declared deliberate copiescheck-changeset-presence✅ 6 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-major/check-changeset-fixed✅botheslint(6 changed.ts/.tsx)no-explicit-any, the idiom already in these files; CI sets no--max-warningsby deliberate policyHeavy steps ran through the shared verify lock.
packages/app-shell's whole-package suite (~784 s) was not run; the run above is path-filtered, as dispatched.Two ablations, each mutation confirmed on disk by grep counts of the removed and injected text, each with a
trap ... EXIT INT TERMrestore, each restore confirmed byte-identical bycmp:resolveSentryGateTests 3 failed | 17 passed (20); marker count 1 → 0 on disk=== true→!== false(the classic fail-open dialect)Tests 2 failed | 24 passed (26); strict-marker 2 → 1, injected-marker 0 → 1No rebuild was needed for either leg: both suites import the module under test by relative source path, not through a package
exportsfield pointing atdist/. The ablations going red is itself the evidence for that — a suite resolving a staledist/would have stayed green.check:eager-closurewas not run: it needsapps/console/dist/eager-closure.json, a console SPA build artifact CI produces. Narrowed with proof rather than skipped — the static import set of the one changed file is byte-identical, so the eager closure has nothing to move.Every negative assertion is paired with a counter-probe that must stay green (a granting runtime + injected DSN still reports; the payload that denies is proven to have actually parsed). Absence is the shape a broken test reproduces perfectly, so the negatives are only evidence while the counter-probes hold.
Behaviour change for deployments that already inject a DSN
Reporting now also requires the runtime to grant permission, via
OS_TELEMETRY_CLIENT_ERROR_REPORTING_ENABLEDorRuntimeConfigPlugin'sallowClientErrorReporting. A build that opted in but whose runtime says nothing will go quiet — deliberately, since that is the same artifact an air-gapped customer runs.What would close the card (maintainer actions, not mine)
apps/console/.env.productionbefore console/app-shell: stop shipping an un-disableable Sentry DSN, and make sendDefaultPii opt-in (#5522) #5559 removed it. It is still in git history and presumably still live. Per dispatch I neither invented nor rotated a credential.VITE_SENTRY_DSN— otherwise the hosted console ships with error reporting off after #5546 #5550.Generated by Claude Code