diff --git a/.changeset/5522-runtime-telemetry-consumer.md b/.changeset/5522-runtime-telemetry-consumer.md new file mode 100644 index 0000000000..7e626d0bab --- /dev/null +++ b/.changeset/5522-runtime-telemetry-consumer.md @@ -0,0 +1,37 @@ +--- +'@object-ui/app-shell': minor +'@object-ui/console': minor +--- + +Console telemetry can now be hard-disabled on an already-built artifact + +`/api/v1/runtime/config` gained `telemetry.allowClientErrorReporting` +(objectstack#11382), and the Console now reads it. The Sentry decision becomes a +conjunction of two independent grants — a DSN injected at **build** time AND a +positive permission from the **runtime** — so the single pre-built SPA that both +the hosted SaaS console and the on-premises / air-gapped EE images embed can be +silenced by the deployment it lands in, with no rebuild and without editing files +inside a published bundle. That was the half objectui#5522 could not close before: +every other input to the gate is a Vite build-time variable frozen into the bundle +as a literal, which is how an air-gapped EE Console came to send 14 Sentry +envelopes per session to `sentry.io` carrying IP + User-Agent PII with no way for +the customer to turn it off (objectstack-ai/cloud#1508). + +The permission fails **closed** in every direction: absent key, `telemetry` block +absent, malformed payload, failed fetch, or a runtime predating the key all read as +*do not send* — which is precisely the set of runtimes leaking today. It is a +permission and never a source: the server supplies no DSN and cannot turn telemetry +on for a build that carries none. Only a real boolean `true` grants; `'true'`, `1` +and other truthy lookalikes do not. + +Behaviour change for deployments that already inject a DSN: reporting now also +requires the runtime to grant permission, via +`OS_TELEMETRY_CLIENT_ERROR_REPORTING_ENABLED` (or `RuntimeConfigPlugin`'s +`allowClientErrorReporting`). 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. + +`@object-ui/app-shell` additionally exports `isClientErrorReportingAllowed()` and +the `RuntimeTelemetry` type, so consumers read the permission through the one +fail-closed accessor instead of writing their own optional-chain against the +payload. diff --git a/apps/console/src/main.tsx b/apps/console/src/main.tsx index 22ddeb18d2..7913a705c8 100644 --- a/apps/console/src/main.tsx +++ b/apps/console/src/main.tsx @@ -19,10 +19,6 @@ import { preflightAuth } from './lib/auth-preflight'; const AUTH_URL = `${import.meta.env.VITE_SERVER_URL || ''}/api/v1/auth`; -// Kick off Sentry init in the background (no-op if VITE_SENTRY_DSN is unset). -// Not awaited — observability must never block first paint. -void initSentry(); - // ──────────────────────────────────────────────────────────────────────────── // Plugin registration // ──────────────────────────────────────────────────────────────────────────── @@ -79,6 +75,21 @@ Promise.all([ preflightAuth(AUTH_URL), seedTenantLanguage(SERVER_BASE), ]).finally(() => { + // Kick off Sentry init (no-op unless a DSN was injected at build time AND + // this runtime granted `telemetry.allowClientErrorReporting`). Still not + // awaited — observability must never block first paint. + // + // ⛔ Ordering is load-bearing, not stylistic: this call used to run at + // module-eval time, BEFORE `initRuntimeConfig()` was even started. The + // server-pushed telemetry permission fails closed, so from there it would + // read DENIED on every boot and memoize that verdict — turning the switch + // objectui#5522 asked for into a permanent removal, silently, including for + // the hosted console. Reading a server value requires waiting for the + // server. `.finally()` (not `.then()`) keeps 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. + void initSentry(); + // Apply runtime branding before React mounts — avoids a flash of the // static defaults for operators who configure OS_PRODUCT_NAME etc. document.title = getProductName(); diff --git a/packages/app-shell/src/index.ts b/packages/app-shell/src/index.ts index 4078a06800..e353c85436 100644 --- a/packages/app-shell/src/index.ts +++ b/packages/app-shell/src/index.ts @@ -146,9 +146,13 @@ export { getPwaDescription, getPwaThemeColor, isRuntimeConfigInitialised, + // The fail-closed reading of the runtime's client-telemetry permission. + // Exported so no consumer has to write its own `?.` chain against the + // payload — one `!== false` dialect is all it takes to re-open objectui#5522. + isClientErrorReportingAllowed, resetRuntimeConfigForTesting, } from './runtime-config.js'; -export type { AppShellRuntimeConfig, RuntimeFeatures, RuntimeBranding, PlatformStage } from './runtime-config.js'; +export type { AppShellRuntimeConfig, RuntimeFeatures, RuntimeBranding, RuntimeTelemetry, PlatformStage } from './runtime-config.js'; // Standard inner-SPA views export { diff --git a/packages/app-shell/src/observability/sentry.test.ts b/packages/app-shell/src/observability/sentry.test.ts index 5ed2dd499c..f777760e60 100644 --- a/packages/app-shell/src/observability/sentry.test.ts +++ b/packages/app-shell/src/observability/sentry.test.ts @@ -26,13 +26,27 @@ * covered below for the posture that ships by default, and the enabled path * is covered end-to-end by the production-build counter-probe recorded on the * pull request. + * + * The gate now takes TWO grants — a build-time DSN and the runtime's + * `telemetry.allowClientErrorReporting` permission — so each half is pinned + * against a GRANTING counterpart, never against a second denial. Testing "no + * DSN and no permission ⇒ silence" would prove nothing about either. */ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; import { resolveSentryGate } from './sentry'; +import { isClientErrorReportingAllowed, resetRuntimeConfigForTesting } from '../runtime-config.js'; const DSN = 'https://examplePublicKey@o0.ingest.sentry.io/0'; +/** + * The runtime's answer, named rather than spelled `true`/`false` at 30 call + * sites: `resolveSentryGate(env, false)` reads as "some boolean", while + * `RUNTIME_GRANTS` states which of the two grants a case is holding fixed. + */ +const RUNTIME_GRANTS = true; +const RUNTIME_DENIES = false; + const sentryMock = vi.hoisted(() => ({ init: vi.fn(), captureException: vi.fn(), @@ -42,30 +56,36 @@ const sentryMock = vi.hoisted(() => ({ vi.mock('@sentry/react', () => sentryMock); -describe('resolveSentryGate — fails closed when the opt-in signal is absent', () => { +describe('resolveSentryGate — fails closed when the build-time opt-in is absent', () => { + // Every case here holds the RUNTIME grant fixed at "allowed", so a denial can + // only be coming from the build-time half. Pairing two denials would let + // either one carry the result while the other rotted. + it('withholds reporting when no DSN was injected', () => { - expect(resolveSentryGate({})).toMatchObject({ enabled: false, reason: 'no-dsn' }); + expect(resolveSentryGate({}, RUNTIME_GRANTS)).toMatchObject({ enabled: false, reason: 'no-dsn' }); }); it('withholds reporting when the env object itself is missing', () => { // `(import.meta as any).env` can legitimately be undefined outside Vite. // "Cannot determine the signal" must land on silence, not on send. - expect(resolveSentryGate(undefined)).toMatchObject({ enabled: false, reason: 'no-dsn' }); - expect(resolveSentryGate(null)).toMatchObject({ enabled: false, reason: 'no-dsn' }); + expect(resolveSentryGate(undefined, RUNTIME_GRANTS)).toMatchObject({ enabled: false, reason: 'no-dsn' }); + expect(resolveSentryGate(null, RUNTIME_GRANTS)).toMatchObject({ enabled: false, reason: 'no-dsn' }); }); it('treats an empty or whitespace-only DSN as absent', () => { - expect(resolveSentryGate({ VITE_SENTRY_DSN: '' })).toMatchObject({ enabled: false }); - expect(resolveSentryGate({ VITE_SENTRY_DSN: ' ' })).toMatchObject({ enabled: false }); + expect(resolveSentryGate({ VITE_SENTRY_DSN: '' }, RUNTIME_GRANTS)).toMatchObject({ enabled: false }); + expect(resolveSentryGate({ VITE_SENTRY_DSN: ' ' }, RUNTIME_GRANTS)).toMatchObject({ enabled: false }); }); it('treats a non-string DSN as absent rather than coercing it', () => { - expect(resolveSentryGate({ VITE_SENTRY_DSN: true })).toMatchObject({ enabled: false }); - expect(resolveSentryGate({ VITE_SENTRY_DSN: 1 })).toMatchObject({ enabled: false }); + expect(resolveSentryGate({ VITE_SENTRY_DSN: true }, RUNTIME_GRANTS)).toMatchObject({ enabled: false }); + expect(resolveSentryGate({ VITE_SENTRY_DSN: 1 }, RUNTIME_GRANTS)).toMatchObject({ enabled: false }); }); it('honours the explicit force-off even when a DSN was injected', () => { - expect(resolveSentryGate({ VITE_SENTRY_DSN: DSN, VITE_SENTRY_ENABLED: 'false' })).toMatchObject({ + expect( + resolveSentryGate({ VITE_SENTRY_DSN: DSN, VITE_SENTRY_ENABLED: 'false' }, RUNTIME_GRANTS), + ).toMatchObject({ enabled: false, reason: 'forced-off', }); @@ -79,20 +99,74 @@ describe('resolveSentryGate — fails closed when the opt-in signal is absent', { VITE_SENTRY_SEND_DEFAULT_PII: 'true' }, { VITE_SENTRY_DSN: DSN, VITE_SENTRY_ENABLED: 'false', VITE_SENTRY_SEND_DEFAULT_PII: 'true' }, ]) { - const decision = resolveSentryGate(env); + const decision = resolveSentryGate(env, RUNTIME_GRANTS); expect(decision.enabled).toBe(false); expect(decision.sendDefaultPii).toBe(false); } }); }); +/** + * The post-build off switch (objectui#5522 / objectstack#10805, cloud#1508). + * + * The half that could not be built before: every other input to this gate is a + * Vite build-time variable frozen into the bundle, so an air-gapped EE Console + * running the SAME artifact as the hosted console had no way to be silenced. + * These cases hold the BUILD-time grant fixed at "fully opted in" — a real DSN, + * no force-off — so a denial can only be coming from the runtime. + */ +describe('resolveSentryGate — the runtime permission can silence a build that opted in', () => { + it('withholds reporting when the runtime declines, DSN notwithstanding', () => { + expect(resolveSentryGate({ VITE_SENTRY_DSN: DSN }, RUNTIME_DENIES)).toMatchObject({ + enabled: false, + reason: 'runtime-denied', + }); + }); + + it('still reports the DSN it refused, so a silent deployment is diagnosable', () => { + // The operator's question is "why is nothing arriving" — `reason` has to + // distinguish "you shipped no DSN" from "your runtime said no", and the + // second is invisible from inside the artifact. + expect(resolveSentryGate({ VITE_SENTRY_DSN: DSN }, RUNTIME_DENIES).dsn).toBe(DSN); + }); + + it('sends no PII on a runtime denial even when the build asked for it', () => { + const decision = resolveSentryGate( + { VITE_SENTRY_DSN: DSN, VITE_SENTRY_SEND_DEFAULT_PII: 'true' }, + RUNTIME_DENIES, + ); + expect(decision.enabled).toBe(false); + expect(decision.sendDefaultPii).toBe(false); + }); + + it('requires a real `true`, so no truthy value can grant by accident', () => { + // `!== true` rather than `!`. The parameter is typed `boolean`, but this + // gate is the last thing standing between an air-gapped network and + // sentry.io, and JS callers exist. A permission must be granted, never + // coerced. + for (const truthy of ['true', 1, 'yes', {}, [], 'granted']) { + expect( + resolveSentryGate({ VITE_SENTRY_DSN: DSN }, truthy as unknown as boolean).enabled, + `runtime permission ${JSON.stringify(truthy)} must not grant`, + ).toBe(false); + } + }); + + it('denies on BOTH halves missing without masking either reason', () => { + // Order matters only for the diagnostic: no DSN is the more actionable + // answer, so it wins the `reason` slot when both are absent. + expect(resolveSentryGate({}, RUNTIME_DENIES)).toMatchObject({ enabled: false, reason: 'no-dsn' }); + }); +}); + describe('counter-probe — a posture that SHOULD report still does', () => { - it('grants reporting when a DSN was deliberately injected at build time', () => { + it('grants reporting when a DSN was injected at build time AND the runtime allows it', () => { // The whole point: the fix must not silently disable the hosted SaaS - // build, which opts in by injecting a DSN in its own deploy environment. - // If this case ever goes red, the absence assertions above stop meaning - // "the gate is careful" and start meaning "the gate is stuck shut". - expect(resolveSentryGate({ VITE_SENTRY_DSN: DSN })).toMatchObject({ + // build, which opts in by injecting a DSN in its own deploy environment + // and runs on a runtime that grants the permission. If this case ever goes + // red, the absence assertions above stop meaning "the gate is careful" and + // start meaning "the gate is stuck shut". + expect(resolveSentryGate({ VITE_SENTRY_DSN: DSN }, RUNTIME_GRANTS)).toMatchObject({ enabled: true, reason: 'opted-in', dsn: DSN, @@ -100,28 +174,28 @@ describe('counter-probe — a posture that SHOULD report still does', () => { }); it('does NOT require a separate enable flag alongside the DSN', () => { - // Presence of the DSN is the opt-in. Were `VITE_SENTRY_ENABLED=true` ever - // made mandatory, the SaaS pipeline would go dark the moment it forgot + // Presence of the DSN is the build-time opt-in. Were `VITE_SENTRY_ENABLED=true` + // ever made mandatory, the SaaS pipeline would go dark the moment it forgot // the second variable — a quiet failure, which is the direction this card // exists to avoid. - expect(resolveSentryGate({ VITE_SENTRY_DSN: DSN, VITE_SENTRY_ENABLED: undefined }).enabled).toBe(true); - expect(resolveSentryGate({ VITE_SENTRY_DSN: DSN, VITE_SENTRY_ENABLED: '' }).enabled).toBe(true); - expect(resolveSentryGate({ VITE_SENTRY_DSN: DSN, VITE_SENTRY_ENABLED: 'true' }).enabled).toBe(true); + expect(resolveSentryGate({ VITE_SENTRY_DSN: DSN, VITE_SENTRY_ENABLED: undefined }, RUNTIME_GRANTS).enabled).toBe(true); + expect(resolveSentryGate({ VITE_SENTRY_DSN: DSN, VITE_SENTRY_ENABLED: '' }, RUNTIME_GRANTS).enabled).toBe(true); + expect(resolveSentryGate({ VITE_SENTRY_DSN: DSN, VITE_SENTRY_ENABLED: 'true' }, RUNTIME_GRANTS).enabled).toBe(true); }); it('trims a padded DSN rather than rejecting it', () => { - expect(resolveSentryGate({ VITE_SENTRY_DSN: ` ${DSN} ` })).toMatchObject({ enabled: true, dsn: DSN }); + expect(resolveSentryGate({ VITE_SENTRY_DSN: ` ${DSN} ` }, RUNTIME_GRANTS)).toMatchObject({ enabled: true, dsn: DSN }); }); }); describe('sendDefaultPii — opt-in, because one artifact ships to every posture', () => { it('is OFF when the build said nothing about it', () => { - expect(resolveSentryGate({ VITE_SENTRY_DSN: DSN }).sendDefaultPii).toBe(false); + expect(resolveSentryGate({ VITE_SENTRY_DSN: DSN }, RUNTIME_GRANTS).sendDefaultPii).toBe(false); }); it('is ON only when the build explicitly asked for it', () => { expect( - resolveSentryGate({ VITE_SENTRY_DSN: DSN, VITE_SENTRY_SEND_DEFAULT_PII: 'true' }).sendDefaultPii, + resolveSentryGate({ VITE_SENTRY_DSN: DSN, VITE_SENTRY_SEND_DEFAULT_PII: 'true' }, RUNTIME_GRANTS).sendDefaultPii, ).toBe(true); }); @@ -130,7 +204,7 @@ describe('sendDefaultPii — opt-in, because one artifact ships to every posture // These are the values that flipped meaning; none of them may enable PII. for (const value of ['false', 'TRUE', 'True', '1', 'yes', 'on', '']) { expect( - resolveSentryGate({ VITE_SENTRY_DSN: DSN, VITE_SENTRY_SEND_DEFAULT_PII: value }).sendDefaultPii, + resolveSentryGate({ VITE_SENTRY_DSN: DSN, VITE_SENTRY_SEND_DEFAULT_PII: value }, RUNTIME_GRANTS).sendDefaultPii, `VITE_SENTRY_SEND_DEFAULT_PII=${JSON.stringify(value)} must not enable PII`, ).toBe(false); } @@ -138,6 +212,10 @@ describe('sendDefaultPii — opt-in, because one artifact ships to every posture }); describe('initSentry — the posture that actually ships', () => { + afterEach(() => { + resetRuntimeConfigForTesting(); + }); + it('does not initialize, and does not load the SDK, on a build with no DSN', async () => { // `import.meta.env` under Vitest carries no VITE_SENTRY_DSN, which is // exactly the shape of a console build that never opted in. Not merely @@ -150,11 +228,20 @@ describe('initSentry — the posture that actually ships', () => { expect(getSentry()).toBeNull(); }); - it('agrees with the pure gate about the env it actually reads', () => { - // Ties the two halves together: whatever `import.meta.env` holds in this - // run, `initSentry`'s verdict above must be the one `resolveSentryGate` - // derives from it. Without this, the pure tests and the wiring test could - // drift apart and both stay green. - expect(resolveSentryGate((import.meta as any).env).enabled).toBe(false); + it('agrees with the pure gate about the two inputs it actually reads', () => { + // Ties the halves together: whatever `import.meta.env` and the runtime + // config singleton hold in this run, `initSentry`'s verdict above must be + // the one `resolveSentryGate` derives from them. Without this, the pure + // tests and the wiring test could drift apart and both stay green. + expect(resolveSentryGate((import.meta as any).env, isClientErrorReportingAllowed()).enabled).toBe(false); + }); + + it('reads the runtime permission as DENIED before any config has been fetched', () => { + // The state `initSentry` sees if it is ever called before + // `initRuntimeConfig()` settles. It must be denial: the console's boot + // ordering is what guarantees the permission has arrived, and the cost of + // getting that ordering wrong has to be silence, never a leak. + resetRuntimeConfigForTesting(); + expect(isClientErrorReportingAllowed()).toBe(false); }); }); diff --git a/packages/app-shell/src/observability/sentry.ts b/packages/app-shell/src/observability/sentry.ts index 6fe8a24f85..5b050dbf4d 100644 --- a/packages/app-shell/src/observability/sentry.ts +++ b/packages/app-shell/src/observability/sentry.ts @@ -31,16 +31,41 @@ * commits NO DSN; deployments that want reporting inject one in their own * deploy environment (a ratchet test keeps it that way). * - * ⚠️ Known limitation, deliberately not worked around here: a build that DID - * opt in still has no post-build off switch, because the only server→SPA - * channel is `/api/v1/runtime/config` and a telemetry key on that payload is - * an objectstack contract change, not objectui's to make. Filed upstream; see - * the issue for the fork. Until it lands, "do not inject a DSN" is the - * off switch, and it is now a real one because none is committed. + * ## The post-build off switch (the second conjunct) + * + * The limitation this module used to record here — "a build that DID opt in + * still has no post-build off switch" — is now closed. The upstream contract + * change it was waiting on landed as objectstack#11382, so + * `/api/v1/runtime/config` carries `telemetry.allowClientErrorReporting`, and + * {@link initSentry} reads it through + * {@link isClientErrorReportingAllowed}. + * + * That makes the shipped decision a CONJUNCTION of two independent grants: + * + * ``` + * send ⇔ a DSN was injected at BUILD time ∧ the RUNTIME granted permission + * ``` + * + * Both are opt-in and either one denies alone, which is what finally lets one + * artifact serve every posture: the hosted SaaS console injects a DSN *and* + * runs on a runtime that grants, while the identical bundle inside an + * air-gapped EE image meets a runtime that grants nothing and stays silent — + * with no rebuild, and without anyone editing files inside a published SPA. + * + * The server half is a PERMISSION, never a source: it supplies no DSN and + * cannot switch telemetry ON for a build that carries none. A server able to + * *start* a third-party data flow in someone's browser would be a strictly + * worse surface than the one this card fixes. + * + * Runtime config consumed: + * - `telemetry.allowClientErrorReporting` on `/api/v1/runtime/config` — the + * deployment's permission. Absent/denied/unreachable ⇒ do not send. * * Env vars consumed (all optional): * - `VITE_SENTRY_DSN` — DSN; absent disables the integration entirely. - * Presence IS the opt-in — there is no separate "enable" flag to forget. + * Presence is the BUILD-time half of the opt-in — there is no separate + * "enable" flag to forget — but it no longer suffices on its own: the + * runtime must also grant permission (see above). * - `VITE_SENTRY_ENABLED` — set to `"false"` to force-disable reporting * even when a DSN was injected. An ADDITIONAL off switch for a pipeline * that wants to keep the DSN in its environment but stop reporting; it is @@ -57,6 +82,8 @@ * @module */ +import { isClientErrorReportingAllowed } from '../runtime-config.js'; + type SentryModule = typeof import('@sentry/react'); let sentryModule: SentryModule | null = null; @@ -67,7 +94,7 @@ export interface SentryGateDecision { /** Whether reporting may start at all. */ enabled: boolean; /** Why — useful in tests and when explaining a silent deployment. */ - reason: 'no-dsn' | 'forced-off' | 'opted-in'; + reason: 'no-dsn' | 'forced-off' | 'runtime-denied' | 'opted-in'; /** The trimmed DSN, or `''` when there is none. */ dsn: string; /** Whether IP address + User-Agent may be attached to events. */ @@ -75,7 +102,8 @@ export interface SentryGateDecision { } /** - * The whole telemetry decision, as a pure function of the build-time env. + * The whole telemetry decision, as a pure function of its two inputs: what the + * build was compiled with, and what the runtime permits. * * Split out from {@link initSentry} deliberately. The decision is the part * with the security consequence, and leaving it inline made it unreachable @@ -84,11 +112,24 @@ export interface SentryGateDecision { * WITHOUT reaching `import.meta.env` (measured — a suite that stubbed a DSN * and asserted "enabled" failed, because the module never saw it). An * untestable gate is how the previous one stayed broken; this one is pinned - * case by case in `sentry.test.ts`. + * case by case in `sentry.test.ts`. Keeping the runtime permission INSIDE this + * function rather than adding a second gate at the call site is the same + * argument applied once more: one decision, one place, one suite. + * + * `runtimeAllowsClientErrorReporting` is REQUIRED, not an optional argument + * defaulting to `false`. Both spellings fail closed, but only a required + * parameter makes the compiler refuse a caller that never considered the + * question — and "a caller that never considered the question" is this card's + * entire defect class. Callers read it from + * {@link isClientErrorReportingAllowed}, which owns the fail-closed reading of + * the payload. * * Fails CLOSED in every branch that is not an affirmative opt-in. */ -export function resolveSentryGate(env: Record | null | undefined): SentryGateDecision { +export function resolveSentryGate( + env: Record | null | undefined, + runtimeAllowsClientErrorReporting: boolean, +): SentryGateDecision { const rawDsn = env?.VITE_SENTRY_DSN; const dsn = typeof rawDsn === 'string' ? rawDsn.trim() : ''; @@ -105,6 +146,17 @@ export function resolveSentryGate(env: Record | null | undefine return { enabled: false, reason: 'forced-off', dsn, sendDefaultPii: false }; } + // The runtime's post-build permission — the one input here that a shipped + // artifact cannot have frozen into itself. A deployment that declines beats + // a DSN someone compiled in, which is the whole point: the air-gapped EE + // image runs the SAME bundle as the hosted console, so the build-time + // signals cannot tell them apart and only the server can. + // + // `!== true`, not `!`: a truthy non-boolean must not be able to grant. + if (runtimeAllowsClientErrorReporting !== true) { + return { enabled: false, reason: 'runtime-denied', dsn, sendDefaultPii: false }; + } + return { enabled: true, reason: 'opted-in', @@ -134,7 +186,13 @@ export function initSentry(): Promise { initPromise = (async () => { const env = (import.meta as any).env ?? {}; - const gate = resolveSentryGate(env); + // Read at init time, not at module-eval time: this is a server-pushed + // value, so it is only meaningful once `initRuntimeConfig()` has settled. + // Until then — and if the fetch failed, or the runtime predates the key — + // it reads DENIED, so an `initSentry()` that runs too early withholds + // telemetry rather than granting it. Ordering is the caller's to get + // right; the failure mode of getting it wrong is silence, not a leak. + const gate = resolveSentryGate(env, isClientErrorReportingAllowed()); // Returning BEFORE the dynamic import is load-bearing, not an early-exit // micro-optimisation: it keeps the vendor-sentry chunk unfetched, so a // deployment that never opted in issues no third-party request at all — diff --git a/packages/app-shell/src/runtime-config.test.ts b/packages/app-shell/src/runtime-config.test.ts index 9f0df0d199..14f4ae464d 100644 --- a/packages/app-shell/src/runtime-config.test.ts +++ b/packages/app-shell/src/runtime-config.test.ts @@ -9,7 +9,7 @@ */ import { describe, it, expect, afterEach, vi } from 'vitest'; -import { initRuntimeConfig, getRuntimeConfig, getPlatformStage, isAiStudioEnabled, isMarketplaceEnabled, resetRuntimeConfigForTesting } from './runtime-config.js'; +import { initRuntimeConfig, getRuntimeConfig, getPlatformStage, isAiStudioEnabled, isMarketplaceEnabled, isClientErrorReportingAllowed, resetRuntimeConfigForTesting } from './runtime-config.js'; function mockConfig(features: Record) { vi.stubGlobal('fetch', vi.fn(async () => ({ @@ -178,3 +178,130 @@ describe('runtime-config platform stage', () => { expect(getPlatformStage()).toBe('preview'); }); }); + + +/** + * `telemetry.allowClientErrorReporting` — the post-build off switch + * (objectui#5522 / objectstack#10805, upstream half of cloud#1508). + * + * ## Why this suite is stricter than its neighbours above + * + * `customDomain` / `sso` withhold a PAID surface and `marketplace` / `aiStudio` + * deliberately fail OPEN. This one withholds an OUTBOUND THIRD-PARTY REQUEST + * from inside customer networks: an air-gapped on-prem EE Console was measured + * sending 14 Sentry envelopes per session to `sentry.io` carrying IP + + * User-Agent PII. So every "cannot determine the answer" state must land on + * DENIED, and each is pinned separately below rather than represented by one + * case — they arrive through different code paths (early return, absent key, + * absent block, `catch`) and only their ANSWER is shared. + * + * ## This is a mirror, and drift is the risk it is guarding + * + * `grantsClientErrorReporting` in `runtime-config.ts` is a hand copy of + * `isClientErrorReportingAllowed` from + * `@objectstack/cloud-connection/telemetry-posture`. That is not a shortcut: + * this repo has NO dependency on that package (nothing here names it, and + * neither `@objectstack/spec` nor `@objectstack/client`, which we do pin, + * re-export it), exactly as `branding` and `features` are mirrored above. No + * version bump can hand us the key and no pin lag can withhold it — the only + * failure mode available is the two readings drifting apart, so the table + * below pins the producer's documented semantics verbatim. + */ +function mockBody(body: unknown) { + vi.stubGlobal('fetch', vi.fn(async () => ({ + ok: true, + json: async () => body, + })) as any); +} + +describe('runtime-config client-telemetry permission', () => { + it('is DENIED before init — the state a too-early caller sees', () => { + resetRuntimeConfigForTesting(); + expect(isClientErrorReportingAllowed()).toBe(false); + expect(getRuntimeConfig().telemetry.allowClientErrorReporting).toBe(false); + }); + + it('is DENIED on a runtime that predates the key', async () => { + // The population this switch exists for: every deployment leaking + // today is running a server that has never heard of it. + mockBody({ features: { aiStudio: true }, branding: { productName: 'Acme' } }); + await initRuntimeConfig(); + expect(isClientErrorReportingAllowed()).toBe(false); + // Counter-probe: the payload WAS parsed, so the denial above is the + // gate answering and not the fetch having quietly done nothing. + expect(getRuntimeConfig().branding.productName).toBe('Acme'); + }); + + it('is DENIED when the telemetry block is present but empty', async () => { + mockBody({ telemetry: {} }); + await initRuntimeConfig(); + expect(isClientErrorReportingAllowed()).toBe(false); + }); + + it('is DENIED when the fetch fails outright', async () => { + vi.stubGlobal('fetch', vi.fn(async () => { + throw new Error('air-gapped: no route to control plane'); + }) as any); + await initRuntimeConfig(); + expect(isClientErrorReportingAllowed()).toBe(false); + }); + + it('is DENIED on a malformed payload', async () => { + for (const body of [null, 'nonsense', 42, []]) { + resetRuntimeConfigForTesting(); + mockBody(body); + await initRuntimeConfig(); + expect(isClientErrorReportingAllowed(), `payload ${JSON.stringify(body)} must not grant`).toBe(false); + } + }); + + it('is GRANTED when the runtime positively says so', async () => { + // The counter-probe for the whole suite. Without it every assertion + // above is satisfied by a gate that is simply stuck shut, which is the + // one failure this file cannot otherwise see. + mockBody({ telemetry: { allowClientErrorReporting: true } }); + await initRuntimeConfig(); + expect(isClientErrorReportingAllowed()).toBe(true); + }); + + it('requires a real boolean `true`, never a truthy lookalike', async () => { + // `=== true`, mirroring the producer. `'true'`, `1` and `'yes'` are + // payloads a consumer should not teach itself to accept. + for (const value of ['true', 1, 'yes', 'on', {}, []]) { + resetRuntimeConfigForTesting(); + mockBody({ telemetry: { allowClientErrorReporting: value } }); + await initRuntimeConfig(); + expect(isClientErrorReportingAllowed(), `${JSON.stringify(value)} must not grant`).toBe(false); + } + }); + + it('is DENIED when an explicit false arrives', async () => { + mockBody({ telemetry: { allowClientErrorReporting: false } }); + await initRuntimeConfig(); + expect(isClientErrorReportingAllowed()).toBe(false); + }); + + it('does NOT accept the permission from the open-ended `features` map', async () => { + // The producer keeps this key in its own namespace precisely because a + // host's `resolveFeatures` hook merges arbitrary keys into `features` + // verbatim — so a distribution could otherwise grant a security + // permission from code whose subject is billing tiers. Pinned on both + // sides; this is our half. + mockBody({ features: { allowClientErrorReporting: true } }); + await initRuntimeConfig(); + expect(isClientErrorReportingAllowed()).toBe(false); + }); + + it('withdraws a previous grant when a later fetch no longer carries it', async () => { + // `telemetry` is REPLACED per payload, not merged like `features` / + // `branding`. A permission that outlived the response that carried it + // would let a re-fetch against a withdrawing runtime keep sending. + mockBody({ telemetry: { allowClientErrorReporting: true } }); + await initRuntimeConfig(); + expect(isClientErrorReportingAllowed()).toBe(true); + + mockBody({ telemetry: { allowClientErrorReporting: false } }); + await initRuntimeConfig(); + expect(isClientErrorReportingAllowed()).toBe(false); + }); +}); diff --git a/packages/app-shell/src/runtime-config.ts b/packages/app-shell/src/runtime-config.ts index fdc32b469e..f740f1865a 100644 --- a/packages/app-shell/src/runtime-config.ts +++ b/packages/app-shell/src/runtime-config.ts @@ -97,6 +97,66 @@ export interface RuntimeBranding { pwaThemeColor?: string; } +/** + * The runtime's post-build permission for SPA client telemetry + * (objectui#5522 / objectstack#10805, upstream half of cloud#1508). + * + * Its OWN namespace on the payload, deliberately NOT a member of + * {@link RuntimeFeatures}. That map is open-ended — a host's `resolveFeatures` + * hook merges arbitrary keys into it verbatim — so putting the permission + * there would let a distribution grant it from code whose subject is billing + * tiers. A security permission has exactly one author. The producer + * (`RuntimeConfigPlugin` in `@objectstack/cloud-connection`) pins the same + * separation from its side. + */ +export interface RuntimeTelemetry { + /** + * May the SPA send client error reports to the sink its build was compiled + * with? `false` unless a runtime positively granted it. + * + * A PERMISSION, not a source: the server supplies no DSN and cannot turn + * telemetry on for a build that carries none. `true` says only "this + * deployment does not object to the sink you were built with", so the + * composed decision is `Boolean(buildTimeDsn) && thisPermission`. + */ + allowClientErrorReporting: boolean; +} + +/** + * The canonical fail-closed reading of a `/api/v1/runtime/config` payload's + * telemetry permission. + * + * ⚠️ **A MIRROR, and the mirror is the whole risk.** This repo has no + * dependency on `@objectstack/cloud-connection` (the package that serves the + * payload and owns the shape) — measured, not assumed: no `package.json` here + * names it, and neither `@objectstack/spec` nor `@objectstack/client`, which we + * do pin, re-export it. So no version bump can deliver this key to us and no + * pin lag can withhold it; the shape reaches us only by being retyped here, the + * way `branding` and `features` above already are. What CAN go wrong is drift, + * so this function is a deliberate line-for-line mirror of + * `isClientErrorReportingAllowed` exported from + * `@objectstack/cloud-connection/telemetry-posture`, and + * `runtime-config.test.ts` pins it in both directions. + * + * The test is `=== true`, not truthiness: `'true'`, `1` and `'yes'` are + * payloads a consumer should not teach itself to accept. On the wire the + * producer emits a real boolean. + * + * Every "cannot determine the answer" state — a non-object body, an absent + * `telemetry` block, an absent key, a payload from an older ObjectStack or a + * third-party host — collapses onto `false`. That is the point of phrasing the + * key as a positive PERMISSION rather than a negative kill switch: a + * `telemetry: { disabled: true }` spelling would read `undefined` (falsy, + * therefore "not disabled", therefore SEND) on exactly the legacy runtimes that + * are leaking today. + */ +function grantsClientErrorReporting(payload: unknown): boolean { + if (typeof payload !== 'object' || payload === null) return false; + const telemetry = (payload as { telemetry?: unknown }).telemetry; + if (typeof telemetry !== 'object' || telemetry === null) return false; + return (telemetry as { allowClientErrorReporting?: unknown }).allowClientErrorReporting === true; +} + /** * The SPA's server-pushed runtime configuration — which cloud to talk to, which * features are on, and how the product is branded. @@ -121,6 +181,12 @@ export interface AppShellRuntimeConfig { defaultEnvironmentId?: string | null; features: RuntimeFeatures; branding: RuntimeBranding; + /** + * The runtime's post-build permission for client telemetry. Always present + * on the singleton (defaulting to denied) so consumers never have to spell + * the absent case themselves. + */ + telemetry: RuntimeTelemetry; } const defaults: AppShellRuntimeConfig = { @@ -132,6 +198,11 @@ const defaults: AppShellRuntimeConfig = { // `stage: 'preview'` while the whole platform is pre-GA, so the badge shows // out of the box on any runtime that hasn't sent an explicit stage yet. branding: { productName: 'ObjectOS', productShortName: 'ObjectOS', stage: 'preview', brandColor: '#4F46E5', pwaThemeColor: '#4f46e5' }, + // ⛔ Denied until a runtime positively grants it — including before + // `initRuntimeConfig()` has ever run. This default IS the fail-closed + // guarantee for the two states no payload can speak for: "the config has + // not arrived yet" and "the fetch failed". + telemetry: { allowClientErrorReporting: false }, }; /** Valid {@link PlatformStage} values, for validating server-pushed config. */ @@ -157,6 +228,11 @@ function applyUpdate(patch: Partial): void { ...current.branding, ...(patch.branding ?? {}), }, + // NOT merged like `features`/`branding`: a security permission is + // re-derived from each payload in full, so a grant can never outlive the + // response that carried it (a later re-fetch against a runtime that + // withdrew it must withdraw it here too). + telemetry: patch.telemetry ? { ...patch.telemetry } : current.telemetry, }; } @@ -206,6 +282,11 @@ export async function initRuntimeConfig(baseUrl: string = ''): Promise { sso: body.features.sso === true, } : current.features, + // Read off the RAW body, not off `body.telemetry`: the mirrored reader + // owns every malformed/absent shape in one place, and handing it the + // whole payload is what keeps this call site from growing its own + // `?.` dialect. + telemetry: { allowClientErrorReporting: grantsClientErrorReporting(body) }, branding: body.branding ? { productName: @@ -379,12 +460,43 @@ export function isAiStudioEnabled(): boolean { return current.features?.aiStudio !== false; } +/** + * May this deployment's Console send client error reports to the telemetry + * sink its bundle was compiled with? (objectui#5522) + * + * The post-build off switch cloud#1508 asked for: an air-gapped on-premises EE + * Console was measured sending 14 Sentry envelopes per session to `sentry.io` + * carrying IP + User-Agent PII, and could not be silenced because every knob in + * `observability/sentry.ts` is a Vite build-time variable that Vite inlines as + * a frozen literal. `@object-ui/console` publishes ONE pre-built SPA that the + * hosted SaaS console and the on-prem/air-gapped EE images both embed, so the + * artifact cannot tell those postures apart and editing env vars on the + * deployed host does nothing. This value comes from the server on every boot, + * so it CAN. + * + * ⛔ Fails CLOSED (`=== true`), the opposite direction from + * {@link isMarketplaceEnabled} and {@link isAiStudioEnabled} above — do not + * "make it consistent" with them. Those withhold a working capability on an + * unanswered question, which is the worse direction for a feature; here an + * unreported error is recoverable and PII leaving an air-gapped deployment is + * not, so every unanswered question must land on silence. Concretely: a runtime + * predating the key, a third-party host, a 404 and a network failure all read + * as DENIED, which is exactly the set of runtimes leaking today. + * + * A conjunct, never a source — it cannot start telemetry on a build that + * carries no DSN. See `observability/sentry.ts` for the composition. + */ +export function isClientErrorReportingAllowed(): boolean { + return current.telemetry?.allowClientErrorReporting === true; +} + /** Test/dev helper. */ export function resetRuntimeConfigForTesting(): void { current = { ...defaults, features: { ...defaults.features }, branding: { ...defaults.branding }, + telemetry: { ...defaults.telemetry }, }; initialised = false; }