diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 41be9c9ee7..9aae3c6d06 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -17,7 +17,11 @@ updates: # Dependabot's npm parser reads only dependencies, devDependencies, and # optionalDependencies — never peerDependencies. It cannot see the # optional OpenTelemetry peer dependencies in server/package.json, so it - # never bumps their declared versions. + # never bumps their declared versions. The same limit applies to the + # optional @sentry/node peer dependency in server/package.json: Dependabot + # cannot bump it either, for the same reason. @sentry/browser stays a + # normal devDependency of ui/package.json, so Dependabot does track that + # one. ignore: # @types/node describes the APIs available in the supported Node runtime. # Runtime major upgrades are deliberate compatibility changes, so keep diff --git a/README.md b/README.md index cea39e55c8..601e0252a1 100644 --- a/README.md +++ b/README.md @@ -469,7 +469,7 @@ Find Plugins and more at [awesome-paperclip](https://github.com/gsxdsm/awesome-p Paperclip ships with opt-in OpenTelemetry auto-instrumentation for the server (traces only). It activates when `OTEL_EXPORTER_OTLP_ENDPOINT` is set and supports `grpc`, `http/protobuf`, and `http/json` via the standard `OTEL_EXPORTER_OTLP_PROTOCOL` env var. `@opentelemetry/api` is a normal server dependency; the SDK, auto-instrumentation, and exporter packages are optional peer dependencies — install them only if you want tracing. See [doc/observability.md](doc/observability.md) for install commands and the full env-var reference. -Paperclip also ships with opt-in Sentry error monitoring for the server and the browser. Set `SENTRY_DSN` to activate it — the server and the browser then report to the same Sentry project. `@sentry/node` is an optional peer dependency for the server; install it only if you want error monitoring. See [doc/observability.md](doc/observability.md#sentry-error-monitoring) for the install command, the privacy settings, and the full default capture set. +Paperclip also ships with opt-in Sentry error monitoring for the server and the browser. Set `SENTRY_DSN` to activate it — the server and the browser then report to the same Sentry project. The supported server SDK version is `@sentry/node@10.71.0`; it is an optional peer dependency for the server, so install it only if you want error monitoring. The browser SDK, `@sentry/browser`, is pinned to the same exact version. See [doc/observability.md](doc/observability.md#sentry-error-monitoring) for the install command, the privacy settings, and the full default capture set. ## Telemetry diff --git a/doc/observability.md b/doc/observability.md index 2a7865308b..3c25a9e5ec 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -148,19 +148,27 @@ imports no Sentry package. The browser fetches no Sentry chunk. #### 1. Install the Sentry peer dependency -Install `@sentry/node` in the server, the same way you install the +The supported server SDK version is **`@sentry/node@10.71.0`** — the exact +version this feature is audited against (see "Server request data" +below). Install it in the server, the same way you install the OpenTelemetry packages above. `@sentry/node` is an *optional peer dependency*: it is not in the default lockfile, and the server loads it -dynamically only when `SENTRY_DSN` is set. +dynamically only when `SENTRY_DSN` is set. `server/package.json` declares +this exact version; installing a different version defeats the audit, so +the server checks the installed version against the declared one at +startup and logs one diagnostic instead of enabling error monitoring on a +mismatch (see "Server request data" below). ```bash -pnpm add @sentry/node +pnpm add @sentry/node@10.71.0 ``` The browser package, `@sentry/browser`, needs no install step. It is -already a development dependency of the `ui` package, so the browser code -ships inside every build. A signed-out browser, or a browser with no DSN, -never fetches the Sentry chunk — see "DSN delivery to the browser" below. +already a development dependency of the `ui` package, pinned to the same +exact version, **`10.71.0`**, so the browser code ships inside every +build at the audited version. A signed-out browser, or a browser with no +DSN, never fetches the Sentry chunk — see "DSN delivery to the browser" +below. #### 2. Set the environment diff --git a/server/package.json b/server/package.json index f3a057cd4e..3bc3f0e0c8 100644 --- a/server/package.json +++ b/server/package.json @@ -107,7 +107,8 @@ "@opentelemetry/exporter-trace-otlp-proto": "0.221.0", "@opentelemetry/resources": "2.10.0", "@opentelemetry/sdk-node": "0.221.0", - "@opentelemetry/semantic-conventions": "1.43.0" + "@opentelemetry/semantic-conventions": "1.43.0", + "@sentry/node": "10.71.0" }, "peerDependenciesMeta": { "@opentelemetry/auto-instrumentations-node": { @@ -130,6 +131,9 @@ }, "@opentelemetry/semantic-conventions": { "optional": true + }, + "@sentry/node": { + "optional": true } }, "engines": { diff --git a/server/src/__tests__/sentry.test.ts b/server/src/__tests__/sentry.test.ts index 2e7ab88390..627804e4de 100644 --- a/server/src/__tests__/sentry.test.ts +++ b/server/src/__tests__/sentry.test.ts @@ -32,6 +32,12 @@ async function importFreshSentry() { * Register a fake `@sentry/node` module for the next dynamic import. Each * mock function is returned so a test can assert on the call it received. * The mock stays in place until `vi.doUnmock` runs, so `afterEach` clears it. + * + * `@sentry/node` is not installed on disk in this test environment, so the + * exact-version gate would report it missing before the dynamic import ever + * runs. This helper also mocks the gate module itself to report success, so + * a test can exercise the "package present and at the right version" path + * without installing the real package. */ function mockSentryPackage() { const init = vi.fn(); @@ -50,6 +56,9 @@ function mockSentryPackage() { httpIntegration, onUnhandledRejectionIntegration, })); + vi.doMock("../peer-version-check.js", () => ({ + checkExactPeerVersions: () => ({ ok: true }), + })); return { init, captureException, close, httpIntegration, onUnhandledRejectionIntegration }; } @@ -88,6 +97,7 @@ afterEach(() => { else process.env[DSN_ENV] = originalDsn; vi.restoreAllMocks(); vi.doUnmock("@sentry/node"); + vi.doUnmock("../peer-version-check.js"); }); describe("sentryReady", () => { @@ -262,6 +272,37 @@ describe("missing @sentry/node package", () => { }); }); +describe("@sentry/node installed at an unsupported version", () => { + it("logs one diagnostic and resolves without importing the package", async () => { + process.env[DSN_ENV] = "https://public@o0.ingest.sentry.io/1"; + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + vi.doMock("../peer-version-check.js", () => ({ + checkExactPeerVersions: () => ({ + ok: false, + diagnostic: "unused by the Sentry gate; see server/src/sentry.ts", + detail: { + missing: [], + mismatched: [{ name: "@sentry/node", installed: "9.0.0", expected: "10.71.0" }], + }, + }), + })); + + const { sentryReady } = await importFreshSentry(); + + // Bootstrap must absorb the reported mismatch — the server keeps + // booting without error monitoring rather than crashing on an opt-in + // feature. + await expect(sentryReady).resolves.toBeUndefined(); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining("@sentry/node package is not installed"), + expect.anything(), + ); + + vi.doUnmock("../peer-version-check.js"); + }); +}); + describe("buildSentryInitOptions", () => { it("sets sendDefaultPii false, tracesSampleRate 0, and skipOpenTelemetrySetup true", async () => { const { buildSentryInitOptions } = await importFreshSentry(); diff --git a/server/src/__tests__/server-package-sentry-peer-metadata.test.ts b/server/src/__tests__/server-package-sentry-peer-metadata.test.ts new file mode 100644 index 0000000000..78ab19b281 --- /dev/null +++ b/server/src/__tests__/server-package-sentry-peer-metadata.test.ts @@ -0,0 +1,32 @@ +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +/** + * `server/src/sentry.ts` loads `@sentry/node` through a dynamic import. It + * must be an optional peer dependency, not a normal dependency, so a default + * install never pulls in the SDK. This test guards the manifest half of that + * contract; `sentry.test.ts` guards the runtime half (the bootstrap fails + * open when the package is absent or at an unsupported version). + */ + +const packageJsonPath = fileURLToPath(new URL("../../package.json", import.meta.url)); + +describe("server package Sentry peer metadata", () => { + const packageJson = JSON.parse(readFileSync(packageJsonPath, "utf8")) as { + dependencies?: Record; + devDependencies?: Record; + peerDependencies?: Record; + peerDependenciesMeta?: Record; + }; + + it("declares @sentry/node as an optional peer dependency", () => { + expect(packageJson.peerDependencies?.["@sentry/node"]).toBeDefined(); + expect(packageJson.peerDependenciesMeta?.["@sentry/node"]?.optional).toBe(true); + }); + + it("does not list @sentry/node in dependencies or devDependencies", () => { + expect(packageJson.dependencies?.["@sentry/node"]).toBeUndefined(); + expect(packageJson.devDependencies?.["@sentry/node"]).toBeUndefined(); + }); +}); diff --git a/server/src/instrumentation.ts b/server/src/instrumentation.ts index 4b2d744458..34275e1dbd 100644 --- a/server/src/instrumentation.ts +++ b/server/src/instrumentation.ts @@ -33,9 +33,11 @@ // handler before `process.exit`. import { execFileSync } from "node:child_process"; -import { existsSync, readFileSync } from "node:fs"; +import { readFileSync } from "node:fs"; import { createRequire } from "node:module"; -import { dirname, join } from "node:path"; +import { checkExactPeerVersions } from "./peer-version-check.js"; + +export { checkExactPeerVersions } from "./peer-version-check.js"; const endpoint = process.env.OTEL_EXPORTER_OTLP_ENDPOINT; @@ -361,112 +363,6 @@ export function resolveProtocol(): { } } -/** - * Read this package's own `peerDependencies`, so the exact-version gate - * compares an installed package against the same version this manifest - * declares — one source of truth, not a second hardcoded copy. Returns an - * empty map on any read or parse failure (fail open: an unreadable manifest - * skips the version check rather than blocking startup). - */ -function readOwnPeerDependencies(): Record { - try { - const pkgUrl = new URL("../package.json", import.meta.url); - const raw = readFileSync(pkgUrl, "utf8"); - const parsed = JSON.parse(raw) as { peerDependencies?: Record }; - return parsed.peerDependencies ?? {}; - } catch { - return {}; - } -} - -/** - * Read an installed package's own declared `version`, without importing or - * executing the package. Resolves the package's main entry point (which - * respects its `exports` map) and then walks up the filesystem to the - * nearest `package.json` whose `name` matches — a direct - * `require.resolve(\`${packageName}/package.json\`)` throws for a package - * whose `exports` map does not expose `./package.json` as a subpath, which - * several `@opentelemetry/*` packages do not, even though the package is - * correctly installed. Returns null when the package cannot be resolved or no - * matching `package.json` is found. - */ -function readInstalledPackageVersion(packageName: string): string | null { - try { - const require = createRequire(import.meta.url); - let dir = dirname(require.resolve(packageName)); - for (;;) { - const candidate = join(dir, "package.json"); - if (existsSync(candidate)) { - const parsed = JSON.parse(readFileSync(candidate, "utf8")) as { - name?: unknown; - version?: unknown; - }; - if (parsed.name === packageName) { - return typeof parsed.version === "string" ? parsed.version : null; - } - } - const parent = dirname(dir); - if (parent === dir) return null; - dir = parent; - } - } catch { - return null; - } -} - -/** - * Verify that every package in `packageNames` is installed at the exact - * version `peerDependencies` declares. Checks only the packages the caller - * passes in — the bootstrap passes the four common packages plus the one - * exporter `OTEL_EXPORTER_OTLP_PROTOCOL` selected, never the two unselected - * exporters. Never throws: a missing manifest, a missing package, or an - * unreadable `package.json` all resolve to a reported issue, not an - * exception. - * - * `peerDependencies` defaults to this manifest's own declared versions - * (`readOwnPeerDependencies()`), which is what the bootstrap uses. A test - * passes an explicit map instead, so it can check the comparison logic - * against a package it controls without writing into `node_modules`. - */ -export function checkExactPeerVersions( - packageNames: readonly string[], - peerDependencies: Record = readOwnPeerDependencies(), -): { ok: true } | { ok: false; diagnostic: string; detail: unknown } { - const missing: string[] = []; - const mismatched: { name: string; installed: string; expected: string }[] = []; - - for (const name of packageNames) { - const expected = peerDependencies[name]; - const installed = readInstalledPackageVersion(name); - if (installed === null) { - missing.push(name); - } else if (expected && installed !== expected) { - mismatched.push({ name, installed, expected }); - } - } - - if (missing.length === 0 && mismatched.length === 0) return { ok: true }; - - const parts: string[] = []; - if (missing.length > 0) { - parts.push(`the @opentelemetry/* packages are not installed: ${missing.join(", ")}`); - } - if (mismatched.length > 0) { - const detail = mismatched - .map((m) => `${m.name}@${m.installed} (expected ${m.expected})`) - .join(", "); - parts.push(`a package is installed at an unsupported version: ${detail}`); - } - - return { - ok: false, - diagnostic: - `[paperclip] OTEL_EXPORTER_OTLP_ENDPOINT is set but ${parts.join("; and ")}. ` + - "Continuing without tracing.", - detail: { missing, mismatched }, - }; -} - async function importExporter(protocol: ExporterProtocol): Promise<{ OTLPTraceExporter: new (config?: Record) => unknown; }> { diff --git a/server/src/peer-version-check.ts b/server/src/peer-version-check.ts new file mode 100644 index 0000000000..ef6609de1e --- /dev/null +++ b/server/src/peer-version-check.ts @@ -0,0 +1,122 @@ +// Exact-version peer-dependency gate, shared by every optional-SDK bootstrap +// (OpenTelemetry in `instrumentation.ts`, Sentry in `sentry.ts`). +// +// This module has no module-init side effect — it only defines functions. A +// bootstrap module imports it and calls `checkExactPeerVersions` itself. That +// matters for `sentry.ts`: a direct import of `instrumentation.ts` would run +// the OpenTelemetry bootstrap (`instrumentationReady`) as a side effect of +// loading the Sentry gate, which this module avoids. + +import { existsSync, readFileSync } from "node:fs"; +import { createRequire } from "node:module"; +import { dirname, join } from "node:path"; + +/** + * Read this package's own `peerDependencies`, so the exact-version gate + * compares an installed package against the same version this manifest + * declares — one source of truth, not a second hardcoded copy. Returns an + * empty map on any read or parse failure (fail open: an unreadable manifest + * skips the version check rather than blocking startup). + */ +function readOwnPeerDependencies(): Record { + try { + const pkgUrl = new URL("../package.json", import.meta.url); + const raw = readFileSync(pkgUrl, "utf8"); + const parsed = JSON.parse(raw) as { peerDependencies?: Record }; + return parsed.peerDependencies ?? {}; + } catch { + return {}; + } +} + +/** + * Read an installed package's own declared `version`, without importing or + * executing the package. Resolves the package's main entry point (which + * respects its `exports` map) and then walks up the filesystem to the + * nearest `package.json` whose `name` matches — a direct + * `require.resolve(\`${packageName}/package.json\`)` throws for a package + * whose `exports` map does not expose `./package.json` as a subpath, which + * several `@opentelemetry/*` packages do not, even though the package is + * correctly installed. Returns null when the package cannot be resolved or no + * matching `package.json` is found. + */ +function readInstalledPackageVersion(packageName: string): string | null { + try { + const require = createRequire(import.meta.url); + let dir = dirname(require.resolve(packageName)); + for (;;) { + const candidate = join(dir, "package.json"); + if (existsSync(candidate)) { + const parsed = JSON.parse(readFileSync(candidate, "utf8")) as { + name?: unknown; + version?: unknown; + }; + if (parsed.name === packageName) { + return typeof parsed.version === "string" ? parsed.version : null; + } + } + const parent = dirname(dir); + if (parent === dir) return null; + dir = parent; + } + } catch { + return null; + } +} + +/** + * Verify that every package in `packageNames` is installed at the exact + * version `peerDependencies` declares. The caller passes only the packages it + * needs checked — the OpenTelemetry bootstrap passes the four common packages + * plus the one exporter `OTEL_EXPORTER_OTLP_PROTOCOL` selected, and the + * Sentry gate passes `["@sentry/node"]`. Never throws: a missing manifest, a + * missing package, or an unreadable `package.json` all resolve to a reported + * issue, not an exception. + * + * `peerDependencies` defaults to this manifest's own declared versions + * (`readOwnPeerDependencies()`), which is what each bootstrap uses. A test + * passes an explicit map instead, so it can check the comparison logic + * against a package it controls without writing into `node_modules`. + * + * The returned `diagnostic` string names the OpenTelemetry endpoint variable, + * because the OpenTelemetry bootstrap logs it directly. The Sentry gate reads + * `detail` instead and builds its own diagnostic line — see `sentry.ts`. + */ +export function checkExactPeerVersions( + packageNames: readonly string[], + peerDependencies: Record = readOwnPeerDependencies(), +): { ok: true } | { ok: false; diagnostic: string; detail: unknown } { + const missing: string[] = []; + const mismatched: { name: string; installed: string; expected: string }[] = []; + + for (const name of packageNames) { + const expected = peerDependencies[name]; + const installed = readInstalledPackageVersion(name); + if (installed === null) { + missing.push(name); + } else if (expected && installed !== expected) { + mismatched.push({ name, installed, expected }); + } + } + + if (missing.length === 0 && mismatched.length === 0) return { ok: true }; + + const parts: string[] = []; + if (missing.length > 0) { + parts.push(`the @opentelemetry/* packages are not installed: ${missing.join(", ")}`); + } + if (mismatched.length > 0) { + const detail = mismatched + .map((m) => `${m.name}@${m.installed} (expected ${m.expected})`) + .join(", "); + parts.push(`a package is installed at an unsupported version: ${detail}`); + } + + return { + ok: false, + diagnostic: + `[paperclip] OTEL_EXPORTER_OTLP_ENDPOINT is set but ${parts.join("; and ")}. ` + + "Continuing without tracing.", + detail: { missing, mismatched }, + }; +} diff --git a/server/src/sentry.ts b/server/src/sentry.ts index 76f75d2a1d..6dee0b5cbc 100644 --- a/server/src/sentry.ts +++ b/server/src/sentry.ts @@ -33,6 +33,16 @@ // The server relies on that crash today, so the initializer passes // `mode: "strict"`: Sentry still captures the event, then exits the process, // so the existing crash-and-restart behavior stays. +// +// Before it imports the package, the bootstrap checks the installed +// `@sentry/node` version against the exact version this manifest's +// `peerDependencies` declares — the same audited version documented in +// `doc/observability.md`. A missing or a mismatched version logs one +// diagnostic and leaves the server running without error monitoring; it +// never throws. This gate mirrors the OpenTelemetry gate in +// `instrumentation.ts`. + +import { checkExactPeerVersions } from "./peer-version-check.js"; const dsn = process.env.SENTRY_DSN; @@ -146,6 +156,24 @@ export function buildSentryInitOptions( } async function bootstrapSentry(dsn: string): Promise { + // Gate on the exact peer version before touching the dynamic import: a + // package installed at the wrong version can still load and start, which + // would silently invalidate the privacy audit `doc/observability.md` + // records against one exact version. Checking first turns that into one + // precise, fail-open diagnostic. + const versionCheck = checkExactPeerVersions(["@sentry/node"]); + if (!versionCheck.ok) { + // eslint-disable-next-line no-console + console.warn( + "[paperclip] SENTRY_DSN is set but the @sentry/node package is not " + + "installed, or is installed at an unsupported version. Install the " + + "declared version of @sentry/node to enable server error " + + "monitoring. Continuing without it.", + versionCheck.detail, + ); + return; + } + try { // Dynamic import so type-resolution doesn't require the package to be // installed unless the operator actually opts in. @@ -159,14 +187,14 @@ async function bootstrapSentry(dsn: string): Promise { close: (timeout) => Sentry.close(timeout), }; } catch (err) { - // The package is not installed, or the dynamic import or init call - // failed. Fall through with a single diagnostic so the opt-in path is - // self-documenting. The gate fails open — the server keeps booting - // without error monitoring rather than crashing on an opt-in feature. + // The exact-version gate above already confirmed @sentry/node is + // installed at the declared version, so only a load or init failure + // after that point reaches this block. // eslint-disable-next-line no-console console.warn( - "[paperclip] SENTRY_DSN is set but the @sentry/node package is not " + - "installed. Install @sentry/node to enable server error monitoring.", + "[paperclip] SENTRY_DSN is set and @sentry/node passed the version " + + "check, but it failed to load or initialize. Continuing without " + + "error monitoring.", err, ); } diff --git a/ui/package.json b/ui/package.json index 49e04bf283..520d91b203 100644 --- a/ui/package.json +++ b/ui/package.json @@ -73,7 +73,7 @@ "yjs": "13.6.29" }, "devDependencies": { - "@sentry/browser": "^10.71.0", + "@sentry/browser": "10.71.0", "@storybook/addon-a11y": "10.5.10", "@storybook/addon-docs": "10.5.10", "@storybook/react-vite": "10.5.10", diff --git a/ui/src/lib/sentry-package-version.test.ts b/ui/src/lib/sentry-package-version.test.ts new file mode 100644 index 0000000000..2640d25cd3 --- /dev/null +++ b/ui/src/lib/sentry-package-version.test.ts @@ -0,0 +1,32 @@ +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +/** + * `doc/observability.md` audits the browser Sentry privacy behavior against + * one exact `@sentry/browser` release. A caret range lets a lockfile refresh + * move the installed version and silently invalidate that audit. This test + * guards the manifest: the declared version must be exact, not a range. + */ + +const packageJsonPath = fileURLToPath(new URL("../../package.json", import.meta.url)); + +describe("ui package Sentry version pin", () => { + const packageJson = JSON.parse(readFileSync(packageJsonPath, "utf8")) as { + dependencies?: Record; + devDependencies?: Record; + }; + + it("pins @sentry/browser to an exact version", () => { + const declared = packageJson.devDependencies?.["@sentry/browser"]; + expect(declared).toBeDefined(); + // An exact version has no range operator (^, ~, >=, ||, x, *, …). A + // caret range such as "^10.71.0" must fail this assertion. + expect(declared).toMatch(/^\d+\.\d+\.\d+$/); + }); + + it("keeps @sentry/browser a development dependency, not a normal dependency", () => { + expect(packageJson.dependencies?.["@sentry/browser"]).toBeUndefined(); + expect(packageJson.devDependencies?.["@sentry/browser"]).toBeDefined(); + }); +});