fix(server): record inspector provenance for dynamic import callback errors (#17844)

https://entire.com/gh/paperclipai/-/trails/49/changes/paperclip/49  ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server sends errors to Sentry when an operator sets a backend DSN. An unhandled rejection is captured in `strict` mode, so the server process exits after the capture. > - A hosted server reported `TypeError [ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING]: A dynamic import callback was not specified.` two times, eight days apart, on two unrelated releases. Each event stopped the process. > - The stack has no first-party frame. An anonymous `setTimeout` callback at `<anonymous>:1:N` calls `import()`. The column changed between the two events, so the source text changed. > - In local reproduction, only two paths make that frame: `vm.compileFunction` without a filename, and an expression that a client evaluates through the Node inspector. No shipped server code or production dependency uses either path. The current events give no data about the source. > - This pull request adds bounded provenance to that one error: the inspector state when the event is reported (never its URL), allow-listed Node flag names, the process ids, the main-thread flag and the uptime. > - The benefit is that the next event gives evidence about the source of the anonymous code. A `closed` state rules out an expression from a still-open inspector session. An `open` state needs other evidence before a debugger is blamed. ## Linked Issues or Issue Description No public issue exists. This section follows the bug report template. **What happened?** A hosted server process stopped two times with an unhandled rejection: `TypeError [ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING]: A dynamic import callback was not specified.` The stack is `importModuleDynamicallyCallback (node:internal/modules/esm/utils:280)` ← `Timeout._onTimeout (<anonymous>:1:20)` (first event) or `(<anonymous>:1:41)` (second event) ← `listOnTimeout`. The events came 3 minutes and 17 minutes after process start, on two releases that changed only skill documents and UI files. The Sentry event has no data that shows where the anonymous code came from. **Expected behavior** The error report gives enough data to investigate the source of the anonymous code: whether an inspector session was open in the process, and which inspector-related Node flags were set. **Steps to reproduce** 1. `node -e "require('node:vm').compileFunction(\"setTimeout(() => import('node:fs'), 1)\", [])()"` gives the same error and the same `Timeout._onTimeout (<anonymous>:1:N)` frame. 2. An expression such as `setTimeout(() => import('node:fs'), 1)`, evaluated through the inspector (`Runtime.evaluate`), gives the same error and frame. 3. `vm.Script` (`evalmachine.<anonymous>`), `eval` or `new Function` in a `vm` context (`eval at …`), `new Function`, indirect `eval`, `node -e`, `node -p` and stdin give a different frame or import successfully. **Paperclip version or commit** `master` at `0cb3fc7db591ba23e21fbbc70042bc44ec7d6f20`. The hosted events came from builds `f96a1e64` and `3e7c3ee9`, on Node v24.22.0 and v24.22.1. **Relevant subsystem** Server Sentry error monitoring (`server/src/sentry.ts`). ## What Changed - New `server/src/sentry-dynamic-import-provenance.ts`: `addDynamicImportProvenance(event, hint)` acts only when `hint.originalException.code` is `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`. It adds: - the tag `dynamic_import_inspector` with the value `open`, `closed` or `unavailable` - the context `dynamic_import_provenance` with `inspector`, `nodeFlags`, `mainThread`, `pid`, `ppid` and `uptimeSeconds` - `nodeFlags` holds only the names of `--inspect`, `--inspect-brk`, `--inspect-wait`, `--inspect-port`, `--debug-port`, `--disable-sigusr1` and `--experimental-vm-modules` from `execArgv` and `NODE_OPTIONS`. Flag values and all other flags are dropped. - The hook never sends the inspector URL, because the URL contains a session secret. It loads `node:inspector` lazily, so a Node build without the inspector still starts and reports `unavailable`. The hook never throws. Every other event is unchanged. - The module comment and the doc say that the state is evidence at report time, not proof of origin. - `server/src/sentry.ts`: `beforeSend` is now `enrichServerEvent`. It runs the existing run-link enrichment, then the new hook. - `doc/observability.md`: the default capture set names the new tag and context, and says what is never sent. - `.github/workflows/sentry-contract.yml`: the Real Sentry SDK isolation job also runs the new test file. - Tests in `server/src/__tests__/sentry-dynamic-import-provenance.test.ts`: - the real Node error from `vm.compileFunction` - a real open inspector session: the URL and its secret are absent from the event. The test closes only the session it opened, and it skips when a developer already has an inspector open. - flag filtering with values and other flags present in `NODE_OPTIONS` - other codes and other hint shapes stay unchanged - a failed state read keeps the original event - the real `@sentry/node@10.71.0` SDK sends the original rejection to the server `beforeSend`, and enriches only the matching event. With `PAPERCLIP_REQUIRE_SENTRY_TEST_SDK=1`, a missing SDK fails the file. ## Verification - With the audited SDK on `NODE_PATH` and `PAPERCLIP_REQUIRE_SENTRY_TEST_SDK=1`, as in the contract job: `run-failure-sentry-real-sdk.test.ts` and the new test file pass (17 tests). - Without the SDK: the new test file, `sentry.test.ts` and `sentry-links.test.ts` pass (52 pass, 6 skipped). - With `PAPERCLIP_REQUIRE_SENTRY_TEST_SDK=1` and no SDK: the new test file fails at once, as intended. - Red: with `beforeSend` set back to `addSentryRunLinks`, the real-SDK test fails (`expected [] to have a length of 1`). Green with the change. - Hosted CI on this head: typecheck, build, all test partitions and Real Sentry SDK isolation pass. - Not run locally: the full Vitest suite and browser suites. A live hosted check is not possible before deployment. ## Risks - Low risk. The hook acts only on one error code. All other events keep their current content and grouping. - The process still exits after the capture, as before. This change adds diagnostics. It does not fix the source of the anonymous code. - `pid`, `ppid` and uptime describe the server process only. They hold no tenant, user or request data. - If a later Node version changes the error code, the hook stops acting and the events are the same as today. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through the Claude Agent SDK, with tool use (shell, file edit, code execution, GitHub and Sentry connectors). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-11-08 22:13:59 +01:00 · 2026-11-04 14:07:26 -08:00
1 parent a3313f6944
commit c19c4348c5
@@ -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<string, unknown>): unknown;
+      captureException(error: unknown): string;
+      flush(timeout?: number): Promise<boolean>;
+    };
+  } 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<Error> {
+  const run = vm.compileFunction("return import('node:fs')", []) as () => Promise<unknown>;
+  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<ProvenanceEvent>({}, { 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<ProvenanceEvent>({}, { 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<Record<string, unknown>> = [];
+    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<string, unknown>) => {
+        const beforeSend = options.beforeSend as (event: unknown, hint: unknown) => Promise<Record<string, unknown>>;
+        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<string, unknown> | 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 } },
+    });
+  });
+});
@@ -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 `<anonymous>: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<string, string | number | boolean | bigint | symbol | null | undefined>;
+  contexts?: Record<string, Record<string, unknown> | null | undefined>;
+}
+
+export interface ProvenanceHint {
+  originalException?: unknown;
+}
+
+/** Reads the inspector state of this process. Never returns the URL. */
+export async function readInspectorState(): Promise<InspectorState> {
+  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<T extends ProvenanceEvent>(
+  event: T,
+  hint?: ProvenanceHint,
+  readState: () => Promise<InspectorState> = readInspectorState,
+): Promise<T> {
+  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;
+}
@@ -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<typeof addSentryRunLinks>[0] & Parameters<typeof addDynamicImportProvenance>[0];
+
+/** The server's `beforeSend`: run navigation links, then dynamic-import provenance. */
+export async function enrichServerEvent<T extends ServerEvent>(event: T, hint?: ProvenanceHint): Promise<T> {
+  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(