diff --git a/.github/workflows/sentry-contract.yml b/.github/workflows/sentry-contract.yml index 87515150a0e..6134ad91c0b 100644 --- a/.github/workflows/sentry-contract.yml +++ b/.github/workflows/sentry-contract.yml @@ -66,4 +66,4 @@ jobs: env: NODE_PATH: ${{ runner.temp }}/sentry-contract-sdk/node_modules PAPERCLIP_REQUIRE_SENTRY_TEST_SDK: "1" - run: pnpm --filter @paperclipai/server exec vitest run src/__tests__/run-failure-sentry-real-sdk.test.ts + run: pnpm --filter @paperclipai/server exec vitest run src/__tests__/run-failure-sentry-real-sdk.test.ts src/__tests__/sentry-dynamic-import-provenance.test.ts diff --git a/doc/observability.md b/doc/observability.md index d503db0f7dc..3b1df075f26 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -1638,6 +1638,20 @@ These fields contain build identifiers; they add no tenant or user identity. `errorCode`, and `agentAdapter`. The server redacts the error message and the error code before it sends the event. +A server event whose original error has the code +`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING` gets bounded provenance. The tag +`dynamic_import_inspector` holds `open`, `closed`, or `unavailable`. The +`dynamic_import_provenance` context holds the same state, the process id, the +parent process id, the main-thread flag, the uptime in seconds, and the names +of inspector-related Node flags from `execArgv` and `NODE_OPTIONS`. The event +never holds the inspector URL, because that URL contains a session secret. It +also never holds a flag value or any other flag. This error has reached +Sentry with no first-party frame. The state shows whether an inspector +session was open when the server reported the event. It does not prove which +code caused the error: code compiled without a filename fails the same way +while a debugger is attached. A `closed` state rules out an expression that a +still-open session evaluated. Other events do not change. + Native runner identity and harness failures retain their existing error prefixes. Their terminal messages now include a bounded guard reason, such as `session_scope_mismatch`, `durable_identity_unreadable`, or diff --git a/server/src/__tests__/sentry-dynamic-import-provenance.test.ts b/server/src/__tests__/sentry-dynamic-import-provenance.test.ts new file mode 100644 index 00000000000..40aa00795fa --- /dev/null +++ b/server/src/__tests__/sentry-dynamic-import-provenance.test.ts @@ -0,0 +1,147 @@ +import inspector from "node:inspector"; +import { createRequire } from "node:module"; +import vm from "node:vm"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { addDynamicImportProvenance, readInspectorState, type ProvenanceEvent } from "../sentry-dynamic-import-provenance.js"; + +// Sentry is an optional peer. When installed, prove the real SDK passes the +// original rejection to the server's `beforeSend`. +const sentryPackage = (() => { + try { + const require = createRequire(import.meta.url); + return require("@sentry/node") as { + init(options: Record): unknown; + captureException(error: unknown): string; + flush(timeout?: number): Promise; + }; + } catch { + return null; + } +})(); + +if (process.env.PAPERCLIP_REQUIRE_SENTRY_TEST_SDK === "1" && !sentryPackage) { + throw new Error("The Sentry SDK contract job requires the audited optional peer"); +} + +// A developer may debug this file with an inspector already open. Only the +// test that opens its own session may close it. +const inspectorOpenAtStart = inspector.url() !== undefined; + +/** The real Node error: code compiled without a filename calls `import()`. */ +async function dynamicImportCallbackMissing(): Promise { + const run = vm.compileFunction("return import('node:fs')", []) as () => Promise; + try { + await run(); + } catch (error) { + return error as Error; + } + throw new Error("expected the dynamic import to reject"); +} + +afterEach(() => { + vi.unstubAllEnvs(); + vi.doUnmock("@sentry/node"); + vi.doUnmock("../peer-version-check.js"); +}); + +describe("Sentry dynamic import provenance", () => { + it("adds bounded inspector and process provenance to the real Node error", async () => { + const error = await dynamicImportCallbackMissing(); + expect((error as NodeJS.ErrnoException).code).toBe("ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING"); + const input: ProvenanceEvent = { tags: { existing: "kept" }, contexts: { runtime: { name: "node" } } }; + const event = await addDynamicImportProvenance( + input, + { originalException: error }, + async () => "closed", + ); + expect(event.tags).toEqual({ existing: "kept", dynamic_import_inspector: "closed" }); + expect(event.contexts?.runtime).toEqual({ name: "node" }); + expect(event.contexts?.dynamic_import_provenance).toEqual({ + inspector: "closed", + nodeFlags: expect.any(Array), + mainThread: expect.any(Boolean), + pid: process.pid, + ppid: process.ppid, + uptimeSeconds: expect.any(Number), + }); + }); + + it.skipIf(inspectorOpenAtStart)("reports an open inspector session without sending its secret URL", async () => { + expect(await readInspectorState()).toBe("closed"); + inspector.open(0, "127.0.0.1"); + try { + const url = inspector.url(); + expect(url).toMatch(/^ws:\/\/127\.0\.0\.1:\d+\//); + const event = await addDynamicImportProvenance({}, { originalException: await dynamicImportCallbackMissing() }); + expect(event).toMatchObject({ tags: { dynamic_import_inspector: "open" }, contexts: { dynamic_import_provenance: { inspector: "open" } } }); + const serialized = JSON.stringify(event); + expect(serialized).not.toContain(url!); + expect(serialized).not.toContain(url!.split("/").at(-1)!); + } finally { + inspector.close(); + } + }); + + it("keeps only allow-listed Node flag names and drops their values", async () => { + vi.stubEnv("NODE_OPTIONS", "--max-old-space-size=4096 --inspect=0.0.0.0:9229 --require /private/preload.cjs --inspect"); + const event = await addDynamicImportProvenance({}, { originalException: await dynamicImportCallbackMissing() }, async () => "open"); + const flags = event.contexts?.dynamic_import_provenance?.nodeFlags; + expect(flags).toContain("--inspect"); + expect(flags).not.toContain("--max-old-space-size"); + expect(flags).not.toContain("--require"); + expect(JSON.stringify(event)).not.toMatch(/0\.0\.0\.0|9229|private|4096/); + }); + + it("leaves every other event unchanged", async () => { + const unrelated = { tags: { run_id: "kept" }, contexts: {} }; + const readState = vi.fn(async () => "open" as const); + const coded = Object.assign(new Error("other"), { code: "ERR_MODULE_NOT_FOUND" }); + for (const hint of [undefined, {}, { originalException: coded }, { originalException: "text" }, { originalException: null }]) { + expect(await addDynamicImportProvenance(structuredClone(unrelated), hint, readState)).toEqual(unrelated); + } + expect(readState).not.toHaveBeenCalled(); + }); + + it("returns the original event when the state read fails", async () => { + const event = { tags: { existing: "kept" } }; + const result = await addDynamicImportProvenance(event, { originalException: await dynamicImportCallbackMissing() }, async () => { + throw new Error("inspector read failed"); + }); + expect(result).toEqual({ tags: { existing: "kept" } }); + }); + + it.skipIf(!sentryPackage || inspectorOpenAtStart)("receives the original rejection through the real SDK beforeSend", async () => { + const Sentry = sentryPackage!; + const events: Array> = []; + vi.stubEnv("SENTRY_DSN_BACKEND", "https://public@example.invalid/1"); + vi.doMock("../peer-version-check.js", () => ({ checkExactPeerVersions: () => ({ ok: true }) })); + vi.doMock("@sentry/node", () => ({ + ...Sentry, + init: (options: Record) => { + const beforeSend = options.beforeSend as (event: unknown, hint: unknown) => Promise>; + return Sentry.init({ + ...options, + transport: () => ({ send: async () => ({}), flush: async () => true }), + beforeSend: async (event: unknown, hint: unknown) => { + const enriched = await beforeSend(event, hint); + events.push(enriched); + return enriched; + }, + }); + }, + })); + vi.resetModules(); + const { sentryReady, captureException } = await import("../sentry.js"); + await sentryReady; + captureException(await dynamicImportCallbackMissing()); + captureException(new Error("unrelated fixture")); + await Sentry.flush(2000); + expect(events).toHaveLength(2); + const enriched = events.filter((event) => (event.tags as Record | undefined)?.dynamic_import_inspector); + expect(enriched).toHaveLength(1); + expect(enriched[0]).toMatchObject({ + tags: { dynamic_import_inspector: "closed" }, + contexts: { dynamic_import_provenance: { inspector: "closed", pid: process.pid } }, + }); + }); +}); diff --git a/server/src/sentry-dynamic-import-provenance.ts b/server/src/sentry-dynamic-import-provenance.ts new file mode 100644 index 00000000000..50bacc7826c --- /dev/null +++ b/server/src/sentry-dynamic-import-provenance.ts @@ -0,0 +1,90 @@ +// Bounded provenance for `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING` events. +// +// This error has reached Sentry with no first-party frame: an anonymous +// `setTimeout` callback at `:1:N` calls `import()`, and the +// unhandled rejection stops the server. In local reproduction only two paths +// produce that exact frame: `vm.compileFunction` without a filename, and code +// that a client evaluates in the process through the Node inspector. No +// shipped server code takes either path. This hook records whether an +// inspector session was open in the reporting process when the event was +// reported. That state is evidence, not proof of origin: code compiled +// without a filename fails the same way while a debugger is attached. A +// `closed` state rules out an evaluated expression from a session that is +// still open; an `open` state needs other evidence before blaming a debugger. +// +// Privacy: the inspector URL embeds a session secret, so only a three-value +// state leaves the process. Flag values are dropped; only allow-listed Node +// flag names remain. Every other event passes through unchanged. + +import { isMainThread } from "node:worker_threads"; + +const DYNAMIC_IMPORT_CALLBACK_MISSING = "ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING"; + +// Node flags that open, configure, or disable the inspector, plus the flag +// that changes `vm` dynamic import behavior. Nothing else is reported. +const REPORTED_NODE_FLAGS = new Set([ + "--inspect", + "--inspect-brk", + "--inspect-wait", + "--inspect-port", + "--debug-port", + "--disable-sigusr1", + "--experimental-vm-modules", +]); + +export type InspectorState = "open" | "closed" | "unavailable"; + +export interface ProvenanceEvent { + tags?: Record; + contexts?: Record | null | undefined>; +} + +export interface ProvenanceHint { + originalException?: unknown; +} + +/** Reads the inspector state of this process. Never returns the URL. */ +export async function readInspectorState(): Promise { + try { + // A Node build without the inspector throws on import. Load it lazily so + // that build still starts and still reports the event. + const inspector = await import("node:inspector"); + return inspector.url() === undefined ? "closed" : "open"; + } catch { + return "unavailable"; + } +} + +function reportedNodeFlags(): string[] { + const tokens = [...process.execArgv, ...(process.env.NODE_OPTIONS ?? "").split(/\s+/)]; + const names = tokens.map((token) => token.split("=", 1)[0]).filter((name) => REPORTED_NODE_FLAGS.has(name)); + return [...new Set(names)].sort(); +} + +/** + * Add inspector and process provenance to a dynamic-import-callback event. + * Other events are returned unchanged. Never throws: provenance must not + * discard the original error. + */ +export async function addDynamicImportProvenance( + event: T, + hint?: ProvenanceHint, + readState: () => Promise = readInspectorState, +): Promise { + try { + const error = hint?.originalException; + if (typeof error !== "object" || error === null || + (error as { code?: unknown }).code !== DYNAMIC_IMPORT_CALLBACK_MISSING) return event; + const inspector = await readState(); + event.tags = { ...event.tags, dynamic_import_inspector: inspector }; + event.contexts = { ...event.contexts, dynamic_import_provenance: { + inspector, + nodeFlags: reportedNodeFlags(), + mainThread: isMainThread, + pid: process.pid, + ppid: process.ppid, + uptimeSeconds: Math.round(process.uptime()), + } }; + } catch { /* Provenance enrichment must not discard the original error. */ } + return event; +} diff --git a/server/src/sentry.ts b/server/src/sentry.ts index 8cc63533bbd..016f13e01be 100644 --- a/server/src/sentry.ts +++ b/server/src/sentry.ts @@ -47,6 +47,9 @@ // `mode: "strict"`: Sentry still captures the event, then exits the process, // so the existing crash-and-restart behavior stays. // +// `beforeSend` adds run navigation links, then bounded provenance for the +// dynamic-import-callback error — see `sentry-dynamic-import-provenance.ts`. +// // 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 @@ -66,6 +69,7 @@ import { checkExactPeerVersions } from "./peer-version-check.js"; import { resolveSentryDsns } from "./sentry-dsn.js"; import { readRunFailureReportingMode } from "./sentry-run-failure-policy.js"; import { addSentryRunLinks, type SentryRunReference } from "./sentry-links.js"; +import { addDynamicImportProvenance, type ProvenanceHint } from "./sentry-dynamic-import-provenance.js"; const { backend: dsn, legacyFallbackUsed } = resolveSentryDsns(); @@ -402,7 +406,14 @@ export interface SentryInitOptions { sendDefaultPii: boolean; serverName: string; integrations: (defaults: Array<{ name: string }>) => Array<{ name: string }>; - beforeSend: typeof addSentryRunLinks; + beforeSend: typeof enrichServerEvent; +} + +type ServerEvent = Parameters[0] & Parameters[0]; + +/** The server's `beforeSend`: run navigation links, then dynamic-import provenance. */ +export async function enrichServerEvent(event: T, hint?: ProvenanceHint): Promise { + return addDynamicImportProvenance(await addSentryRunLinks(event), hint); } /** @@ -421,7 +432,7 @@ export function buildSentryInitOptions( skipOpenTelemetrySetup: true, tracesSampleRate: 0, sendDefaultPii: false, - beforeSend: addSentryRunLinks, + beforeSend: enrichServerEvent, serverName: process.env.SENTRY_NAME || os.hostname(), integrations: (defaults: Array<{ name: string }>) => { const kept = defaults.filter(