diff --git a/.changeset/runtime-config-client-error-reporting-dsn.md b/.changeset/runtime-config-client-error-reporting-dsn.md new file mode 100644 index 0000000000..acecc7eccd --- /dev/null +++ b/.changeset/runtime-config-client-error-reporting-dsn.md @@ -0,0 +1,55 @@ +--- +"@objectstack/cloud-connection": minor +--- + +**Security (p0, upstream half):** `GET /api/v1/runtime/config` now serves the Console's client error-reporting **sink** — the DSN itself, plus the closed set of knobs that travel with it — so a self-hosting operator configures telemetry on the server, in one place, with no frontend rebuild (#12681, upstream half of `cloud#1508`). + +```json +{ + "telemetry": { + "errorReporting": { + "dsn": "https://PUBLIC_KEY@o1.ingest.sentry.io/42", + "sendDefaultPii": false, + "environment": "production", + "tracesSampleRate": 0.1, + "replaysOnErrorSampleRate": 0 + } + } +} +``` + +An air-gapped on-premises EE Console was measured sending **14 Sentry envelopes per session** to `sentry.io`, carrying IP and User-Agent PII, with no way for the customer to turn it off. The first fix (#10805) served a runtime *permission* and left the *source* where it was — a build-time `VITE_SENTRY_DSN` inlined into the published bundle. That closed the leak and opened a different hole, which the maintainer named on 2026-08-27: + +> 「我是一个开发平台呀，我的用户并不会去构建我的前端，我理解这种应该在服务端传进去。」 + +ObjectStack's users consume a **prebuilt** Console. They cannot set a build-time key, so under the two-key gate a self-hosting operator could not enable client error reporting at all: the permission was reachable and the source was not. + +**The DSN's presence IS the grant.** There is no second boolean, and this is not shorthand — it removes the failure mode the two-key shape had. With a permission and a source configured in different places, "permission on, no DSN" and "DSN in, permission off" are two silent dead states that look identical from the browser. One knob cannot disagree with itself. + +The fail-closed direction survives the collapse for free, and more robustly than the boolean managed: the grant is now "a non-empty DSN reached me", so an older runtime, a third-party host, a 404, a network error, a malformed body and a payload that has not arrived yet all carry no DSN and therefore deny. A boolean needed `=== true` plus a written argument about why `disabled: true` would have been vacuous; absence of a *source* is not a value that can be misread. + +**Everything that must travel with the DSN travels with it.** `sendDefaultPii`, `environment`, `tracesSampleRate` and `replaysOnErrorSampleRate` were build-time `VITE_SENTRY_*` variables, which a prebuilt-console consumer could set none of — including the one deciding whether IP and User-Agent leave the network. This is not new surface; it is the same surface moved to the side that can operate it. One knob deliberately did **not** move: a release identifies *which bundle* produced a stack trace and must match that build's uploaded source maps, so `VITE_SENTRY_RELEASE` stays build-time in objectui and is the only `VITE_SENTRY_*` knob that does. + +**Malformed is refused at mount, never coerced**, and every refusal lands on the safer value. A DSN that is not an `https://PUBLIC_KEY@HOST/PROJECT_ID` URL is refused and the whole block withheld — there is no safe default for a source. A DSN carrying a **secret** after the public key is refused for a different reason: this payload is read by every browser that loads the Console, so a legacy secret-bearing DSN would publish that secret to every visitor while looking entirely ordinary. A bad sample rate falls back to its documented default instead, because silencing error reporting over a typo in a volume knob would be strictness pointed away from the hazard. Quoted values are key-redacted: boot logs travel further than the configuration they quote. + +**A runtime that declared its control plane off serves no sink.** `OS_CLOUD_URL=off` (or `none` / `local` / `disabled`) refuses the DSN and says so in the boot log — the copied-hosted-config-onto-an-air-gapped-box shape. That declaration is the repo's one existing network-posture signal and needs no new knob: the EE image's compose file already defaults `OS_CLOUD_URL` to `off`, so the operator this failed is safe with zero configuration. + +**Absence is denial, and the reading ships with the contract.** `readClientErrorReporting(payload)` is the canonical fail-closed reader, returning the sink or `null`; a failed fetch is spelled by passing `undefined`, so the error path and the absent path reach the same answer through the same function. It is exported rather than left to consumers because "no DSN means do not send" is a claim about *their* code. + +### Breaking: `telemetry.allowClientErrorReporting` is REPLACED, not paralleled + +The #10805 permission boolean is removed in this same change — no dual-spelling window. It was added days ago, is **unreleased** (it appears in no published `CHANGELOG.md`), and no deployment consumes it; its pending changeset is superseded by this one rather than shipping a feature and its removal in the same release notes. + +| FROM | TO | +|:--|:--| +| `OS_TELEMETRY_CLIENT_ERROR_REPORTING_ENABLED=true` | `OS_TELEMETRY_CLIENT_ERROR_REPORTING_DSN=https://PUBLIC_KEY@HOST/PROJECT_ID` | +| `new RuntimeConfigPlugin({ allowClientErrorReporting: true })` | `new RuntimeConfigPlugin({ clientErrorReporting: { dsn: '…' } })` | +| `telemetry.allowClientErrorReporting: boolean` on the payload | `telemetry.errorReporting?: { dsn, sendDefaultPii, environment?, tracesSampleRate, replaysOnErrorSampleRate }` | +| `isClientErrorReportingAllowed(payload): boolean` | `readClientErrorReporting(payload): ClientErrorReportingConfig \| null` | +| `CLIENT_ERROR_REPORTING_ENV` | `CLIENT_ERROR_REPORTING_DSN_ENV` (plus `..._PII_ENV`, `..._ENVIRONMENT_ENV`, `..._TRACES_RATE_ENV`, `..._REPLAY_RATE_ENV`) | + +One-line fix for an operator: replace the `..._ENABLED=true` line with a `..._DSN=` line carrying your DSN. One-line fix for a consumer: `if (readClientErrorReporting(payload)) …` in place of `if (buildTimeDsn && isClientErrorReportingAllowed(payload)) …` — the build-time conjunct is gone, because the server now supplies the source. + +**Landing order is safe in both directions.** An old client meeting this server reads an absent `allowClientErrorReporting` and denies; a new client meeting an old server reads an absent DSN and stays off. Neither half can turn reporting on by itself, so the two repos' PRs can land in any order. + + diff --git a/.changeset/runtime-config-telemetry-posture.md b/.changeset/runtime-config-telemetry-posture.md deleted file mode 100644 index 0abe29ff93..0000000000 --- a/.changeset/runtime-config-telemetry-posture.md +++ /dev/null @@ -1,25 +0,0 @@ ---- -"@objectstack/cloud-connection": minor ---- - -**Security (p0, upstream half):** `GET /api/v1/runtime/config` now carries a `telemetry` block, giving the Console SPA a **post-build off switch** for client error reporting (#10805, upstream half of `cloud#1508`). - -An air-gapped on-premises EE Console was measured sending **14 Sentry envelopes per session** to `sentry.io`, carrying IP and User-Agent PII, with no way for the customer to turn it off. objectui closed the half it owns — a build that never opts in now issues no third-party request at all — and could not close the other: every telemetry knob there is a Vite build-time variable inlined into the bundle as a frozen literal, so a build that **did** opt in (the hosted console, and the identical artifact shipped on-prem) had no switch that editing env vars on the deployed host could reach. The only server-to-SPA channel is this endpoint. - -```json -{ "telemetry": { "allowClientErrorReporting": false } } -``` - -Operators grant it with `OS_TELEMETRY_CLIENT_ERROR_REPORTING_ENABLED=true`, or hosts with `new RuntimeConfigPlugin({ allowClientErrorReporting: true })`. The switch answers to the repo's usual truthy vocabulary (`1` / `true` / `on` / `yes`); an unrecognised spelling is refused and named at mount time rather than coerced. - -**Denied by default on every posture, not only the air-gapped one.** Deriving "connected therefore allowed" would have left the reported injury class open one deployment over: an internet-connected on-prem box runs the *same build artifact* as the hosted console, so the DSN cannot tell them apart, and its customer has equally never heard of Sentry. A universal opt-in satisfies "air-gap defaults off" strictly, and satisfies it without having to identify the posture correctly — which matters, because a posture predicate that is wrong in the *allow* direction is this card's own defect. - -**A permission, never a source.** The server supplies no DSN and cannot start telemetry for a build that carries none; `true` means only "this deployment does not object to the sink you were compiled with". The composed decision stays `Boolean(buildTimeDsn) && isClientErrorReportingAllowed(payload)`. - -**A runtime that declared its control plane off cannot grant it.** `OS_CLOUD_URL=off` (or `none` / `local` / `disabled`) refuses the grant and says so in the boot log — the copied-hosted-config-onto-an-air-gapped-box shape. That declaration is the repo's one existing network-posture signal and needs no new knob: the EE image's compose file already defaults `OS_CLOUD_URL` to `off`, so the operator this failed is safe with zero configuration. - -**Absence is denial, and the reading ships with the contract.** A new export, `isClientErrorReportingAllowed(payload)`, is the canonical fail-closed reader: an older runtime's payload, a malformed body, a 404 and a failed fetch (pass `undefined`) all answer `false`. It is exported rather than left to consumers because "absent means do not send" is a claim about *their* code, and a hand-written `?.` chain is one `!== false` away from re-opening the leak on exactly the legacy payloads the guarantee is for. The key is spelled as a permission for the same reason: a negative `disabled` flag would have read falsy — therefore "send" — on every one of those states. - -`isControlPlaneDeclined()` is factored out of `cloud-url.ts` so "what counts as off" has one definition shared by the URL resolution and the telemetry refusal. No behaviour change to `resolveCloudUrl()`. - -The consumer half (reading the key and gating `initSentry`) is objectui's and is filed separately. diff --git a/content/docs/deployment/environment-variables.mdx b/content/docs/deployment/environment-variables.mdx index d56336f68c..49fcda0319 100644 --- a/content/docs/deployment/environment-variables.mdx +++ b/content/docs/deployment/environment-variables.mdx @@ -339,7 +339,11 @@ the hosted ObjectOS Cloud control plane. | `OS_OTLP_ENDPOINT` | url | — | OTLP/HTTP collector endpoint. Required when `OS_OBS_EXPORTER=otlp`. | | `OS_OTLP_HEADERS` | csv | — | Comma-separated `key=value` pairs added to every OTLP export. | | `OS_OTLP_FLUSH_MS` | number | `10000` | OTLP batch flush interval. | -| `OS_TELEMETRY_CLIENT_ERROR_REPORTING_ENABLED` | boolean | `false` | Permits the Console SPA to send client error reports to the sink its build was compiled with, via `telemetry.allowClientErrorReporting` on `/api/v1/runtime/config`. Opt-in on every posture: an unset switch, an unrecognised value, or a runtime that declared its control plane off (`OS_CLOUD_URL=off`) all deny. It is a permission, not a source — it cannot start telemetry for a build that carries no DSN. | +| `OS_TELEMETRY_CLIENT_ERROR_REPORTING_DSN` | url | — | Error-reporting sink for the Console SPA, served as `telemetry.errorReporting.dsn` on `/api/v1/runtime/config`. **Presence is the grant** — unset means the Console sends nothing, and there is no separate permission flag. Set it here and nowhere else: the Console is consumed prebuilt, so this is the only place a self-hosting operator can configure it, and no frontend rebuild is involved. Refused loudly at mount when it is not an `https://PUBLIC_KEY@HOST/PROJECT_ID` URL, when it carries a secret after the public key (this payload is public to every browser), or when the runtime declared its control plane off (`OS_CLOUD_URL=off`). | +| `OS_TELEMETRY_CLIENT_ERROR_REPORTING_SEND_DEFAULT_PII` | boolean | `false` | Attach IP address and User-Agent to client error events. Opt-in; an unrecognised value is refused at mount and stays off. Inert without a DSN. | +| `OS_TELEMETRY_CLIENT_ERROR_REPORTING_ENVIRONMENT` | string | — | `environment` tag on client error events (`production`, `staging`, …). Unset lets the Console tag events with its own build mode. Inert without a DSN. | +| `OS_TELEMETRY_CLIENT_ERROR_REPORTING_TRACES_SAMPLE_RATE` | number | `0.1` | Fraction (`0`–`1`) of Console transactions sampled for performance tracing. Out-of-range or unparseable values are refused at mount and fall back to the default. Inert without a DSN. | +| `OS_TELEMETRY_CLIENT_ERROR_REPORTING_REPLAY_SAMPLE_RATE` | number | `0` | Fraction (`0`–`1`) of Console **error** sessions recorded as session replays. Off by default — replay records what the user did, so it is the deliberate choice of the deployment that wants it. Inert without a DSN. | --- diff --git a/packages/cli/test/serve-marketplace-offline-runtime-config.test.ts b/packages/cli/test/serve-marketplace-offline-runtime-config.test.ts index 9bf0544278..529d77773f 100644 --- a/packages/cli/test/serve-marketplace-offline-runtime-config.test.ts +++ b/packages/cli/test/serve-marketplace-offline-runtime-config.test.ts @@ -431,11 +431,12 @@ describe('#8389: the identities and options the arm mounts with are the real one }); /** - * #10805 — the same offline arm must also serve the SPA telemetry refusal. + * #12681 — the same offline arm must also refuse to serve the telemetry sink. * - * This is cloud#1508's acceptance expressed on the server side: on a - * composed / air-gapped posture, a Console build that DOES carry a Sentry DSN - * must be told not to send, through a switch that needs no rebuild. + * This is cloud#1508's acceptance expressed on the server side, now that the + * DSN itself travels on the payload: on a composed / air-gapped posture, the + * Console must be given no sink at all, and an operator who configured one + * anyway must be told it was refused — with no rebuild involved on either side. * * It belongs here rather than only in the plugin's own suite because of one * measured property of this wiring: `Serve.RUNTIME_CONFIG_OPTIONS` hands the @@ -451,12 +452,13 @@ describe('#8389: the identities and options the arm mounts with are the real one * blocks above do, because that simulation is precisely the half that would * hide the defect. */ -describe('#10805: the offline arm refuses client telemetry on a real OS_CLOUD_URL=off boot', () => { - const GRANT_ENV = 'OS_TELEMETRY_CLIENT_ERROR_REPORTING_ENABLED'; +describe('#12681: the offline arm serves no client telemetry sink on a real OS_CLOUD_URL=off boot', () => { + const DSN_ENV = 'OS_TELEMETRY_CLIENT_ERROR_REPORTING_DSN'; + const DSN = 'https://abc123@o1.ingest.sentry.io/42'; async function bootAirGapped(run: (body: any) => void | Promise): Promise { const savedCloudUrl = process.env.OS_CLOUD_URL; - const savedGrant = process.env[GRANT_ENV]; + const savedDsn = process.env[DSN_ENV]; const dir = tempStorageDir(); try { process.env.OS_CLOUD_URL = 'off'; @@ -465,52 +467,52 @@ describe('#10805: the offline arm refuses client telemetry on a real OS_CLOUD_UR } finally { if (savedCloudUrl === undefined) delete process.env.OS_CLOUD_URL; else process.env.OS_CLOUD_URL = savedCloudUrl; - if (savedGrant === undefined) delete process.env[GRANT_ENV]; - else process.env[GRANT_ENV] = savedGrant; + if (savedDsn === undefined) delete process.env[DSN_ENV]; + else process.env[DSN_ENV] = savedDsn; rmSync(dir, { recursive: true, force: true }); } } - it('THE ACCEPTANCE — an air-gapped boot tells the Console not to send, with zero configuration', async () => { + it('THE ACCEPTANCE — an air-gapped boot hands the Console no sink, with zero configuration', async () => { await bootAirGapped((body) => { expect( - body.telemetry.allowClientErrorReporting, + body.telemetry.errorReporting, 'an operator who has never heard of Sentry must be safe without configuring anything', - ).toBe(false); + ).toBeUndefined(); }); }); - it('...and refuses even an explicit grant, because this runtime declared its control plane off', async () => { - process.env[GRANT_ENV] = 'true'; + it('...and refuses even an explicit DSN, because this runtime declared its control plane off', async () => { + process.env[DSN_ENV] = DSN; await bootAirGapped(async (body) => { - const { isClientErrorReportingAllowed } = await import('@objectstack/cloud-connection'); - expect(body.telemetry.allowClientErrorReporting).toBe(false); + const { readClientErrorReporting } = await import('@objectstack/cloud-connection'); + expect(body.telemetry.errorReporting).toBeUndefined(); // Read through the exported contract too: what the SPA will actually do - // with this payload is the thing under test, not the raw boolean. - expect(isClientErrorReportingAllowed(body)).toBe(false); + // with this payload is the thing under test, not the raw key. + expect(readClientErrorReporting(body)).toBeNull(); }); }); - it('POSITIVE CONTROL — the same grant on the CLOUD arm is honoured', async () => { + it('POSITIVE CONTROL — the same DSN on the CLOUD arm is served', async () => { // Without this, the refusal above could be an artifact of the fixture // rather than a posture reading, and the pin would stay green on a build - // that denies everything unconditionally. + // that serves nothing unconditionally. const savedCloudUrl = process.env.OS_CLOUD_URL; - const savedGrant = process.env[GRANT_ENV]; + const savedDsn = process.env[DSN_ENV]; try { process.env.OS_CLOUD_URL = 'https://cloud.objectos.ai'; - process.env[GRANT_ENV] = 'true'; + process.env[DSN_ENV] = DSN; const { RuntimeConfigPlugin } = await import('@objectstack/cloud-connection'); const app = createApp(); // The cloud arm's own mount, verbatim — same shared frozen options. await startOn(app, new RuntimeConfigPlugin({ ...Serve.RUNTIME_CONFIG_OPTIONS })); const body = await readConfig(app); - expect(body.telemetry.allowClientErrorReporting).toBe(true); + expect(body.telemetry.errorReporting.dsn).toBe(DSN); } finally { if (savedCloudUrl === undefined) delete process.env.OS_CLOUD_URL; else process.env.OS_CLOUD_URL = savedCloudUrl; - if (savedGrant === undefined) delete process.env[GRANT_ENV]; - else process.env[GRANT_ENV] = savedGrant; + if (savedDsn === undefined) delete process.env[DSN_ENV]; + else process.env[DSN_ENV] = savedDsn; } }); }); diff --git a/packages/cloud-connection/README.md b/packages/cloud-connection/README.md index 1284d5314e..8132f6071a 100644 --- a/packages/cloud-connection/README.md +++ b/packages/cloud-connection/README.md @@ -55,44 +55,79 @@ const plugins = [ ]; ``` -## SPA telemetry is denied unless a runtime grants it +## The Console's error-reporting sink is served by this runtime -`GET /api/v1/runtime/config` carries a `telemetry` block: +`GET /api/v1/runtime/config` carries a `telemetry` block. Unconfigured, it is +empty — which is what a deployment that never asked for error reporting serves: ```json -{ "telemetry": { "allowClientErrorReporting": false } } +{ "telemetry": {} } ``` -It is the Console's **post-build off switch**. Every telemetry knob in the SPA -is a build-time variable frozen into the bundle, so a build that opted in has -no other way to be turned off on a deployed host — and an air-gapped -deployment measurably shipped one that could not be (`cloud#1508`: 14 Sentry -envelopes per session carrying IP and User-Agent PII). +Configure a sink and the block carries it, together with the closed set of +knobs that must travel with it: -It is **denied by default on every posture**. Grant it explicitly: +```json +{ + "telemetry": { + "errorReporting": { + "dsn": "https://PUBLIC_KEY@o1.ingest.sentry.io/42", + "sendDefaultPii": false, + "environment": "production", + "tracesSampleRate": 0.1, + "replaysOnErrorSampleRate": 0 + } + } +} +``` + +Everything is set on the **runtime**, in one place, with no frontend rebuild — +which is the point: ObjectStack's users consume a prebuilt Console and cannot +set build-time keys. ```bash -OS_TELEMETRY_CLIENT_ERROR_REPORTING_ENABLED=true # or: new RuntimeConfigPlugin({ allowClientErrorReporting: true }) +OS_TELEMETRY_CLIENT_ERROR_REPORTING_DSN=https://PUBLIC_KEY@o1.ingest.sentry.io/42 +OS_TELEMETRY_CLIENT_ERROR_REPORTING_SEND_DEFAULT_PII=true # IP + User-Agent, off by default +OS_TELEMETRY_CLIENT_ERROR_REPORTING_ENVIRONMENT=production +OS_TELEMETRY_CLIENT_ERROR_REPORTING_TRACES_SAMPLE_RATE=0.1 +OS_TELEMETRY_CLIENT_ERROR_REPORTING_REPLAY_SAMPLE_RATE=0 +``` + +…or, from a host that composes the plugin directly: + +```ts +new RuntimeConfigPlugin({ clientErrorReporting: { dsn: 'https://PUBLIC_KEY@…/42' } }) ``` -Three properties worth knowing before you build on it: +An explicit option wins over the matching env var, **per field** — a host that +sets only `sendDefaultPii` does not discard the operator's DSN. + +Four properties worth knowing before you build on it: -- **A permission, not a source.** The server supplies no DSN and cannot start - telemetry for a build that carries none. `true` means only "this deployment - does not object to the sink you were compiled with". -- **A runtime that declared its control plane off cannot grant it.** - `OS_CLOUD_URL=off` (or `none` / `local` / `disabled`) refuses the grant and - says so in the boot log, so an air-gapped box stays silent even if a hosted +- **The DSN's presence IS the grant.** There is no separate permission boolean + (the one that shipped in #10805 was removed by #12681, not paralleled). A + runtime that serves a DSN is asking for reports; a runtime that serves none + is not. Two knobs in two places had two silent dead states — "permission on, + no DSN" and "DSN in, permission off" — that look identical from the browser. +- **A runtime that declared its control plane off serves no sink.** + `OS_CLOUD_URL=off` (or `none` / `local` / `disabled`) refuses the DSN and says + so in the boot log, so an air-gapped box stays silent even if a hosted configuration is copied onto it. -- **Absence means denied.** An older runtime, a third-party host, a 404 or a - failed fetch all read the same way. Consumers should use the reading that +- **Malformed is refused at mount, never coerced.** A DSN that is not an + `https://PUBLIC_KEY@HOST/PROJECT_ID` URL is refused and named in the boot log, + and so is one carrying a **secret** after the public key — this payload is read + by every browser that loads the Console. A bad sample rate falls back to its + default rather than taking the sink down with it. +- **Absence means no reporting.** An older runtime, a third-party host, a 404 or + a failed fetch all read the same way. Consumers should use the reading that ships with the contract rather than writing their own: ```ts -import { isClientErrorReportingAllowed } from '@objectstack/cloud-connection'; +import { readClientErrorReporting } from '@objectstack/cloud-connection'; // `payload` may be the parsed body, or undefined when the fetch failed. -if (buildTimeDsn && isClientErrorReportingAllowed(payload)) initErrorReporting(); +const sink = readClientErrorReporting(payload); +if (sink) initErrorReporting(sink); ``` ## Boundary (open mechanism, closed intelligence) @@ -106,7 +141,7 @@ rules. Plan-derived feature flags are injected by the host via `OS_CLOUD_URL=off` disables every remote call; air-gapped installs keep working via inline manifests handed to `install-local`, and the SPA telemetry -permission above cannot be granted. +sink above is refused rather than served. See `docs/adr` in the cloud repository (ADR-0008) for the full architecture decision. diff --git a/packages/cloud-connection/src/index.ts b/packages/cloud-connection/src/index.ts index d08c009eaa..4efe005277 100644 --- a/packages/cloud-connection/src/index.ts +++ b/packages/cloud-connection/src/index.ts @@ -55,13 +55,25 @@ export { CloudConnectionPlugin, createCloudConnectionPlugin } from './cloud-conn export type { CloudConnectionPluginConfig } from './cloud-connection-plugin.js'; export { RuntimeConfigPlugin } from './runtime-config-plugin.js'; export type { RuntimeConfigPluginConfig, RuntimeFeatureOverrides, RuntimeConfigPlanFeatures, PlatformStage } from './runtime-config-plugin.js'; -// #10805 — the SPA telemetry permission carried on that payload, and the -// canonical fail-closed way to read it. The reader is exported deliberately: -// "an absent key means do not send" is a claim about consumer code, and a -// consumer writing its own `?.` chain is one `!== false` away from re-opening -// the PII leak on exactly the legacy payloads the guarantee is for. -export { isClientErrorReportingAllowed, CLIENT_ERROR_REPORTING_ENV } from './telemetry-posture.js'; -export type { RuntimeTelemetryPosture } from './telemetry-posture.js'; +// #12681 — the SPA's client error-reporting SOURCE carried on that payload, +// and the canonical fail-closed way to read it. The reader is exported +// deliberately: "no DSN means do not send" is a claim about consumer code, and +// a consumer writing its own `?.` chain is one loose truthiness check away +// from re-opening the PII leak on exactly the legacy payloads the guarantee is +// for. (Supersedes the #10805 `isClientErrorReportingAllowed` permission +// reader, removed with the boolean it read.) +export { + readClientErrorReporting, + redactDsn, + CLIENT_ERROR_REPORTING_DSN_ENV, + CLIENT_ERROR_REPORTING_PII_ENV, + CLIENT_ERROR_REPORTING_ENVIRONMENT_ENV, + CLIENT_ERROR_REPORTING_TRACES_RATE_ENV, + CLIENT_ERROR_REPORTING_REPLAY_RATE_ENV, + DEFAULT_TRACES_SAMPLE_RATE, + DEFAULT_REPLAYS_ON_ERROR_SAMPLE_RATE, +} from './telemetry-posture.js'; +export type { RuntimeTelemetryPosture, ClientErrorReportingConfig } from './telemetry-posture.js'; // ADR-0008 consumption side — the self-hosted credential ledger (bind // persists the oscc_ bearer here; forwards present it to the control plane). export { ConnectionCredentialStore, DEFAULT_CONNECTION_CREDENTIAL_PATH } from './connection-credential-store.js'; diff --git a/packages/cloud-connection/src/runtime-config-plugin.ts b/packages/cloud-connection/src/runtime-config-plugin.ts index 6debe4703e..b481aa7ffa 100644 --- a/packages/cloud-connection/src/runtime-config-plugin.ts +++ b/packages/cloud-connection/src/runtime-config-plugin.ts @@ -16,7 +16,7 @@ * defaultOrgId?, defaultEnvironmentId?, // multi-tenant, per-hostname * features: { installLocal, marketplace, aiStudio, autoPublishAiBuilds, ... }, * branding: { productName, productShortName, stage?, logoUrl, faviconUrl, brandColor, pwaDescription, pwaThemeColor }, - * telemetry: { allowClientErrorReporting: boolean } + * telemetry: { errorReporting?: { dsn, sendDefaultPii, environment?, tracesSampleRate, replaysOnErrorSampleRate } } * } * * ## `branding.stage` — a documented knob that this runtime never sent (#9252) @@ -58,39 +58,42 @@ * so the Console keeps applying its own documented `'preview'` default and * nothing that works today changes. * - * ## `telemetry.allowClientErrorReporting` — the post-build off switch (#10805) + * ## `telemetry.errorReporting` — the client error-reporting SOURCE (#12681) * * Upstream half of cloud#1508 (p0/security): an air-gapped on-prem EE Console * was measured sending 14 Sentry envelopes per session to `sentry.io` carrying - * IP + User-Agent PII, with no way to turn it off. objectui fixed the half it - * owns — a build that never opts in now issues no third-party request at all — - * and documented the half it could not, verbatim from its shipped - * `app-shell/src/observability/sentry.ts`: *"a build that DID opt in still has - * no post-build off switch, because the only server-to-SPA channel is - * `/api/v1/runtime/config` and a telemetry key on that payload is an - * objectstack contract change, not objectui's to make."* This is that key. - * - * Ruled Option A (maintainer, 2026-08-22): server-authoritative, fail-closed, - * and the composed / air-gap posture defaults telemetry off so an operator who - * has never heard of Sentry is safe with zero configuration. + * IP + User-Agent PII, with no way to turn it off. The first fix (#10805) + * served a runtime PERMISSION and left the SOURCE build-time, which closed the + * leak and opened a different hole. The maintainer named it on 2026-08-27, + * verbatim and untranslated: + * + * > 「我是一个开发平台呀，我的用户并不会去构建我的前端，我理解这种应该在服务端传进去。」 + * + * ObjectStack's users consume a PREBUILT console and cannot set a build-time + * key, so under the two-key gate a self-hosting operator could not enable + * client error reporting at all. The DSN therefore moves here, and the + * permission boolean it replaces is REMOVED rather than paralleled — no dual + * spelling, per the startup-stage no-gradualism rule. * * Three properties, and each is a decision rather than a detail — see - * `telemetry-posture.ts` for the vocabulary reasoning: - * - * 1. **A permission, not a kill switch.** Only a positive grant sends. A - * negative `disabled` key would read falsy on every server too old to know - * it, on every malformed payload and on every failed fetch — i.e. it would - * be vacuous on the runtimes that are leaking today. - * 2. **Denied on EVERY posture until granted**, not only on the air-gapped - * one. Deriving "connected therefore allowed" would have left the reported - * injury class open one deployment over: an internet-connected on-prem EE - * box runs the SAME build artifact as the hosted console, so the DSN - * cannot distinguish them and its customer has equally never heard of - * Sentry. A universal opt-in satisfies "air-gap defaults off" strictly, - * and satisfies it without having to identify the posture correctly — - * which matters, because a posture predicate that is wrong in the ALLOW - * direction is this card's own defect. - * 3. **A declared-off control plane REFUSES the grant** — see + * `telemetry-posture.ts` for the full reasoning: + * + * 1. **The DSN's presence IS the grant.** No second boolean. Two knobs in two + * places produced two silent dead states ("permission on, no DSN" / "DSN + * in, permission off") that are indistinguishable from the browser; one + * knob cannot disagree with itself. Fail-closed survives the collapse for + * free: absence of a source is not a value that can be misread, so an + * older runtime, a third-party host, a 404, a network error and a + * malformed body all carry no DSN and therefore deny. + * 2. **Denied on EVERY posture until configured**, not only on the air-gapped + * one. An internet-connected on-prem EE box runs the SAME build artifact + * as the hosted console, so nothing in the bundle distinguishes them and + * its customer has equally never heard of Sentry. A universal opt-in + * satisfies "air-gap defaults off" strictly, and satisfies it without + * having to identify the posture correctly — which matters, because a + * posture predicate that is wrong in the ALLOW direction is this defect + * class exactly. + * 3. **A declared-off control plane REFUSES to serve the DSN** — see * {@link declinesControlPlane}. That is the one place * the posture is load-bearing rather than decorative. * @@ -115,9 +118,10 @@ * requirement — zero configuration — by requiring the operator to learn it * exists. * - * So no posture source is introduced. What is introduced is one opt-in - * permission (`OS_TELEMETRY_CLIENT_ERROR_REPORTING_ENABLED`), and the existing - * decline declaration is honoured as a ceiling on it. + * So no posture source is introduced. What is introduced is one operator + * configuration (`OS_TELEMETRY_CLIENT_ERROR_REPORTING_DSN` and the closed set + * of knobs that travel with it), and the existing decline declaration is + * honoured as a ceiling on it. * * ## Feature seam (open-core boundary — cloud ADR-0012) * @@ -190,10 +194,16 @@ import type { Plugin, PluginContext } from '@objectstack/core'; import { resolveCloudUrl, isControlPlaneDeclined } from './cloud-url.js'; import { - CLIENT_ERROR_REPORTING_ENV, - CLIENT_ERROR_REPORTING_SPELLINGS, - readClientErrorReportingGrant, + CLIENT_ERROR_REPORTING_DSN_ENV, + CLIENT_ERROR_REPORTING_PII_ENV, + CLIENT_ERROR_REPORTING_ENVIRONMENT_ENV, + CLIENT_ERROR_REPORTING_TRACES_RATE_ENV, + CLIENT_ERROR_REPORTING_REPLAY_RATE_ENV, + readClientErrorReportingConfig, + redactDsn, + type ClientErrorReportingConfig, type RuntimeTelemetryPosture, + type TelemetryRefusal, } from './telemetry-posture.js'; import type { IHttpServer } from '@objectstack/spec/contracts'; @@ -525,26 +535,41 @@ export interface RuntimeConfigPluginConfig { /** PWA theme color hex. Falls back to OS_PWA_THEME_COLOR env var. Default: brandColor or '#4f46e5'. */ pwaThemeColor?: string; /** - * Grant the SPA permission to send client error reports to the sink its - * build was compiled with (#10805). Falls back to the - * `OS_TELEMETRY_CLIENT_ERROR_REPORTING_ENABLED` env var. + * The Console's client error-reporting sink and the closed set of knobs + * that travel with it (#12681). Each field falls back to its own env var: + * `OS_TELEMETRY_CLIENT_ERROR_REPORTING_DSN` and siblings. + * + * ⛔ Default **absent**, on every posture. A deployment that says nothing + * sends nothing, and the operator who has never heard of Sentry needs no + * configuration to be safe. * - * ⛔ Default **false**, on every posture. This is the post-build off - * switch cloud#1508 asked for, so it fails closed: a deployment that says - * nothing sends nothing, and the operator who has never heard of Sentry - * needs no configuration to be safe. + * **Presence of the `dsn` IS the grant** — there is no separate permission + * boolean (the #10805 one was removed by this card, not paralleled). A + * runtime that serves a DSN is asking for reports; a runtime that serves + * none is not. * - * It is 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". + * Precedence is **per field**, matching every branding key above: an + * explicit value here wins, the env var fills the rest. Whole-object + * replacement was rejected — a host passing only `sendDefaultPii` would + * silently discard the operator's `..._DSN`, which is the class of quiet + * two-knob failure this card exists to delete. * - * ⛔ It cannot be granted on a runtime that declared its control plane off + * ⛔ Nothing is served on a runtime that declared its control plane off * (`OS_CLOUD_URL=off` / `none` / `local` / `disabled`). That declaration - * IS a runtime declining outbound calls, and the ruling this key - * implements is that a declining runtime wins. Refused loudly at mount - * time, never silently. + * IS a runtime declining outbound calls, and a declining runtime wins. + * Refused loudly at mount time, never silently. + * + * ⛔ A malformed DSN is refused, never coerced — see + * `telemetry-posture.ts` for what "malformed" means and why a bad sample + * rate only takes down itself while a bad DSN takes down the block. */ - allowClientErrorReporting?: boolean; + clientErrorReporting?: { + dsn?: string; + sendDefaultPii?: boolean; + environment?: string; + tracesSampleRate?: number; + replaysOnErrorSampleRate?: number; + }; /** * Distribution feature-policy hook (open-core seam — cloud ADR-0012). * Called with `undefined` for the static default (no environment resolved @@ -593,12 +618,15 @@ export class RuntimeConfigPlugin implements Plugin { private readonly brandColor: string | undefined; private readonly pwaDescription: string; private readonly pwaThemeColor: string; - /** The resolved permission served as `telemetry.allowClientErrorReporting`. */ - private readonly allowClientErrorReporting: boolean; - /** An unrecognised switch spelling, kept so `start()` can name it once. */ - private readonly refusedTelemetryGrant: string | undefined; - /** True when a real grant was overruled by the declined control plane. */ - private readonly telemetryGrantRefusedByPosture: boolean; + /** + * The resolved sink served as `telemetry.errorReporting`, or `undefined` + * for "serve no block" — unset, refused, or lowered by the posture. + */ + private readonly clientErrorReporting: ClientErrorReportingConfig | undefined; + /** Every knob the operator got wrong, kept so `start()` can name them once. */ + private readonly telemetryRefusals: readonly TelemetryRefusal[]; + /** The DSN (redacted) that the declined control plane took away, if any. */ + private readonly telemetryDsnRefusedByPosture: string | undefined; private readonly resolveFeatures?: (token: string | undefined) => RuntimeFeatureOverrides; constructor(config: RuntimeConfigPluginConfig = {}) { @@ -638,23 +666,31 @@ export class RuntimeConfigPlugin implements Plugin { this.pwaDescription = config.pwaDescription ?? envPwaDescription ?? `${this.productName} — runtime console`; this.pwaThemeColor = config.pwaThemeColor ?? envPwaThemeColor ?? this.brandColor ?? '#4f46e5'; - // Telemetry permission (#10805). Same precedence as every branding key - // — the HOST's explicit option wins, the env var is the operator's - // fallback — but the BASE is denial rather than a default value, and - // the posture can only lower the result. - const envGrant = readClientErrorReportingGrant( - typeof process !== 'undefined' ? process.env?.[CLIENT_ERROR_REPORTING_ENV] : undefined, - ); - this.refusedTelemetryGrant = envGrant.refused; - // `=== true`, not `??` then coerce: a JS host outside this type can - // hand us any value, and only the literal boolean grants. - const requested = config.allowClientErrorReporting !== undefined - ? config.allowClientErrorReporting === true - : envGrant.allowed; + // Client error reporting (#12681). Same precedence as every branding + // key above — the HOST's explicit option wins, the env var is the + // operator's fallback — applied PER FIELD, and resolved through one + // validating reader so a host option and an env var cannot be believed + // on different terms. The BASE is "no block" rather than a default + // value, and the posture can only take away. + const env = (name: string): string | undefined => + typeof process !== 'undefined' ? process.env?.[name] : undefined; + const telemetryConfig = config.clientErrorReporting; + const reading = readClientErrorReportingConfig({ + dsn: telemetryConfig?.dsn ?? env(CLIENT_ERROR_REPORTING_DSN_ENV), + sendDefaultPii: telemetryConfig?.sendDefaultPii ?? env(CLIENT_ERROR_REPORTING_PII_ENV), + environment: telemetryConfig?.environment ?? env(CLIENT_ERROR_REPORTING_ENVIRONMENT_ENV), + tracesSampleRate: + telemetryConfig?.tracesSampleRate ?? env(CLIENT_ERROR_REPORTING_TRACES_RATE_ENV), + replaysOnErrorSampleRate: + telemetryConfig?.replaysOnErrorSampleRate ?? env(CLIENT_ERROR_REPORTING_REPLAY_RATE_ENV), + }); + this.telemetryRefusals = reading.refusals; const declined = declinesControlPlane(config.controlPlaneUrl); - this.allowClientErrorReporting = requested && !declined; + this.clientErrorReporting = declined ? undefined : reading.config; // Only worth a diagnostic when something was actually taken away. - this.telemetryGrantRefusedByPosture = requested && declined; + this.telemetryDsnRefusedByPosture = declined && reading.config + ? redactDsn(reading.config.dsn) + : undefined; } init = async (_ctx: PluginContext): Promise => {}; @@ -707,35 +743,39 @@ export class RuntimeConfigPlugin implements Plugin { ); } - // Telemetry switch outside the closed vocabulary (#10805). Same - // shape and same reason as the stage refusal above: the operator - // meant to GRANT something, the value was not understood, and the - // permission stays denied. Denial is the safe direction, so this - // is `warn` — but it must not be silent, or the operator reads a - // console with no error reporting and no explanation. - if (this.refusedTelemetryGrant !== undefined) { + // Telemetry knobs the operator got wrong (#12681). Same shape and + // same reason as the stage refusal above: they meant to CONFIGURE + // something, the value was not understood, and it was refused + // rather than coerced. Every refusal lands on the safer value, so + // this is `warn` — but it must not be silent, or the operator + // reads a console with no error reporting and no explanation. + // + // One line per refusal, each naming the env var, what was said and + // what is accepted, because an operator who mis-set two knobs has + // two things to fix and hearing about one is how the second stays + // hidden. + for (const refusal of this.telemetryRefusals) { ctx.logger?.warn?.( - `[RuntimeConfigPlugin] ignoring unrecognised telemetry switch ` - + `${JSON.stringify(this.refusedTelemetryGrant)} (${CLIENT_ERROR_REPORTING_ENV}) — ` - + `telemetry.allowClientErrorReporting stays false and the Console will not send client ` - + `error reports. Accepted values: ${CLIENT_ERROR_REPORTING_SPELLINGS.join(', ')}.`, + `[RuntimeConfigPlugin] ignoring unrecognised telemetry value ` + + `${JSON.stringify(refusal.value)} (${refusal.env}) — ${refusal.consequence}. ` + + `Accepted: ${refusal.accepted}.`, ); } - // A real, well-spelled grant overruled by the deployment's own - // declaration (#10805). This is the copied-env-file shape that - // cloud#1508 reported: a hosted configuration landing on an - // air-gapped box. The grant loses — a runtime that declined - // outbound calls has declined this one too — and `warn` rather - // than silence because the operator's explicit request is the - // thing being refused. - if (this.telemetryGrantRefusedByPosture) { + // A well-formed DSN overruled by the deployment's own declaration + // (#12681). This is the copied-env-file shape cloud#1508 reported: + // a hosted configuration landing on an air-gapped box. The DSN + // loses — a runtime that declined outbound calls has declined this + // one too — and `warn` rather than silence because the operator's + // explicit configuration is the thing being refused. + if (this.telemetryDsnRefusedByPosture !== undefined) { ctx.logger?.warn?.( - `[RuntimeConfigPlugin] refusing the client-error-reporting grant ` - + `(${CLIENT_ERROR_REPORTING_ENV} / the \`allowClientErrorReporting\` option): this runtime ` + `[RuntimeConfigPlugin] refusing to serve the client error-reporting DSN ` + + `${JSON.stringify(this.telemetryDsnRefusedByPosture)} ` + + `(${CLIENT_ERROR_REPORTING_DSN_ENV} / the \`clientErrorReporting\` option): this runtime ` + `declared its control plane off via OS_CLOUD_URL, which disables every remote call. ` - + `telemetry.allowClientErrorReporting stays false. Point OS_CLOUD_URL at a control plane ` - + `if this deployment is not air-gapped.`, + + `No telemetry.errorReporting block is served and the Console sends no error reports. ` + + `Point OS_CLOUD_URL at a control plane if this deployment is not air-gapped.`, ); } @@ -853,22 +893,26 @@ export class RuntimeConfigPlugin implements Plugin { pwaThemeColor: this.pwaThemeColor, }, // Its OWN namespace, deliberately not a member of - // `features` (#10805). That map is open-ended and a host's + // `features` (#12681). That map is open-ended and a host's // `resolveFeatures` hook merges arbitrary keys into it - // verbatim, so a distribution could grant this permission - // by returning one boolean from code whose subject is - // billing tiers. A security permission has exactly one - // author. Pinned in runtime-config-telemetry.test.ts. + // verbatim, so a distribution could hand out a telemetry + // sink from code whose subject is billing tiers. A + // security-bearing configuration has exactly one author. + // Pinned in runtime-config-telemetry.test.ts. // - // Always present, unlike `branding.stage` above: absence - // is reserved for payloads that did NOT come from a - // runtime that knows this key (older ObjectStack, third - // party, 404, network error), and every one of those must - // read as denial. Emitting an explicit `false` keeps that - // meaning unambiguous and leaves the state diagnosable - // with one curl. + // The `telemetry` block itself is ALWAYS present, unlike + // `branding.stage` above; `errorReporting` inside it is + // present only when a DSN resolved. That pair is what one + // curl has to distinguish: `{"telemetry":{}}` is "this + // runtime knows the key and has no DSN", while no + // `telemetry` key at all is "this payload did not come + // from a runtime that knows it" (older ObjectStack, third + // party, 404, network error). Both deny; only one of them + // is something the operator can fix here. telemetry: { - allowClientErrorReporting: this.allowClientErrorReporting, + ...(this.clientErrorReporting !== undefined + ? { errorReporting: this.clientErrorReporting } + : {}), } satisfies RuntimeTelemetryPosture, }); }; diff --git a/packages/cloud-connection/src/runtime-config-telemetry.test.ts b/packages/cloud-connection/src/runtime-config-telemetry.test.ts index adcc85985d..d9cbd0890e 100644 --- a/packages/cloud-connection/src/runtime-config-telemetry.test.ts +++ b/packages/cloud-connection/src/runtime-config-telemetry.test.ts @@ -1,33 +1,56 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. /** - * `telemetry.allowClientErrorReporting` — the post-build off switch (#10805, - * upstream half of cloud#1508). + * `telemetry.errorReporting` — the client error-reporting SOURCE served by the + * operator's own runtime (#12681, superseding the #10805 permission boolean). * * The measured injury: an air-gapped on-prem EE Console sent 14 Sentry * envelopes per session to `sentry.io` carrying IP + User-Agent PII, and the - * customer had no way to stop it — every knob was a Vite build-time variable - * frozen into the bundle. This file pins the server half of the fix. + * customer had no way to stop it. #10805 shipped a runtime PERMISSION and left + * the SOURCE compiled into the bundle — which closed the leak and left a + * self-hosting operator unable to turn reporting ON at all, because + * ObjectStack's users consume a prebuilt console and cannot set build-time + * keys. This file pins the server half of the replacement. * * Two subjects, and they are different claims: * - * - `RuntimeConfigPlugin` — does the runtime SAY the right thing? - * - `isClientErrorReportingAllowed` — does the documented reading of what it - * said (and of what a runtime that never heard of the key said) come out - * fail-closed? + * - `RuntimeConfigPlugin` — does the runtime SERVE the right thing? + * - `readClientErrorReporting` — does the documented reading of what it + * served (and of what a runtime that never heard of the key served) come + * out fail-closed? * * The second is the one that carries the guarantee, which is why the producer - * owns it: "absent means do not send" is a claim about consumer code, and a - * consumer left to write its own `?.` chain is one `!== false` away from - * re-opening the leak on exactly the legacy payloads the guarantee is for. + * owns it: "no DSN means do not send" is a claim about consumer code, and a + * consumer left to write its own `?.` chain is one loose truthiness check away + * from re-opening the leak on exactly the legacy payloads the guarantee is for. + * + * ## What changed shape here, and why the old conjunction is gone + * + * #10805's suite composed two grants — `Boolean(buildTimeDsn) && permission` — + * because the source and the permission lived in different places. They do not + * any more: the DSN's presence IS the grant, so the composed decision collapsed + * to reading one object. That collapse is the fix, not an accident of it: two + * knobs in two places produced two silent dead states ("permission on, no DSN" + * and "DSN in, permission off") that look identical from the browser. + * + * Every absence assertion below is therefore paired with a COUNTER-PROBE — a + * posture that SHOULD serve a DSN, asserted to actually serve one. Without it, + * a plugin stuck serving nothing at all would satisfy this entire file. */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { RuntimeConfigPlugin, type RuntimeConfigPluginConfig } from './runtime-config-plugin.js'; import { - isClientErrorReportingAllowed, - readClientErrorReportingGrant, - CLIENT_ERROR_REPORTING_ENV, + readClientErrorReporting, + readClientErrorReportingConfig, + redactDsn, + CLIENT_ERROR_REPORTING_DSN_ENV, + CLIENT_ERROR_REPORTING_PII_ENV, + CLIENT_ERROR_REPORTING_ENVIRONMENT_ENV, + CLIENT_ERROR_REPORTING_TRACES_RATE_ENV, + CLIENT_ERROR_REPORTING_REPLAY_RATE_ENV, + DEFAULT_TRACES_SAMPLE_RATE, + DEFAULT_REPLAYS_ON_ERROR_SAMPLE_RATE, } from './telemetry-posture.js'; interface Served { @@ -42,7 +65,7 @@ interface Served { * * The default `controlPlaneUrl: ''` is load-bearing, not boilerplate: it is * what the CLI passes on BOTH its arms, so every test below that does not - * mention the env var is running in the "same origin, not declined" posture. + * mention `OS_CLOUD_URL` is running in the "same origin, not declined" posture. */ async function serve(pluginConfig: RuntimeConfigPluginConfig = {}): Promise { let handler: ((c: any) => Promise) | undefined; @@ -71,215 +94,365 @@ async function serve(pluginConfig: RuntimeConfigPluginConfig = {}): Promise { - const savedGrant = process.env[CLIENT_ERROR_REPORTING_ENV]; - const savedCloudUrl = process.env.OS_CLOUD_URL; +describe('RuntimeConfigPlugin — telemetry.errorReporting (#12681)', () => { + const saved = new Map(); beforeEach(() => { - delete process.env[CLIENT_ERROR_REPORTING_ENV]; - delete process.env.OS_CLOUD_URL; + for (const name of [...TELEMETRY_ENVS, 'OS_CLOUD_URL']) { + if (!saved.has(name)) saved.set(name, process.env[name]); + delete process.env[name]; + } }); afterEach(() => { - if (savedGrant === undefined) delete process.env[CLIENT_ERROR_REPORTING_ENV]; - else process.env[CLIENT_ERROR_REPORTING_ENV] = savedGrant; - if (savedCloudUrl === undefined) delete process.env.OS_CLOUD_URL; - else process.env.OS_CLOUD_URL = savedCloudUrl; + for (const [name, value] of saved) { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; + } }); describe('fail-closed by default — a deployment that says nothing sends nothing', () => { - it('denies with zero configuration (the air-gapped operator who never heard of Sentry)', async () => { + it('serves no sink with zero configuration (the operator who never heard of Sentry)', async () => { const { body, warnings } = await serve(); - expect(body.telemetry.allowClientErrorReporting).toBe(false); + expect(body.telemetry.errorReporting).toBeUndefined(); // Nothing was refused, so nothing is reported: a default is not a // diagnostic, and a warning on every boot is a muted warning. expect(warnings).toEqual([]); }); - it('denies on a CONNECTED posture too, not only on the air-gapped one', async () => { + it('serves no sink on a CONNECTED posture either, not only on the air-gapped one', async () => { // The internet-connected on-prem EE box runs the SAME build - // artifact as the hosted console, so "connected therefore allowed" - // would have left the reported injury class open one deployment - // over, for a customer equally unaware of Sentry. + // artifact as the hosted console, so "connected therefore + // configured" would have left the reported injury class open one + // deployment over, for a customer equally unaware of Sentry. process.env.OS_CLOUD_URL = 'https://cloud.objectos.ai'; const { body } = await serve({ controlPlaneUrl: 'https://cloud.objectos.ai' }); - expect(body.telemetry.allowClientErrorReporting).toBe(false); + expect(body.telemetry.errorReporting).toBeUndefined(); }); - it('the key is ALWAYS present, so absence can mean exactly one thing', async () => { + it('the telemetry BLOCK is always present, so one curl distinguishes the two absences', async () => { const { body } = await serve(); expect(Object.prototype.hasOwnProperty.call(body, 'telemetry')).toBe(true); - expect(Object.prototype.hasOwnProperty.call(body.telemetry, 'allowClientErrorReporting')).toBe(true); - // ...and it survives the wire as a real boolean, not as a dropped - // `undefined` — the failure `branding.stage` had to be spelled - // around, pointing the other way. - const parsed = JSON.parse(JSON.stringify(body)); - expect(parsed.telemetry).toEqual({ allowClientErrorReporting: false }); + // `{"telemetry":{}}` — this runtime knows the key and has no DSN. + // A payload with no `telemetry` key at all came from a runtime that + // does not know it. Both deny; only one is fixable here. + expect(JSON.parse(JSON.stringify(body)).telemetry).toEqual({}); }); }); - describe('direction 1 — an explicit grant reaches the payload', () => { - it.each(['1', 'true', 'on', 'yes', 'TRUE', ' yes '])('grants on %j', async (raw) => { - process.env[CLIENT_ERROR_REPORTING_ENV] = raw; + describe('direction 1 — a configured DSN reaches the payload', () => { + it('THE ACCEPTANCE — the env var alone enables reporting, no frontend rebuild', async () => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; const { body, warnings } = await serve(); - expect(body.telemetry.allowClientErrorReporting).toBe(true); + expect(body.telemetry.errorReporting.dsn).toBe(DSN); expect(warnings).toEqual([]); }); - it('the host option grants with no env var set at all', async () => { - const { body } = await serve({ allowClientErrorReporting: true }); - expect(body.telemetry.allowClientErrorReporting).toBe(true); + it('accepts a self-hosted sink as readily as the SaaS one', async () => { + // The validation must not be a vendor-format parser: refusing a + // working self-hosted DSN would hand the operator this card's own + // defect back. + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = SELF_HOSTED_DSN; + const { body } = await serve(); + expect(body.telemetry.errorReporting.dsn).toBe(SELF_HOSTED_DSN); }); - it('the host option wins over the env var — in BOTH directions', async () => { - process.env[CLIENT_ERROR_REPORTING_ENV] = 'false'; - expect((await serve({ allowClientErrorReporting: true })).body.telemetry.allowClientErrorReporting) - .toBe(true); - process.env[CLIENT_ERROR_REPORTING_ENV] = 'true'; - expect((await serve({ allowClientErrorReporting: false })).body.telemetry.allowClientErrorReporting) - .toBe(false); + it('trims a padded DSN rather than refusing it', async () => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = ` ${DSN} `; + expect((await serve()).body.telemetry.errorReporting.dsn).toBe(DSN); + }); + + it('the host option configures it with no env var set at all', async () => { + const { body } = await serve({ clientErrorReporting: { dsn: DSN } }); + expect(body.telemetry.errorReporting.dsn).toBe(DSN); + }); + + it('the host option wins over the env var, per field', async () => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = SELF_HOSTED_DSN; + const { body } = await serve({ clientErrorReporting: { dsn: DSN } }); + expect(body.telemetry.errorReporting.dsn).toBe(DSN); + }); + + it('a host option for ONE knob does not discard the operator DSN', async () => { + // Whole-object replacement was rejected precisely here: a host + // passing only `sendDefaultPii` silently dropping `..._DSN` is the + // quiet two-knob failure this card exists to delete. + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; + const { body } = await serve({ clientErrorReporting: { sendDefaultPii: true } }); + expect(body.telemetry.errorReporting.dsn).toBe(DSN); + expect(body.telemetry.errorReporting.sendDefaultPii).toBe(true); }); it('a same-origin runtime (controlPlaneUrl: "") is NOT a declined control plane', async () => { - // The conflation this key must not inherit: `resolveCloudUrl()` - // returns '' both for "this runtime IS the cloud" and for - // `OS_CLOUD_URL=off`. Reading the posture off that would deny the - // hosted console — the one deployment that legitimately grants. - process.env[CLIENT_ERROR_REPORTING_ENV] = 'true'; + // The conflation this must not inherit: `resolveCloudUrl()` returns + // '' both for "this runtime IS the cloud" and for `OS_CLOUD_URL=off`. + // Reading the posture off that would silence the hosted console — + // the one deployment that legitimately configures a sink. + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; const { body, warnings } = await serve({ controlPlaneUrl: '' }); - expect(body.telemetry.allowClientErrorReporting).toBe(true); + expect(body.telemetry.errorReporting.dsn).toBe(DSN); expect(warnings).toEqual([]); }); }); - describe('direction 2 — an explicit denial, and the vocabulary is closed', () => { - it.each(['0', 'false', 'off', 'no'])('denies on %j, silently (deliberate, not a typo)', async (raw) => { - process.env[CLIENT_ERROR_REPORTING_ENV] = raw; + describe('the closed set of knobs travels WITH the DSN', () => { + it('defaults every knob to its documented value when only a DSN is set', async () => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; + const { body } = await serve(); + expect(body.telemetry.errorReporting).toEqual({ + dsn: DSN, + // OPT-IN. One artifact serves every posture, so PII collection + // is the deliberate choice of the deployment that wants it. + sendDefaultPii: false, + tracesSampleRate: DEFAULT_TRACES_SAMPLE_RATE, + // Replay records what the user did — the most privacy-bearing + // knob in the set, therefore off unless asked for. + replaysOnErrorSampleRate: DEFAULT_REPLAYS_ON_ERROR_SAMPLE_RATE, + }); + // Absent, not empty-string: there IS a sensible client-side answer + // (the SPA's build mode) and inventing one here would assert + // something this side does not know. + expect(Object.prototype.hasOwnProperty.call(body.telemetry.errorReporting, 'environment')) + .toBe(false); + }); + + it.each(['1', 'true', 'on', 'yes', 'TRUE', ' yes '])('opts into PII on %j', async (raw) => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; + process.env[CLIENT_ERROR_REPORTING_PII_ENV] = raw; const { body, warnings } = await serve(); - expect(body.telemetry.allowClientErrorReporting).toBe(false); + expect(body.telemetry.errorReporting.sendDefaultPii).toBe(true); expect(warnings).toEqual([]); }); - it.each(['enable', 'enabled', 'y', 'sure', '2'])('refuses %j rather than guessing', async (raw) => { - process.env[CLIENT_ERROR_REPORTING_ENV] = raw; + it.each(['0', 'false', 'off', 'no'])('keeps PII off on %j, silently', async (raw) => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; + process.env[CLIENT_ERROR_REPORTING_PII_ENV] = raw; + const { body, warnings } = await serve(); + expect(body.telemetry.errorReporting.sendDefaultPii).toBe(false); + expect(warnings).toEqual([]); + }); + + it('carries the operator environment tag and the sample rates', async () => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; + process.env[CLIENT_ERROR_REPORTING_ENVIRONMENT_ENV] = 'staging'; + process.env[CLIENT_ERROR_REPORTING_TRACES_RATE_ENV] = '0.25'; + process.env[CLIENT_ERROR_REPORTING_REPLAY_RATE_ENV] = '1'; + const { body, warnings } = await serve(); + expect(body.telemetry.errorReporting).toMatchObject({ + environment: 'staging', + tracesSampleRate: 0.25, + replaysOnErrorSampleRate: 1, + }); + expect(warnings).toEqual([]); + }); + + it('accepts a rate of exactly 0 rather than reading it as unset', async () => { + // `0` is a real answer ("sample nothing"), and `??`-style coalescing + // would quietly replace it with the default — the operator would + // have turned sampling DOWN and got it turned back up. + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; + process.env[CLIENT_ERROR_REPORTING_TRACES_RATE_ENV] = '0'; + expect((await serve()).body.telemetry.errorReporting.tracesSampleRate).toBe(0); + }); + + it('survives the wire — the whole block round-trips through JSON unchanged', async () => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; + process.env[CLIENT_ERROR_REPORTING_PII_ENV] = 'true'; + process.env[CLIENT_ERROR_REPORTING_ENVIRONMENT_ENV] = 'production'; const { body } = await serve(); - expect(body.telemetry.allowClientErrorReporting).toBe(false); + expect(JSON.parse(JSON.stringify(body)).telemetry.errorReporting) + .toEqual(body.telemetry.errorReporting); }); + }); - it('names the refused value AND the accepted set, so the operator can fix it', async () => { - process.env[CLIENT_ERROR_REPORTING_ENV] = 'enable'; + describe('direction 2 — malformed is REFUSED at mount, never coerced', () => { + it.each([ + ['not a URL at all', 'enable'], + ['a bare host', 'sentry.io/42'], + ['the wrong scheme', 'ftp://abc@o1.ingest.sentry.io/42'], + ['no public key', 'https://o1.ingest.sentry.io/42'], + ['no project id', 'https://abc123@o1.ingest.sentry.io'], + ])('refuses %s and serves no block', async (_label, raw) => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = raw; + const { body } = await serve(); + expect(body.telemetry.errorReporting).toBeUndefined(); + }); + + it('names the refused DSN, the env var and the accepted form, so it is fixable', async () => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = 'enable'; const { warnings } = await serve(); - const warning = warnings.find((w) => w.includes('telemetry switch')); + const warning = warnings.find((w) => w.includes(CLIENT_ERROR_REPORTING_DSN_ENV)); expect(warning).toBeDefined(); expect(warning).toContain('"enable"'); - expect(warning).toContain(CLIENT_ERROR_REPORTING_ENV); + expect(warning).toContain('PUBLIC_KEY'); + expect(warning).toContain('sends no error reports'); + }); + + it('REFUSES A SECRET-BEARING DSN — this payload is read by every browser', async () => { + // Not shape policing. A legacy Sentry DSN carrying a secret after + // the public key would be published to every Console visitor, and + // the value looks entirely ordinary while doing it. + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = 'https://public:secret@o1.ingest.sentry.io/42'; + const { body, warnings } = await serve(); + expect(body.telemetry.errorReporting).toBeUndefined(); + const warning = warnings.find((w) => w.includes(CLIENT_ERROR_REPORTING_DSN_ENV)); + expect(warning).toBeDefined(); + expect(warning).toContain('secret'); + }); + + it('REDACTS the key when quoting a refused DSN — boot logs travel further than config', async () => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = 'https://sup3rsecret@o1.ingest.sentry.io'; + const { warnings } = await serve(); + const warning = warnings.find((w) => w.includes(CLIENT_ERROR_REPORTING_DSN_ENV)); + expect(warning).not.toContain('sup3rsecret'); + // ...and the SHAPE is kept, which is the half the operator needs. + expect(warning).toContain('o1.ingest.sentry.io'); + }); + + it('an EMPTY DSN reads as unset — no block, and silent', async () => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = ' '; + const { body, warnings } = await serve(); + expect(body.telemetry.errorReporting).toBeUndefined(); + expect(warnings).toEqual([]); + }); + + it('a bad SAMPLE RATE takes down only itself — the sink still ships', async () => { + // Refusal always lands on the safer value, and the safer value for + // a volume knob is its default. Silencing error reporting over a + // typo in an unrelated knob would be strictness pointed away from + // the hazard. + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; + process.env[CLIENT_ERROR_REPORTING_TRACES_RATE_ENV] = 'lots'; + const { body, warnings } = await serve(); + expect(body.telemetry.errorReporting.dsn).toBe(DSN); + expect(body.telemetry.errorReporting.tracesSampleRate).toBe(DEFAULT_TRACES_SAMPLE_RATE); + expect(warnings.some((w) => w.includes(CLIENT_ERROR_REPORTING_TRACES_RATE_ENV))).toBe(true); + }); + + it.each(['2', '-0.5', 'lots', ''])('refuses the out-of-range rate %j', (raw) => { + const reading = readClientErrorReportingConfig({ dsn: DSN, tracesSampleRate: raw }); + expect(reading.config?.tracesSampleRate).toBe(DEFAULT_TRACES_SAMPLE_RATE); + }); + + it('a bad PII spelling falls back to OFF and says so', async () => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; + process.env[CLIENT_ERROR_REPORTING_PII_ENV] = 'sure'; + const { body, warnings } = await serve(); + expect(body.telemetry.errorReporting.sendDefaultPii).toBe(false); + const warning = warnings.find((w) => w.includes(CLIENT_ERROR_REPORTING_PII_ENV)); + expect(warning).toBeDefined(); + expect(warning).toContain('"sure"'); for (const accepted of ['1', 'true', 'on', 'yes', '0', 'false', 'off', 'no']) { expect(warning).toContain(accepted); } }); - it('an EMPTY env var reads as unset — denied, and silent', async () => { - process.env[CLIENT_ERROR_REPORTING_ENV] = ' '; + it('reports EVERY wrong knob, not just the first', async () => { + // An operator who mis-set two knobs has two things to fix, and + // hearing about one is how the second stays hidden. + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; + process.env[CLIENT_ERROR_REPORTING_PII_ENV] = 'sure'; + process.env[CLIENT_ERROR_REPORTING_REPLAY_RATE_ENV] = 'always'; + const { warnings } = await serve(); + expect(warnings.some((w) => w.includes(CLIENT_ERROR_REPORTING_PII_ENV))).toBe(true); + expect(warnings.some((w) => w.includes(CLIENT_ERROR_REPORTING_REPLAY_RATE_ENV))).toBe(true); + }); + + it('reports knob refusals even when the DSN is missing too', async () => { + // Both are broken; reporting only the DSN would leave the operator + // fixing it and then meeting the second failure on the next boot. + process.env[CLIENT_ERROR_REPORTING_PII_ENV] = 'sure'; const { body, warnings } = await serve(); - expect(body.telemetry.allowClientErrorReporting).toBe(false); - expect(warnings).toEqual([]); + expect(body.telemetry.errorReporting).toBeUndefined(); + expect(warnings.some((w) => w.includes(CLIENT_ERROR_REPORTING_PII_ENV))).toBe(true); }); }); - describe('the declared-off control plane refuses the grant (the posture ceiling)', () => { + describe('the declared-off control plane refuses to serve the sink (the posture ceiling)', () => { it.each(['off', 'none', 'local', 'disabled', 'OFF', ' off '])( - 'OS_CLOUD_URL=%j overrules a well-spelled grant', + 'OS_CLOUD_URL=%j overrules a well-formed DSN', async (raw) => { process.env.OS_CLOUD_URL = raw; - process.env[CLIENT_ERROR_REPORTING_ENV] = 'true'; + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; const { body } = await serve(); - expect(body.telemetry.allowClientErrorReporting).toBe(false); + expect(body.telemetry.errorReporting).toBeUndefined(); }, ); it('overrules the HOST option too, not only the env var', async () => { process.env.OS_CLOUD_URL = 'off'; - const { body } = await serve({ allowClientErrorReporting: true }); - expect(body.telemetry.allowClientErrorReporting).toBe(false); + const { body } = await serve({ clientErrorReporting: { dsn: DSN } }); + expect(body.telemetry.errorReporting).toBeUndefined(); }); it('catches the decline spelled in the host argument, with no env var at all', async () => { - process.env[CLIENT_ERROR_REPORTING_ENV] = 'true'; + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; const { body } = await serve({ controlPlaneUrl: 'off' }); - expect(body.telemetry.allowClientErrorReporting).toBe(false); + expect(body.telemetry.errorReporting).toBeUndefined(); }); - it('says so — an explicit request refused in silence is the defect this card is about', async () => { + it('says so — an explicit configuration refused in silence is this defect class', async () => { process.env.OS_CLOUD_URL = 'off'; - process.env[CLIENT_ERROR_REPORTING_ENV] = 'true'; + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; const { warnings } = await serve(); - const warning = warnings.find((w) => w.includes('client-error-reporting grant')); + const warning = warnings.find((w) => w.includes('refusing to serve')); expect(warning).toBeDefined(); expect(warning).toContain('OS_CLOUD_URL'); - expect(warning).toContain(CLIENT_ERROR_REPORTING_ENV); + expect(warning).toContain(CLIENT_ERROR_REPORTING_DSN_ENV); + // The key is masked here too — same reason as every other quote. + expect(warning).not.toContain('abc123'); }); - it('stays silent when there was no grant to refuse', async () => { + it('stays silent when there was no DSN to refuse', async () => { process.env.OS_CLOUD_URL = 'off'; const { body, warnings } = await serve(); - expect(body.telemetry.allowClientErrorReporting).toBe(false); - expect(warnings.some((w) => w.includes('client-error-reporting grant'))).toBe(false); + expect(body.telemetry.errorReporting).toBeUndefined(); + expect(warnings.some((w) => w.includes('refusing to serve'))).toBe(false); }); }); - describe('the permission has exactly one author — resolveFeatures cannot grant it', () => { - it('a distribution feature hook returning the key does not move the permission', async () => { + describe('the sink has exactly one author — resolveFeatures cannot supply it', () => { + it('a distribution feature hook returning telemetry keys does not move the sink', async () => { const { body } = await serve({ - resolveFeatures: () => ({ allowClientErrorReporting: true } as any), + resolveFeatures: () => ({ errorReporting: { dsn: DSN } } as any), }); // It may land in the open-ended feature map — that map is the - // distribution's — but the permission is a sibling of it, not a - // member, so billing-tier code cannot reach it. - expect(body.telemetry.allowClientErrorReporting).toBe(false); - expect(isClientErrorReportingAllowed(body)).toBe(false); + // distribution's — but the sink is a sibling of it, not a member, + // so billing-tier code cannot reach it. + expect(body.telemetry.errorReporting).toBeUndefined(); + expect(readClientErrorReporting(body)).toBeNull(); }); }); }); -describe('isClientErrorReportingAllowed — the fail-closed reading (#10805)', () => { - describe('a declining server beats a build-time DSN', () => { - it('an opted-in BUILD sends nothing when the runtime declines', () => { - const declining = { telemetry: { allowClientErrorReporting: false } }; - expect(wouldSendToThirdParty(OPTED_IN_BUILD, declining)).toBe(false); +describe('readClientErrorReporting — the fail-closed reading (#12681)', () => { + describe('a configured runtime is what turns reporting on, and nothing else is', () => { + it('reads the sink out of a runtime that serves one', () => { + const payload = { telemetry: { errorReporting: { dsn: DSN, sendDefaultPii: true } } }; + expect(readClientErrorReporting(payload)).toMatchObject({ dsn: DSN, sendDefaultPii: true }); }); - it('...and sends when the same build meets a runtime that grants', () => { - // The control that makes the line above a reading rather than a - // function that always answers false. - const granting = { telemetry: { allowClientErrorReporting: true } }; - expect(wouldSendToThirdParty(OPTED_IN_BUILD, granting)).toBe(true); - }); - - it('a granting runtime cannot START telemetry on a build with no DSN', () => { - // The permission is a conjunct, never a source: this side supplies - // no sink and must not be able to open one. - const granting = { telemetry: { allowClientErrorReporting: true } }; - expect(wouldSendToThirdParty(undefined, granting)).toBe(false); + it('a runtime serving an empty telemetry block sends nothing', () => { + expect(readClientErrorReporting({ telemetry: {} })).toBeNull(); }); }); describe('absent reads as do-not-send', () => { it('a payload from a runtime that never heard of the key', () => { - // Every field a pre-#10805 runtime really serves, and no telemetry. + // Every field a pre-#12681 runtime really serves, and no telemetry. const legacy = { cloudUrl: '', singleEnvironment: true, @@ -287,29 +460,30 @@ describe('isClientErrorReportingAllowed — the fail-closed reading (#10805)', ( branding: { productName: 'ObjectOS' }, }; expect(Object.prototype.hasOwnProperty.call(legacy, 'telemetry')).toBe(false); - expect(isClientErrorReportingAllowed(legacy)).toBe(false); - expect(wouldSendToThirdParty(OPTED_IN_BUILD, legacy)).toBe(false); + expect(readClientErrorReporting(legacy)).toBeNull(); + }); + + it('a payload from the runtime this card REPLACED — the permission boolean alone', () => { + // The landing-order case stated in both PR bodies: an intermediate + // runtime serving only #10805's boolean carries no source, so a new + // client reads it as off. No dual-spelling window in either + // direction. + expect(readClientErrorReporting({ telemetry: { allowClientErrorReporting: true } })).toBeNull(); }); it.each([ - ['an empty telemetry block', { telemetry: {} }], - ['a present-but-undefined permission', { telemetry: { allowClientErrorReporting: undefined } }], + ['an empty errorReporting block', { telemetry: { errorReporting: {} } }], + ['a present-but-undefined dsn', { telemetry: { errorReporting: { dsn: undefined } } }], + ['an empty-string dsn', { telemetry: { errorReporting: { dsn: '' } } }], + ['a whitespace-only dsn', { telemetry: { errorReporting: { dsn: ' ' } } }], + ['a non-string dsn', { telemetry: { errorReporting: { dsn: 42 } } }], + ['a null errorReporting block', { telemetry: { errorReporting: null } }], + ['an errorReporting block that is not an object', { telemetry: { errorReporting: DSN } }], ['a null telemetry block', { telemetry: null }], ['a telemetry block that is not an object', { telemetry: 'on' }], ['an empty payload', {}], ])('%s', (_label, payload) => { - expect(isClientErrorReportingAllowed(payload)).toBe(false); - }); - - it.each([ - ['the STRING "true"', 'true'], - ['the number 1', 1], - ['the string "yes"', 'yes'], - ['a truthy object', {}], - ])('does not accept %s as the permission', (_label, value) => { - // `=== true`, not truthiness. A consumer should not be taught that - // any truthy shape on this key opens a third-party data flow. - expect(isClientErrorReportingAllowed({ telemetry: { allowClientErrorReporting: value } })).toBe(false); + expect(readClientErrorReporting(payload)).toBeNull(); }); }); @@ -321,8 +495,7 @@ describe('isClientErrorReportingAllowed — the fail-closed reading (#10805)', ( ['the body was not JSON at all', ''], ['the endpoint answered with an array', []], ])('%s', (_label, payload) => { - expect(isClientErrorReportingAllowed(payload)).toBe(false); - expect(wouldSendToThirdParty(OPTED_IN_BUILD, payload)).toBe(false); + expect(readClientErrorReporting(payload)).toBeNull(); }); it('the real thing: a rejected fetch, caught, read through the same function', async () => { @@ -330,41 +503,130 @@ describe('isClientErrorReportingAllowed — the fail-closed reading (#10805)', ( try { throw new TypeError('Failed to fetch'); } catch { - // The whole point of accepting `unknown`: the error path - // is the absent path, so there is no second reading to - // forget to write. + // The whole point of accepting `unknown`: the error path is + // the absent path, so there is no second reading to forget. return undefined; } }; - expect(wouldSendToThirdParty(OPTED_IN_BUILD, await fetchRuntimeConfig())).toBe(false); + expect(readClientErrorReporting(await fetchRuntimeConfig())).toBeNull(); + }); + }); + + describe('the knobs are re-derived defensively, never coerced', () => { + it.each([ + ['the STRING "true"', 'true'], + ['the number 1', 1], + ['the string "yes"', 'yes'], + ['a truthy object', {}], + ])('does not accept %s as a PII opt-in', (_label, value) => { + // `=== true`, not truthiness. A consumer should not be taught that + // any truthy shape on this key opens a PII flow. + const payload = { telemetry: { errorReporting: { dsn: DSN, sendDefaultPii: value } } }; + expect(readClientErrorReporting(payload)?.sendDefaultPii).toBe(false); + }); + + it.each([['a numeric string', '0.5'], ['out of range', 2], ['not a number', NaN]])( + 'falls back to the default rate on %s', + (_label, value) => { + const payload = { telemetry: { errorReporting: { dsn: DSN, tracesSampleRate: value } } }; + expect(readClientErrorReporting(payload)?.tracesSampleRate).toBe(DEFAULT_TRACES_SAMPLE_RATE); + }, + ); + + it('REFUSES a secret-bearing DSN from an untrusted host', () => { + // The one shape check the consumer keeps, because its failure mode + // is a secret published to every browser rather than a + // misconfiguration. A well-behaved ObjectStack runtime never serves + // one, so this can only fire against a third-party host. + const payload = { + telemetry: { errorReporting: { dsn: 'https://public:secret@o1.ingest.sentry.io/42' } }, + }; + expect(readClientErrorReporting(payload)).toBeNull(); + }); + + it('does NOT re-run the producer full shape check', () => { + // A server serving a working DSN that this reader quietly discarded + // would be the two-places-disagreeing failure this card deletes, + // one layer down. `Sentry.init` is the authority on its own format. + const odd = { telemetry: { errorReporting: { dsn: 'https://k@host/1?tunnel=x' } } }; + expect(readClientErrorReporting(odd)?.dsn).toBe('https://k@host/1?tunnel=x'); }); }); describe('the served payload round-trips through JSON into the same verdict', () => { - it('granting', async () => { - process.env[CLIENT_ERROR_REPORTING_ENV] = 'true'; + const savedDsn = process.env[CLIENT_ERROR_REPORTING_DSN_ENV]; + afterEach(() => { + if (savedDsn === undefined) delete process.env[CLIENT_ERROR_REPORTING_DSN_ENV]; + else process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = savedDsn; + }); + + it('configured — and every field the producer emitted survives the reader', async () => { + process.env[CLIENT_ERROR_REPORTING_DSN_ENV] = DSN; const { body } = await serve(); - delete process.env[CLIENT_ERROR_REPORTING_ENV]; - expect(isClientErrorReportingAllowed(JSON.parse(JSON.stringify(body)))).toBe(true); + delete process.env[CLIENT_ERROR_REPORTING_DSN_ENV]; + // Producer-accepted implies consumer-accepted: the two sides must + // never disagree about a DSN, or the operator gets a silent dead + // state in the one place this card removed it from. + expect(readClientErrorReporting(JSON.parse(JSON.stringify(body)))) + .toEqual(body.telemetry.errorReporting); }); - it('declining', async () => { + it('unconfigured', async () => { const { body } = await serve(); - expect(isClientErrorReportingAllowed(JSON.parse(JSON.stringify(body)))).toBe(false); + expect(readClientErrorReporting(JSON.parse(JSON.stringify(body)))).toBeNull(); }); }); }); -describe('readClientErrorReportingGrant — the closed switch vocabulary (#10805)', () => { - it('unset is unset: denied, and not a refusal', () => { - expect(readClientErrorReportingGrant(undefined)).toEqual({ allowed: false }); +describe('readClientErrorReportingConfig — the pure resolution (#12681)', () => { + it('unset is unset: no config, and not a refusal', () => { + expect(readClientErrorReportingConfig({})).toEqual({ refusals: [] }); + }); + + it('an unrecognised DSN is refused AND held for reporting', () => { + const reading = readClientErrorReportingConfig({ dsn: 'enable' }); + expect(reading.config).toBeUndefined(); + expect(reading.refusals).toHaveLength(1); + expect(reading.refusals[0]).toMatchObject({ env: CLIENT_ERROR_REPORTING_DSN_ENV, value: 'enable' }); + }); + + it('believes a typed host boolean without running it through the string vocabulary', () => { + expect(readClientErrorReportingConfig({ dsn: DSN, sendDefaultPii: true }).config?.sendDefaultPii) + .toBe(true); + }); + + it('refuses a non-string, non-boolean knob rather than coercing it', () => { + // A JS host outside the type system can hand over anything at all. + const reading = readClientErrorReportingConfig({ dsn: DSN, sendDefaultPii: {} }); + expect(reading.config?.sendDefaultPii).toBe(false); + expect(reading.refusals).toHaveLength(1); + }); + + it('reads a typed host number for a sample rate', () => { + expect(readClientErrorReportingConfig({ dsn: DSN, tracesSampleRate: 0.5 }).config?.tracesSampleRate) + .toBe(0.5); + }); +}); + +describe('redactDsn — boot logs travel further than the configuration they quote', () => { + it('masks the public key and keeps the shape', () => { + expect(redactDsn(DSN)).toBe('https://***@o1.ingest.sentry.io/42'); + }); + + it('masks a secret-bearing DSN too, both halves of the userinfo', () => { + expect(redactDsn('https://public:secret@o1.ingest.sentry.io/42')) + .toBe('https://***@o1.ingest.sentry.io/42'); + }); + + it('still masks a value that does NOT parse as a URL — those are the ones it sees', () => { + expect(redactDsn('htp://key@host/1')).toBe('htp://***@host/1'); }); - it('an unrecognised spelling is denied AND held for reporting', () => { - expect(readClientErrorReportingGrant('enable')).toEqual({ allowed: false, refused: 'enable' }); + it('leaves a value with no userinfo alone', () => { + expect(redactDsn('enable')).toBe('enable'); }); - it('keeps the operator original spelling, untrimmed, so the diagnostic quotes what they typed', () => { - expect(readClientErrorReportingGrant(' Enable ').refused).toBe(' Enable '); + it('truncates something pasted in by mistake', () => { + expect(redactDsn('x'.repeat(500))).toHaveLength(121); }); }); diff --git a/packages/cloud-connection/src/telemetry-posture.ts b/packages/cloud-connection/src/telemetry-posture.ts index f449785ff8..bfcc53da86 100644 --- a/packages/cloud-connection/src/telemetry-posture.ts +++ b/packages/cloud-connection/src/telemetry-posture.ts @@ -1,69 +1,122 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. /** - * `telemetry.allowClientErrorReporting` — the runtime's post-build permission - * for SPA client telemetry (#10805, upstream half of cloud#1508). + * `telemetry.errorReporting` — the client error-reporting SOURCE, served by + * the operator's own runtime (#12681, superseding the #10805 permission). * * ## The injury this exists to end * * An air-gapped on-premises EE Console was measured sending 14 Sentry * envelopes per session to `sentry.io`, carrying IP + User-Agent PII, with no - * way for the customer to turn it off. objectui closed one half (a build that - * never opts in now issues no third-party request at all), and could not close - * the other: every knob there is a Vite build-time variable that is inlined - * into the bundle as a frozen literal, so a build that DID opt in — the hosted - * console, and the identical artifact shipped on-prem — has no post-build off - * switch. Editing env vars on the deployed host does nothing. The only - * server-to-SPA channel is `GET /api/v1/runtime/config`, which this package - * owns, so the switch has to be a key on that payload. - * - * ## The permission is a CONJUNCT, never a source - * - * The server never supplies a DSN and cannot turn telemetry ON for a build - * that carries none. `allowClientErrorReporting: true` means only "this - * deployment does not object to the sink your build was compiled with"; the - * consumer still needs its own build-time DSN. Deliberate: a server that could - * *start* a third-party data flow in someone's browser is a strictly worse - * surface than the one being fixed. So the composed decision is - * `Boolean(buildTimeDsn) && isClientErrorReportingAllowed(payload)`, and this - * side owns the second conjunct only. - * - * ## Why a positive permission and not a negative kill switch - * - * Spelling it `telemetry: { disabled: true }` would have read as `undefined` - * on every server too old to know the key, on every malformed payload, and on - * every failed fetch — falsy, therefore "not disabled", therefore SEND. The - * gate would be vacuous exactly where it is needed: the legacy runtimes that - * are leaking today. Phrased as a permission that must be positively granted, - * every one of those states collapses onto "not `true`" and denies. The - * fail-closed reading is then a property of the VOCABULARY rather than a - * discipline each consumer has to remember. - * - * The boolean is chosen over a `'allowed' | 'denied'` union for the same - * reason: the shortest expression a consumer can write — `if (allowed)` — is - * already the safe one, whereas the laziest string test (`!== 'denied'`) fails - * OPEN on absence. - * - * ## Absence - * - * `RuntimeConfigPlugin` always emits the key, so absence never means "this - * server had no opinion" — it means the payload did not come from a runtime - * that knows about it (an older ObjectStack, a third-party host, a 404, a - * network error). {@link isClientErrorReportingAllowed} answers `false` for - * every one of those, and that reading is the contract, not an implementation - * detail: see its own note for why the producer owns it. + * way for the customer to turn it off. The first fix (#10805) shipped a + * runtime PERMISSION and left the SOURCE where it was: a build-time + * `VITE_SENTRY_DSN` inlined into the published bundle. That closed the leak + * and opened a different hole, which the maintainer named on 2026-08-27, + * verbatim and untranslated: + * + * > 「我是一个开发平台呀，我的用户并不会去构建我的前端，我理解这种应该在服务端传进去。」 + * + * ObjectStack's users consume a PREBUILT console. They cannot set a build-time + * key, so under the two-key gate a self-hosting operator could not enable + * client error reporting at all: the permission was reachable and the source + * was not. Both halves now live on `GET /api/v1/runtime/config`, which this + * package owns, so the operator configures telemetry in exactly one place. + * + * ## The DSN's presence IS the grant + * + * There is no separate boolean. A runtime that serves a DSN is asking for + * reports; a runtime that serves none is not. That is not a shorthand — it is + * what removes the failure mode the two-key shape had: with a permission and a + * source configured in different places, "permission on, no DSN" and "DSN in, + * permission off" are two silent dead states that look identical from the + * browser. One knob cannot disagree with itself. + * + * The fail-closed direction survives the collapse for free, and more robustly + * than the boolean managed. The grant is now "a non-empty DSN string reached + * me", so every indeterminate state — an older runtime that never heard of the + * key, a third-party host, a 404, a network error, a malformed body, a payload + * that has not arrived yet — carries no DSN and therefore denies. A boolean + * needed `=== true` and a written argument about why `disabled: true` would + * have been vacuous; a source needs neither, because absence of a source is + * not a value that can be misread. + * + * ## Everything that must travel with the DSN travels WITH it + * + * {@link ClientErrorReportingConfig} is a CLOSED enumeration, served as one + * object, and it is closed for the same reason the DSN moved: a knob the + * platform user cannot reach is a knob that does not exist for them. The + * previous shape scattered these across build-time `VITE_SENTRY_*` variables, + * where a prebuilt-console consumer could set none of them — including + * `sendDefaultPii`, the one that decides whether IP + User-Agent leave the + * network. Relocating them is not new surface; it is the same surface moved to + * the side that can actually operate it. + * + * One knob deliberately did NOT move: `VITE_SENTRY_RELEASE`. A release + * identifies WHICH BUNDLE produced a stack trace and has to match the source + * maps that bundle's pipeline uploaded. That is a property of the build, and a + * server cannot know which console build it is serving. It stays build-time in + * objectui, and it is the only `VITE_SENTRY_*` knob that does. + * + * ## Malformed is REFUSED, never coerced + * + * The strictness precedent this file already set for the retired boolean, and + * that `asPlatformStage` sets for `branding.stage`: a knob that appears to work + * while doing nothing is how this whole family of defects reaches production. + * Every refusal below is reported once at mount time, naming the knob, the + * value and the accepted form — see {@link TelemetryRefusal}. + * + * Refusal always lands on the SAFER value, which is what decides whether a bad + * knob takes down the whole block or only itself: + * + * - a malformed **DSN** refuses the whole block — there is no safe default for + * a source, and serving none is the safe direction; + * - a malformed **sendDefaultPii** falls back to `false`, a malformed sample + * rate to its documented default. Silencing error reporting because of a + * typo in an unrelated volume knob would be strictness pointed away from + * the hazard. */ /** - * Operator switch that grants the permission. Boolean feature flag, - * default-off / opt-in, per the `OS_{DOMAIN}_{FEATURE}_ENABLED` rule. + * The DSN — the source, and therefore the grant. Follows the + * `OS_{DOMAIN}_{FEATURE}_{KNOB}` rule and keeps the + * `..._CLIENT_ERROR_REPORTING_...` family the retired boolean established, so + * the replacement reads as the same knob rather than as a new one. * - * Named for the narrow thing it grants rather than for "telemetry": a later - * sibling permission (session replay, product analytics) must be a SEPARATE - * grant, and an operator who set a var called `OS_CONSOLE_TELEMETRY_ENABLED` - * would reasonably read it as having already granted those too. + * Named for the narrow thing it configures rather than for the vendor: a + * later sibling capability (session replay of every session, product + * analytics) must be a SEPARATE knob, and an operator who set something called + * `OS_TELEMETRY_SENTRY_*` would reasonably read it as having configured those + * too. It also keeps a vendor out of an operator-facing contract that a + * self-hosted or DSN-compatible sink satisfies equally well. + */ +export const CLIENT_ERROR_REPORTING_DSN_ENV = 'OS_TELEMETRY_CLIENT_ERROR_REPORTING_DSN'; + +/** Opt in to attaching IP address + User-Agent to events. */ +export const CLIENT_ERROR_REPORTING_PII_ENV = 'OS_TELEMETRY_CLIENT_ERROR_REPORTING_SEND_DEFAULT_PII'; + +/** The `environment` tag on emitted events (`production`, `staging`, ...). */ +export const CLIENT_ERROR_REPORTING_ENVIRONMENT_ENV = 'OS_TELEMETRY_CLIENT_ERROR_REPORTING_ENVIRONMENT'; + +/** Fraction of transactions sampled for performance tracing. */ +export const CLIENT_ERROR_REPORTING_TRACES_RATE_ENV = 'OS_TELEMETRY_CLIENT_ERROR_REPORTING_TRACES_SAMPLE_RATE'; + +/** Fraction of ERROR sessions recorded as session replays. */ +export const CLIENT_ERROR_REPORTING_REPLAY_RATE_ENV = 'OS_TELEMETRY_CLIENT_ERROR_REPORTING_REPLAY_SAMPLE_RATE'; + +/** + * Default transaction sampling. Carried here rather than left to the consumer + * so the served object is complete: a consumer filling in its own default is a + * consumer that has an opinion the operator cannot override. */ -export const CLIENT_ERROR_REPORTING_ENV = 'OS_TELEMETRY_CLIENT_ERROR_REPORTING_ENABLED'; +export const DEFAULT_TRACES_SAMPLE_RATE = 0.1; + +/** + * Default session-replay sampling: OFF. Replay records what the user did, so + * it is the most privacy-bearing knob in the set and must be the deliberate + * choice of the deployment that wants it — never an inherited default of the + * deployment that does not. + */ +export const DEFAULT_REPLAYS_ON_ERROR_SAMPLE_RATE = 0; /** * The truthy vocabulary the whole repo's opt-in flags answer to @@ -76,77 +129,327 @@ const GRANT_SPELLINGS: readonly string[] = ['1', 'true', 'on', 'yes']; /** The matching falsy vocabulary — a DELIBERATE denial, not a typo, so silent. */ const DENY_SPELLINGS: readonly string[] = ['0', 'false', 'off', 'no']; -/** Every spelling the switch accepts, in the order a diagnostic lists them. */ -export const CLIENT_ERROR_REPORTING_SPELLINGS: readonly string[] = [ +/** Every spelling a boolean knob accepts, in the order a diagnostic lists them. */ +export const TELEMETRY_BOOLEAN_SPELLINGS: readonly string[] = [ ...GRANT_SPELLINGS, ...DENY_SPELLINGS, ]; -/** What an operator's raw switch value resolved to. */ -export interface TelemetryGrantReading { - /** The permission. Fail-closed: only a recognised grant spelling is `true`. */ - readonly allowed: boolean; +/** + * The client error-reporting configuration served on + * `/api/v1/runtime/config`, as ONE object. + * + * A CLOSED enumeration. Anything an operator cannot set here, they cannot set + * at all — which is the point, and also the reason to keep the set small: each + * key is an operator-facing contract that has to keep working. + */ +export interface ClientErrorReportingConfig { + /** + * The sink. Non-empty by construction: this object is served only when a + * well-formed DSN resolved, so a consumer holding one is holding a grant. + */ + readonly dsn: string; + /** May IP address + User-Agent be attached to events? Opt-in. */ + readonly sendDefaultPii: boolean; /** - * The rejected spelling, when the operator said something outside the - * closed set. Held rather than warned about here so the caller can report - * it once, at mount time, where it has a logger. + * The `environment` tag. Optional because there is a sensible client-side + * answer when the operator has no opinion (the SPA's own build mode), and + * inventing one here would assert something this side does not know. */ - readonly refused?: string; + readonly environment?: string; + /** Transaction sampling, `0`..`1`. */ + readonly tracesSampleRate: number; + /** Error-session replay sampling, `0`..`1`. */ + readonly replaysOnErrorSampleRate: number; } -const DENIED: TelemetryGrantReading = { allowed: false }; -const GRANTED: TelemetryGrantReading = { allowed: true }; - /** - * Read the operator's switch through a CLOSED vocabulary. + * The telemetry block served on `/api/v1/runtime/config`. + * + * A namespace of its own — deliberately NOT a member of `features`. That map + * is open-ended and a host's `resolveFeatures` hook merges arbitrary keys into + * it verbatim, so a distribution's plan policy could hand out a telemetry sink + * from code whose subject is billing tiers. A security-bearing configuration + * must have exactly one author. The separation is pinned by test. + * + * The block itself is ALWAYS served; `errorReporting` is present only when a + * DSN resolved. That pair of facts is deliberate and is what a single `curl` + * has to be able to distinguish: * - * An unrecognised spelling is REFUSED and reported, never coerced — the same - * discipline `asPlatformStage` applies to `branding.stage`, and for the same - * reason: a knob that appears to work while doing nothing is how this whole - * family of defects reaches production. Here the refusal also happens to be - * the safe direction, but the diagnostic is what the operator needs, because - * their intent (`=enable`, `=Y`) was to grant and nothing would have said - * otherwise. + * `{"telemetry":{}}` this runtime knows the key and has no DSN + * no `telemetry` key at all this payload did not come from a runtime + * that knows the key * - * Unset, empty, or whitespace-only is UNSET, not a typo: denied, and silent. + * Both deny. Serving an explicit `errorReporting: null` for the first case was + * rejected: it is a second spelling of a state absence already expresses, and a + * consumer would have to handle both anyway — the older-runtime case cannot be + * spelled by a runtime that does not exist yet. */ -export function readClientErrorReportingGrant(raw: string | undefined): TelemetryGrantReading { - if (raw === undefined) return DENIED; - const value = raw.trim().toLowerCase(); - if (value === '') return DENIED; - if (GRANT_SPELLINGS.includes(value)) return GRANTED; - if (DENY_SPELLINGS.includes(value)) return DENIED; - return { allowed: false, refused: raw }; +export interface RuntimeTelemetryPosture { + /** The sink and its knobs. Absent means no client error reporting. */ + readonly errorReporting?: ClientErrorReportingConfig; } /** - * The telemetry block served on `/api/v1/runtime/config`. + * One knob the operator got wrong, held for a single mount-time diagnostic. * - * A namespace of its own — deliberately NOT a member of `features`. That map - * is open-ended and a host's `resolveFeatures` hook merges arbitrary keys into - * it verbatim, so a distribution's plan policy could grant this permission by - * returning one boolean, from code whose subject is billing tiers. A security - * permission must have exactly one author. The separation is pinned by test. + * Held rather than warned about at construction: the constructor has no + * logger, and a silently dropped operator knob is exactly the thing that must + * not be invisible from the SPA end. */ -export interface RuntimeTelemetryPosture { +export interface TelemetryRefusal { + /** The env var name, so the operator can grep their own configuration. */ + readonly env: string; + /** What they said, DSN-redacted — see {@link redactDsn}. */ + readonly value: string; + /** The accepted form, phrased for someone fixing it right now. */ + readonly accepted: string; + /** What the runtime did instead, so silence is never left unexplained. */ + readonly consequence: string; +} + +/** What an operator's raw telemetry configuration resolved to. */ +export interface ClientErrorReportingReading { /** - * May the SPA send client error reports to the sink its build was - * compiled with? `false` unless a runtime positively granted it. + * The resolved configuration, or `undefined` for "serve no + * `errorReporting` block" — unset, or a DSN that was refused. */ - readonly allowClientErrorReporting: boolean; + readonly config?: ClientErrorReportingConfig; + /** Everything refused along the way. Empty on the ordinary paths. */ + readonly refusals: readonly TelemetryRefusal[]; +} + +/** + * The raw, untyped configuration as it arrives from a host option or an env + * var, before any of it is believed. + * + * Every field is `unknown` on purpose. Env vars are strings, host options are + * typed, and a JS host outside the type system can hand over anything at all; + * one validating reader for all three doors is what keeps the doors from + * disagreeing. + */ +export interface ClientErrorReportingSource { + readonly dsn?: unknown; + readonly sendDefaultPii?: unknown; + readonly environment?: unknown; + readonly tracesSampleRate?: unknown; + readonly replaysOnErrorSampleRate?: unknown; +} + +/** + * Render a DSN safe to put in a log line: the public key is masked, the shape + * is kept. + * + * A Sentry public key is designed to be public — it ships inside the browser + * bundle — so this is not a secret-protection measure. It is a + * log-aggregation hygiene measure: boot logs travel further than the + * configuration they quote, and an operator diagnosing a refusal needs the + * SHAPE of what they typed (scheme, host, project path), never the key. + * + * The regex runs whether or not the value parses as a URL, because the values + * reaching this function are by definition the ones that did not parse. + */ +export function redactDsn(raw: string): string { + const masked = raw.replace(/\/\/[^/@\s]*@/, '//***@'); + return masked.length > 120 ? `${masked.slice(0, 120)}…` : masked; +} + +/** Read a boolean knob through the closed vocabulary. `undefined` = unset. */ +function readBoolean( + value: unknown, + env: string, + consequence: string, + refusals: TelemetryRefusal[], +): boolean | undefined { + if (value === undefined || value === null) return undefined; + // A real boolean from a typed host is the one shape needing no vocabulary. + if (typeof value === 'boolean') return value; + if (typeof value !== 'string') { + refusals.push({ + env, + value: String(value), + accepted: TELEMETRY_BOOLEAN_SPELLINGS.join(', '), + consequence, + }); + return undefined; + } + const normalised = value.trim().toLowerCase(); + // Unset, empty or whitespace-only is UNSET, not a typo: silent. + if (normalised === '') return undefined; + if (GRANT_SPELLINGS.includes(normalised)) return true; + if (DENY_SPELLINGS.includes(normalised)) return false; + refusals.push({ + env, + value, + accepted: TELEMETRY_BOOLEAN_SPELLINGS.join(', '), + consequence, + }); + return undefined; +} + +/** Read a `0`..`1` sample rate. `undefined` = unset. */ +function readSampleRate( + value: unknown, + env: string, + fallback: number, + refusals: TelemetryRefusal[], +): number | undefined { + if (value === undefined || value === null) return undefined; + if (typeof value === 'string' && value.trim() === '') return undefined; + // `Number('')` is 0 and `Number(' ')` is 0 — both would read as "sample + // nothing", which is a plausible-looking answer to a question the operator + // never answered. Handled above so this stays a real parse. + const parsed = typeof value === 'number' ? value : Number(String(value).trim()); + if (Number.isFinite(parsed) && parsed >= 0 && parsed <= 1) return parsed; + refusals.push({ + env, + value: String(value), + accepted: 'a number between 0 and 1 inclusive', + consequence: `falling back to the default of ${fallback}`, + }); + return undefined; +} + +/** + * Validate a DSN, or explain why not. + * + * The checks are the ones every real DSN passes and every typo fails — + * deliberately not a vendor-format parser, which would refuse a valid + * self-hosted or DSN-compatible sink and hand the operator a defect this card + * exists to remove. + * + * One check is not shape policing but a leak guard: a DSN carrying a PASSWORD + * (the deprecated Sentry *secret* key) must never be served, because this + * payload is read by every browser that loads the Console. An operator pasting + * a legacy secret-bearing DSN would be publishing that secret to every visitor, + * and the value looks entirely ordinary while doing it. + */ +function readDsn(value: unknown, refusals: TelemetryRefusal[]): string | undefined { + const CONSEQUENCE = 'no telemetry.errorReporting block is served and the Console sends no error reports'; + const ACCEPTED = 'an http(s) DSN of the form https://PUBLIC_KEY@HOST/PROJECT_ID'; + if (value === undefined || value === null) return undefined; + if (typeof value !== 'string') { + refusals.push({ + env: CLIENT_ERROR_REPORTING_DSN_ENV, + value: String(value), + accepted: ACCEPTED, + consequence: CONSEQUENCE, + }); + return undefined; + } + const dsn = value.trim(); + // Unset is unset: denied, and silent. An operator who never configured + // telemetry is not making a mistake, and a warning on every boot is a + // muted warning. + if (dsn === '') return undefined; + + const refuse = (accepted: string): undefined => { + refusals.push({ + env: CLIENT_ERROR_REPORTING_DSN_ENV, + value: redactDsn(dsn), + accepted, + consequence: CONSEQUENCE, + }); + return undefined; + }; + + let url: URL; + try { + url = new URL(dsn); + } catch { + return refuse(ACCEPTED); + } + if (url.protocol !== 'https:' && url.protocol !== 'http:') return refuse(ACCEPTED); + if (!url.hostname) return refuse(ACCEPTED); + if (!url.username) return refuse('a DSN carrying a public key, as in https://PUBLIC_KEY@HOST/PROJECT_ID'); + if (url.password) { + return refuse( + 'a DSN with NO secret after the public key — this payload is read by every browser that ' + + 'loads the Console, so a secret-bearing (legacy) DSN would publish that secret. Reissue ' + + 'the DSN without it', + ); + } + // The project id: the last non-empty path segment. + const projectId = url.pathname.split('/').filter(Boolean).pop(); + if (!projectId) return refuse('a DSN ending in a project id, as in https://PUBLIC_KEY@HOST/PROJECT_ID'); + return dsn; +} + +/** + * Resolve the operator's raw configuration into the object to serve. + * + * Pure, and takes its input rather than reading `process.env`, so the whole + * refusal matrix is testable without a live environment — the half of the + * retired boolean's design that was worth keeping. + */ +export function readClientErrorReportingConfig( + source: ClientErrorReportingSource, +): ClientErrorReportingReading { + const refusals: TelemetryRefusal[] = []; + + const dsn = readDsn(source.dsn, refusals); + + const sendDefaultPii = readBoolean( + source.sendDefaultPii, + CLIENT_ERROR_REPORTING_PII_ENV, + 'IP address and User-Agent stay OFF', + refusals, + ); + const tracesSampleRate = readSampleRate( + source.tracesSampleRate, + CLIENT_ERROR_REPORTING_TRACES_RATE_ENV, + DEFAULT_TRACES_SAMPLE_RATE, + refusals, + ); + const replaysOnErrorSampleRate = readSampleRate( + source.replaysOnErrorSampleRate, + CLIENT_ERROR_REPORTING_REPLAY_RATE_ENV, + DEFAULT_REPLAYS_ON_ERROR_SAMPLE_RATE, + refusals, + ); + + let environment: string | undefined; + if (source.environment !== undefined && source.environment !== null) { + if (typeof source.environment === 'string') { + environment = source.environment.trim() || undefined; + } else { + refusals.push({ + env: CLIENT_ERROR_REPORTING_ENVIRONMENT_ENV, + value: String(source.environment), + accepted: 'a non-empty string', + consequence: 'the Console tags events with its own build mode instead', + }); + } + } + + // No DSN, no block. Note the knob refusals are still reported: an operator + // who mis-set a sample rate AND never set a DSN has two things to fix, and + // hearing about one of them is how the second stays hidden. + if (dsn === undefined) return { refusals }; + + return { + config: { + dsn, + sendDefaultPii: sendDefaultPii === true, + ...(environment !== undefined ? { environment } : {}), + tracesSampleRate: tracesSampleRate ?? DEFAULT_TRACES_SAMPLE_RATE, + replaysOnErrorSampleRate: replaysOnErrorSampleRate ?? DEFAULT_REPLAYS_ON_ERROR_SAMPLE_RATE, + }, + refusals, + }; } /** * The canonical fail-closed reading of a `/api/v1/runtime/config` payload. * - * ## Why the producer owns the consumer's test + * ## Why the producer owns the consumer's read * - * "Absent reads as do-not-send" is the whole guarantee, and it is a claim - * about code that does NOT live here — every consumer writing its own `?.` - * chain is one `!== false` away from re-opening the leak, silently, on exactly - * the legacy payloads the guarantee is for. So the reading ships with the - * contract: one strict function, pinned in both directions here, rather than N - * dialects accumulating in the consumers (Prime Directive #12). + * "No DSN means do not send" is the whole guarantee, and it is a claim about + * code that does NOT live here — every consumer writing its own `?.` chain is + * one loose truthiness check away from re-opening the leak, silently, on + * exactly the legacy payloads the guarantee is for. So the reading ships with + * the contract: one strict function, pinned in both directions here, rather + * than N dialects accumulating in the consumers (Prime Directive #12). * * Accepts `unknown` on purpose. Callers hand it a parsed HTTP body, and the * "the fetch failed" case is spelled by passing `undefined` or `null` — so the @@ -154,13 +457,68 @@ export interface RuntimeTelemetryPosture { * function, instead of the error path being a `catch` block someone forgot to * write. * - * The test is `=== true`, not truthiness: the string `'true'`, `1`, and - * `'yes'` are payloads a consumer should not be teaching itself to accept. On - * the wire the value is produced by `RuntimeConfigPlugin` as a real boolean. + * ## What it re-validates, and what it deliberately does not + * + * The DSN is accepted as any non-empty string. It is NOT re-run through the + * producer's full shape check: a server that serves a working DSN this reader + * quietly discards would be the two-places-disagreeing failure this card + * exists to delete, one layer down. `Sentry.init` is the authority on whether + * its own DSN parses. + * + * The single exception is a DSN carrying a PASSWORD, which is refused here as + * well as at the producer. That check is not shape policing — its failure mode + * is a secret published to every browser that loads the page, and this reader + * is the last thing standing between an untrusted payload and that outcome. + * A well-behaved ObjectStack runtime never serves one, so the check can only + * fire against a third-party host. + * + * The knobs travelling with the DSN are re-derived defensively: only a real + * `true` opts into PII, and only a finite `0`..`1` moves a sample rate. A + * consumer should not be taught that `'true'` or `'yes'` on the wire opens a + * data flow. */ -export function isClientErrorReportingAllowed(runtimeConfig: unknown): boolean { - if (typeof runtimeConfig !== 'object' || runtimeConfig === null) return false; +export function readClientErrorReporting(runtimeConfig: unknown): ClientErrorReportingConfig | null { + if (typeof runtimeConfig !== 'object' || runtimeConfig === null) return null; const telemetry = (runtimeConfig as { telemetry?: unknown }).telemetry; - if (typeof telemetry !== 'object' || telemetry === null) return false; - return (telemetry as { allowClientErrorReporting?: unknown }).allowClientErrorReporting === true; + if (typeof telemetry !== 'object' || telemetry === null) return null; + const block = (telemetry as { errorReporting?: unknown }).errorReporting; + if (typeof block !== 'object' || block === null) return null; + + const raw = block as Record; + if (typeof raw.dsn !== 'string') return null; + const dsn = raw.dsn.trim(); + if (dsn === '') return null; + if (carriesSecret(dsn)) return null; + + return { + dsn, + sendDefaultPii: raw.sendDefaultPii === true, + ...(typeof raw.environment === 'string' && raw.environment.trim() + ? { environment: raw.environment.trim() } + : {}), + tracesSampleRate: sampleRateOr(raw.tracesSampleRate, DEFAULT_TRACES_SAMPLE_RATE), + replaysOnErrorSampleRate: sampleRateOr( + raw.replaysOnErrorSampleRate, + DEFAULT_REPLAYS_ON_ERROR_SAMPLE_RATE, + ), + }; +} + +/** Does this DSN carry a secret after the public key? See the reader's note. */ +function carriesSecret(dsn: string): boolean { + try { + return new URL(dsn).password !== ''; + } catch { + // Unparseable here means `Sentry.init` will reject it too. Not this + // function's question, and answering `true` would silently discard it + // for the wrong stated reason. + return false; + } +} + +/** A finite `0`..`1` rate, or the default. Never a coerced string. */ +function sampleRateOr(value: unknown, fallback: number): number { + return typeof value === 'number' && Number.isFinite(value) && value >= 0 && value <= 1 + ? value + : fallback; }