diff --git a/Dockerfile b/Dockerfile index a3d046c96e..eda927f77a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -128,16 +128,17 @@ COPY --from=deps /app /app COPY . . RUN find packages/paperclip-runner/runner packages/paperclip-runner/protocol -type f -exec touch -d @0 {} + \ && touch -d @0 packages/paperclip-runner/rust-toolchain.toml +# Both the browser bundle and server stamp need the source commit. Declare it +# after the stable dependency layers, before either application build. +ARG PAPERCLIP_BUILD_COMMIT="" RUN pnpm --filter @paperclipai/ui build RUN pnpm --filter @paperclipai/plugin-sdk build # The server build runs scripts/write-build-stamp.mjs, which stamps the built # commit into dist/build-info.json. The build context has no .git, so the # script reads PAPERCLIP_BUILD_COMMIT instead. Docker exposes an ARG to the -# next RUN as an environment variable, so declare it here — in the build -# stage — before the server build. The production stage below declares the +# next RUN as an environment variable. The production stage below declares the # same ARG again for the runtime fallback; an ARG goes out of scope at the # end of its stage. Empty for local `docker build`, which then writes no stamp. -ARG PAPERCLIP_BUILD_COMMIT="" ENV NODE_OPTIONS=--max-old-space-size=4096 RUN pnpm --filter @paperclipai/server build RUN test -f server/dist/index.js || (echo "ERROR: server build output missing" && exit 1) diff --git a/doc/observability.md b/doc/observability.md index f604150e95..501b095e44 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -375,7 +375,23 @@ sends, so an operator can read what the feature does before turning it on. Each Sentry integration name below is verified against the default integration list of `@sentry/node@10.71.0` and `@sentry/browser@10.71.0`. -**Server attribute this feature sets** +**Release attribution** + +The server sets `release` to the full source commit from its build metadata. +An explicit `SENTRY_RELEASE` overrides that default. If neither is available, +the server leaves the release unset. + +The browser also sets `release`, using the full `PAPERCLIP_BUILD_COMMIT` +supplied when its bundle is built, or the checkout commit for source and npm +builds. The server reads its packaged build stamp when no deployment marker +is present. Docker passes the same commit to both +application builds. A cached browser bundle keeps its own release after a +server deployment, so its errors are attributed to the code actually loaded. +Browser builds without a valid full commit leave the release unset. The +browser does not read a release from the current server, page URL, or session. +These fields contain build identifiers; they add no tenant or user identity. + +**Server identity** - `server_name` — every server event carries the host name of the process. The `@sentry/node` client already sets this value by default when the diff --git a/server/scripts/write-build-stamp.mjs b/server/scripts/write-build-stamp.mjs index bceb09455f..684246c65f 100644 --- a/server/scripts/write-build-stamp.mjs +++ b/server/scripts/write-build-stamp.mjs @@ -5,7 +5,7 @@ // report `service.version`, so the value tracks the true built commit. // // The build resolves the commit in two steps: -// 1. `git rev-parse --short HEAD` in the server directory. +// 1. `git rev-parse HEAD` in the server directory. // 2. The `PAPERCLIP_BUILD_COMMIT` environment variable. // A Docker image build excludes `.git`, so the git lookup fails there. The // image build passes the commit in `PAPERCLIP_BUILD_COMMIT` instead, so the @@ -44,14 +44,14 @@ export function resolveBuildCommit(gitCommit, suppliedCommit) { } /** - * Read the short commit SHA with `git rev-parse --short HEAD` in the server + * Read the full commit SHA with `git rev-parse HEAD` in the server * directory. Return the SHA, or null on any failure. * * @returns {string | null} */ function readGitCommit() { try { - const out = execFileSync("git", ["rev-parse", "--short", "HEAD"], { + const out = execFileSync("git", ["rev-parse", "HEAD"], { cwd: serverDir, stdio: ["ignore", "pipe", "ignore"], }) diff --git a/server/src/__tests__/build-commit.test.ts b/server/src/__tests__/build-commit.test.ts index f76dc8505c..c000bcb642 100644 --- a/server/src/__tests__/build-commit.test.ts +++ b/server/src/__tests__/build-commit.test.ts @@ -15,6 +15,28 @@ describe("parseBuildCommit", () => { }); describe("readBuildCommit", () => { + it("reads the built server stamp in npm packages without a deployment marker", () => { + const commit = "0123456789abcdef0123456789abcdef01234567"; + expect(readBuildCommit({ + environmentCommit: null, + buildCommitPath: "/app/.paperclip-build-commit", + buildInfoPath: "/app/server/dist/build-info.json", + readTextFile: (path) => { + if (path.endsWith(".paperclip-build-commit")) throw new Error("ENOENT"); + return JSON.stringify({ commit }); + }, + })).toBe(commit); + }); + + it.each(["invalid json", "null", '{"commit":"short"}', '{"commit":42}'])( + "fails open on an invalid server stamp: %s", (stamp) => { + expect(readBuildCommit({ + environmentCommit: null, + readTextFile: () => stamp, + })).toBeNull(); + }, + ); + it("prefers an explicit environment commit", () => { const readTextFile = vi.fn(() => "ffffffffffffffffffffffffffffffffffffffff"); diff --git a/server/src/__tests__/sentry.test.ts b/server/src/__tests__/sentry.test.ts index 9067b96f36..5f34a1ff97 100644 --- a/server/src/__tests__/sentry.test.ts +++ b/server/src/__tests__/sentry.test.ts @@ -268,6 +268,13 @@ describe("missing @sentry/node package", () => { it("logs one warning and resolves", async () => { process.env[BACKEND_DSN_ENV] = "https://public@o0.ingest.sentry.io/1"; const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + // Keep this failure-mode test valid when the optional real-SDK tests run. + vi.doMock("../peer-version-check.js", () => ({ + checkExactPeerVersions: () => ({ + ok: false, + detail: { missing: ["@sentry/node"], mismatched: [] }, + }), + })); const { sentryReady } = await importFreshSentry(); @@ -537,6 +544,41 @@ describe("buildSentryInitOptions serverName", () => { }); }); +describe("buildSentryInitOptions release", () => { + const commit = "0123456789abcdef0123456789abcdef01234567"; + const integrations = { + httpIntegration: () => ({ name: "Http" }), + onUnhandledRejectionIntegration: () => ({ name: "OnUnhandledRejection" }), + }; + + beforeEach(() => { + vi.stubEnv("SENTRY_RELEASE", ""); + vi.doMock("../build-commit.js", () => ({ readBuildCommit: () => commit })); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + vi.doUnmock("../build-commit.js"); + }); + + it("uses the server build commit", async () => { + const { buildSentryInitOptions } = await importFreshSentry(); + expect(buildSentryInitOptions("test-dsn", integrations).release).toBe(commit); + }); + + it("preserves an operator's explicit release", async () => { + vi.stubEnv("SENTRY_RELEASE", " custom-release "); + const { buildSentryInitOptions } = await importFreshSentry(); + expect(buildSentryInitOptions("test-dsn", integrations).release).toBe("custom-release"); + }); + + it("leaves an unknown build unattributed", async () => { + vi.doMock("../build-commit.js", () => ({ readBuildCommit: () => null })); + const { buildSentryInitOptions } = await importFreshSentry(); + expect(buildSentryInitOptions("test-dsn", integrations).release).toBeUndefined(); + }); +}); + describe("with @sentry/node mocked", () => { it("initializes the client and shares captureException / shutdownSentry with it", async () => { process.env[DSN_ENV] = "https://public@o0.ingest.sentry.io/1"; @@ -644,6 +686,22 @@ describe.skipIf(!sentryPackage)("captured event shape against the real @sentry/n Sentry.init(options); } + it("attaches the actual build commit to an emitted event", async () => { + const commit = "0123456789abcdef0123456789abcdef01234567"; + vi.stubEnv("PAPERCLIP_BUILD_COMMIT", commit); + vi.stubEnv("SENTRY_RELEASE", ""); + try { + let captured: Record | null = null; + await initRealSentryForTest((event) => { captured = event; }); + sentryPackage!.captureException(new Error("build attribution check")); + await sentryPackage!.flush(2000); + expect(captured).toMatchObject({ release: commit }); + expect(captured).not.toHaveProperty("request"); + } finally { + vi.unstubAllEnvs(); + } + }); + it("a server event captured after a console.error call carries no console breadcrumb", async () => { const Sentry = sentryPackage!; let captured: Record | null = null; diff --git a/server/src/__tests__/write-build-stamp.test.ts b/server/src/__tests__/write-build-stamp.test.ts index eb7d55e63a..108d8254dc 100644 --- a/server/src/__tests__/write-build-stamp.test.ts +++ b/server/src/__tests__/write-build-stamp.test.ts @@ -1,7 +1,32 @@ import { describe, expect, it } from "vitest"; +import { execFileSync } from "node:child_process"; +import { copyFileSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { resolveBuildCommit } from "../../scripts/write-build-stamp.mjs"; +it("packages a full source commit without a Docker build argument", () => { + const root = realpathSync(mkdtempSync(join(tmpdir(), "paperclip-build-stamp-"))); + try { + const scriptDir = join(root, "server", "scripts"); + mkdirSync(scriptDir, { recursive: true }); + const script = join(scriptDir, "write-build-stamp.mjs"); + copyFileSync(new URL("../../scripts/write-build-stamp.mjs", import.meta.url), script); + const git = (...args: string[]) => execFileSync("git", args, { cwd: root, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }).trim(); + git("init", "--quiet"); + git("-c", "user.name=Test", "-c", "user.email=test@example.invalid", "commit", "--allow-empty", "--no-gpg-sign", "-m", "fixture"); + const env = { ...process.env }; + delete env.PAPERCLIP_BUILD_COMMIT; + execFileSync(process.execPath, [script], { cwd: root, env, stdio: "pipe" }); + const stamp = JSON.parse(readFileSync(join(root, "server", "dist", "build-info.json"), "utf8")); + expect(stamp.commit).toBe(git("rev-parse", "HEAD")); + expect(stamp.commit).toMatch(/^[0-9a-f]{40}$/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + describe("resolveBuildCommit", () => { it("prefers the git commit over the supplied environment commit", () => { expect(resolveBuildCommit("aaaaaaa", "bbbbbbb")).toBe("aaaaaaa"); diff --git a/server/src/build-commit.ts b/server/src/build-commit.ts index 105c6d590c..7c27a2befd 100644 --- a/server/src/build-commit.ts +++ b/server/src/build-commit.ts @@ -7,6 +7,7 @@ const FULL_SHA_RE = /^[0-9a-f]{40}$/i; const DEFAULT_BUILD_COMMIT_PATH = fileURLToPath( new URL("../../.paperclip-build-commit", import.meta.url), ); +const DEFAULT_BUILD_INFO_PATH = fileURLToPath(new URL("./build-info.json", import.meta.url)); export function parseBuildCommit(value: string | null | undefined): string | null { const commit = value?.trim() ?? ""; @@ -17,6 +18,7 @@ export function readBuildCommit( opts: { environmentCommit?: string | null; buildCommitPath?: string; + buildInfoPath?: string; readTextFile?: ReadTextFile; } = {}, ): string | null { @@ -27,9 +29,14 @@ export function readBuildCommit( ); if (environmentCommit) return environmentCommit; + const readTextFile = opts.readTextFile ?? ((path: string) => readFileSync(path, "utf8")); try { - const readTextFile = opts.readTextFile ?? ((path: string) => readFileSync(path, "utf8")); - return parseBuildCommit(readTextFile(opts.buildCommitPath ?? DEFAULT_BUILD_COMMIT_PATH)); + const markerCommit = parseBuildCommit(readTextFile(opts.buildCommitPath ?? DEFAULT_BUILD_COMMIT_PATH)); + if (markerCommit) return markerCommit; + } catch { /* The deployment marker is absent in npm packages. */ } + try { + const stamp = JSON.parse(readTextFile(opts.buildInfoPath ?? DEFAULT_BUILD_INFO_PATH)); + return typeof stamp?.commit === "string" ? parseBuildCommit(stamp.commit) : null; } catch { return null; } diff --git a/server/src/sentry.ts b/server/src/sentry.ts index cfe7ecb929..2b8c59423e 100644 --- a/server/src/sentry.ts +++ b/server/src/sentry.ts @@ -56,6 +56,7 @@ // `instrumentation.ts`. import os from "node:os"; +import { readBuildCommit } from "./build-commit.js"; import { checkExactPeerVersions } from "./peer-version-check.js"; import { resolveSentryDsns } from "./sentry-dsn.js"; @@ -200,12 +201,13 @@ export function shutdownSentry(): Promise { */ interface SentryModuleLike { httpIntegration(options: { breadcrumbs: boolean }): { name: string }; - onUnhandledRejectionIntegration(options: { mode: string }): { name: string }; + onUnhandledRejectionIntegration(options: { mode: "strict" }): { name: string }; } /** The `Sentry.init` options this gate builds. */ export interface SentryInitOptions { dsn: string; + release?: string; skipOpenTelemetrySetup: boolean; tracesSampleRate: number; sendDefaultPii: boolean; @@ -225,6 +227,7 @@ export function buildSentryInitOptions( ): SentryInitOptions { return { dsn, + release: process.env.SENTRY_RELEASE?.trim() || readBuildCommit() || undefined, skipOpenTelemetrySetup: true, tracesSampleRate: 0, sendDefaultPii: false, diff --git a/ui/src/lib/sentry.test.ts b/ui/src/lib/sentry.test.ts index b2838ebb53..e1612a197e 100644 --- a/ui/src/lib/sentry.test.ts +++ b/ui/src/lib/sentry.test.ts @@ -296,6 +296,29 @@ function resolveIntegrations( } describe("buildBrowserSentryInitOptions", () => { + it("keeps the loaded bundle's release when the server version changes", async () => { + const commit = "0123456789abcdef0123456789abcdef01234567"; + vi.stubGlobal("__PAPERCLIP_BUILD_COMMIT__", commit); + vi.stubEnv("PAPERCLIP_BUILD_COMMIT", "abcdef0123456789abcdef0123456789abcdef01"); + try { + const { buildBrowserSentryInitOptions } = await importFreshSentry(); + expect(buildBrowserSentryInitOptions(DSN).release).toBe(commit); + } finally { + vi.unstubAllGlobals(); + vi.unstubAllEnvs(); + } + }); + + it("does not invent a release for an unstamped bundle", async () => { + vi.stubGlobal("__PAPERCLIP_BUILD_COMMIT__", null); + try { + const { buildBrowserSentryInitOptions } = await importFreshSentry(); + expect(buildBrowserSentryInitOptions(DSN).release).toBeUndefined(); + } finally { + vi.unstubAllGlobals(); + } + }); + it("sets the recorded built-in privacy options", async () => { const { buildBrowserSentryInitOptions } = await importFreshSentry(); @@ -367,6 +390,21 @@ describe("captured event shape against the real @sentry/browser SDK", () => { return Sentry; } + it("attaches the bundle release to an emitted event without page context", async () => { + const commit = "0123456789abcdef0123456789abcdef01234567"; + vi.stubGlobal("__PAPERCLIP_BUILD_COMMIT__", commit); + try { + let captured: Record | null = null; + const Sentry = await initRealSentryForTest((event) => { captured = event; }); + Sentry.captureException(new Error("bundle attribution check")); + await Sentry.flush(2000); + expect(captured).toMatchObject({ release: commit }); + expect(captured).not.toHaveProperty("request"); + } finally { + vi.unstubAllGlobals(); + } + }); + it("an event from a page URL that holds a test capability value carries no request URL, no query string, and no referrer", async () => { window.history.pushState({}, "", "/dashboard?token=test-capability-value"); Object.defineProperty(document, "referrer", { diff --git a/ui/src/lib/sentry.ts b/ui/src/lib/sentry.ts index 404f64191e..bb433c5d2f 100644 --- a/ui/src/lib/sentry.ts +++ b/ui/src/lib/sentry.ts @@ -155,6 +155,11 @@ export function captureBrowserException(error: unknown): void { export function buildBrowserSentryInitOptions(dsn: string): BrowserSentryInitOptions { return { dsn, + // Use the loaded bundle's build, even when the server has since deployed. + release: + typeof __PAPERCLIP_BUILD_COMMIT__ === "string" + ? __PAPERCLIP_BUILD_COMMIT__ + : undefined, tracesSampleRate: 0, sendDefaultPii: false, integrations: (defaults) => diff --git a/ui/src/lib/vite-build-commit.test.ts b/ui/src/lib/vite-build-commit.test.ts new file mode 100644 index 0000000000..ea764732ab --- /dev/null +++ b/ui/src/lib/vite-build-commit.test.ts @@ -0,0 +1,44 @@ +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it, vi } from "vitest"; +import { resolveBrowserBuildCommit } from "./vite-build-commit"; + +describe("browser build attribution", () => { + const commit = "0123456789abcdef0123456789abcdef01234567"; + + it("uses the checkout commit for source and npm builds", () => { + expect(resolveBrowserBuildCommit(undefined, () => commit)).toBe(commit); + }); + + it("uses a supplied commit without requiring git in Docker", () => { + const readGitCommit = vi.fn(() => "ffffffffffffffffffffffffffffffffffffffff"); + expect(resolveBrowserBuildCommit(commit, readGitCommit)).toBe(commit); + expect(readGitCommit).not.toHaveBeenCalled(); + }); + + it("leaves builds without git or a supplied commit unattributed", () => { + expect(resolveBrowserBuildCommit(undefined, () => { throw new Error("no git"); })).toBeNull(); + expect(resolveBrowserBuildCommit(undefined, () => "not-a-commit")).toBeNull(); + }); + + it("accepts and normalizes the full source commit supplied by image CI", () => { + expect(resolveBrowserBuildCommit(" 0123456789ABCDEF0123456789ABCDEF01234567\n")) + .toBe("0123456789abcdef0123456789abcdef01234567"); + }); + + it.each([undefined, "", "main", "0123456", "https://example.invalid/private-build?token=canary"])( + "omits an unknown or non-commit value: %s", (value) => { + expect(resolveBrowserBuildCommit(value)).toBeNull(); + }, + ); + + it("makes the Docker build commit available before building the browser", () => { + const dockerfile = readFileSync(fileURLToPath(new URL("../../../Dockerfile", import.meta.url)), "utf8"); + const stage = dockerfile.split("FROM runner-build AS build")[1]?.split("FROM base AS production")[0]; + expect(stage).toBeDefined(); + const arg = stage!.indexOf('ARG PAPERCLIP_BUILD_COMMIT=""'); + const browserBuild = stage!.indexOf("RUN pnpm --filter @paperclipai/ui build"); + expect(arg).toBeGreaterThanOrEqual(0); + expect(browserBuild).toBeGreaterThan(arg); + }); +}); diff --git a/ui/src/lib/vite-build-commit.ts b/ui/src/lib/vite-build-commit.ts new file mode 100644 index 0000000000..b51fdde43c --- /dev/null +++ b/ui/src/lib/vite-build-commit.ts @@ -0,0 +1,31 @@ +import { execFileSync } from "node:child_process"; + +function parseCommit(value: string | undefined): string | null { + const commit = value?.trim() ?? ""; + return /^[0-9a-f]{40}$/i.test(commit) ? commit.toLowerCase() : null; +} + +/** Only a full source commit may enter the public browser bundle. */ +export function resolveBrowserBuildCommit( + value: string | undefined, + readGitCommit: () => string | undefined = () => undefined, +): string | null { + const suppliedCommit = parseCommit(value); + if (suppliedCommit) return suppliedCommit; + try { + return parseCommit(readGitCommit()); + } catch { + return null; + } +} + +export function readBrowserBuildCommit(repositoryDirectory: string): string | null { + return resolveBrowserBuildCommit(process.env.PAPERCLIP_BUILD_COMMIT, () => + execFileSync("git", ["rev-parse", "HEAD"], { + cwd: repositoryDirectory, + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + timeout: 1000, + }), + ); +} diff --git a/ui/src/vite-env.d.ts b/ui/src/vite-env.d.ts index 11f02fe2a0..88b91807f6 100644 --- a/ui/src/vite-env.d.ts +++ b/ui/src/vite-env.d.ts @@ -1 +1,3 @@ /// + +declare const __PAPERCLIP_BUILD_COMMIT__: string | null; diff --git a/ui/vite.config.ts b/ui/vite.config.ts index 3ac9f91485..2ecc84ec77 100644 --- a/ui/vite.config.ts +++ b/ui/vite.config.ts @@ -5,10 +5,16 @@ import tailwindcss from "@tailwindcss/vite"; import { createUiDevWatchOptions } from "./src/lib/vite-watch"; import { createApiProxy } from "./src/lib/vite-api-proxy"; import { serviceWorkerBuildIdPlugin } from "./src/lib/vite-sw-build-id"; +import { readBrowserBuildCommit } from "./src/lib/vite-build-commit"; const apiProxy = createApiProxy(); export default defineConfig(({ mode }) => ({ + define: { + __PAPERCLIP_BUILD_COMMIT__: JSON.stringify( + readBrowserBuildCommit(__dirname), + ), + }, plugins: [react(), tailwindcss(), serviceWorkerBuildIdPlugin()], build: { minify: "esbuild",