From 85621e84e3138bc8d4e03be4b8eac0995ea114ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Beteg=C3=B3n?= Date: Mon, 28 Sep 2026 12:43:16 +0200 Subject: [PATCH 1/3] fix(telemetry): redact credentials from CLI diagnostics --- packages/cli/src/app.ts | 11 +- packages/cli/src/cli.ts | 8 +- packages/cli/src/lib/credential-redaction.ts | 125 ++++++++++++ packages/cli/src/lib/error-reporting.ts | 3 +- packages/cli/src/lib/errors.ts | 5 +- packages/cli/src/lib/sdk-types.ts | 8 +- packages/cli/src/lib/telemetry.ts | 11 +- packages/cli/test/e2e/auth.test.ts | 25 ++- .../cli/test/lib/credential-redaction.test.ts | 182 +++++++++++++++++ .../test/lib/telemetry-credentials.test.ts | 191 ++++++++++++++++++ 10 files changed, 552 insertions(+), 17 deletions(-) create mode 100644 packages/cli/src/lib/credential-redaction.ts create mode 100644 packages/cli/test/lib/credential-redaction.test.ts create mode 100644 packages/cli/test/lib/telemetry-credentials.test.ts diff --git a/packages/cli/src/app.ts b/packages/cli/src/app.ts index 487290140..02ac69b7c 100644 --- a/packages/cli/src/app.ts +++ b/packages/cli/src/app.ts @@ -68,14 +68,15 @@ import { getSynonymSuggestionFromArgv, } from "./lib/command-suggestions.js"; import { CLI_VERSION } from "./lib/constants.js"; +import { redactCredentialText } from "./lib/credential-redaction.js"; import { reportCliError } from "./lib/error-reporting.js"; import { ApiError, AuthError, CliError, + formatError, getExitCode, OutputError, - stringifyUnknown, WizardError, } from "./lib/errors.js"; import { error as errorColor, warning } from "./lib/formatters/colors.js"; @@ -383,7 +384,7 @@ const customText: ApplicationText = { // user mistakes, not real errors. const synonymResult = formatSynonymError(exc, ansiColor); if (synonymResult) { - return synonymResult; + return redactCredentialText(synonymResult); } // Report command errors to Sentry with stable fingerprinting. Stricli @@ -400,12 +401,12 @@ const customText: ApplicationText = { return ""; } const prefix = ansiColor ? errorColor("Error:") : "Error:"; - return `${prefix} ${exc.format()}`; + return `${prefix} ${formatError(exc)}`; } if (exc instanceof Error) { - return `Unexpected error: ${exc.stack ?? exc.message}`; + return `Unexpected error: ${redactCredentialText(exc.stack ?? exc.message)}`; } - return `Unexpected error: ${stringifyUnknown(exc)}`; + return `Unexpected error: ${formatError(exc)}`; }, }; diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 522628cc4..2b95fd8aa 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -11,7 +11,7 @@ */ import { getEnv } from "./lib/env.js"; -import { CliError } from "./lib/errors.js"; +import { CliError, formatError } from "./lib/errors.js"; import { initTimezone } from "./lib/timezone.js"; /** @@ -239,7 +239,7 @@ export async function runCli(cliArgs: string[]): Promise { const { ExitCode, run } = await import("@stricli/core"); const { app } = await import("./app.js"); const { buildContext } = await import("./context.js"); - const { AuthError, OutputError, formatError, getExitCode } = await import( + const { AuthError, OutputError, getExitCode } = await import( "./lib/errors.js" ); const { error } = await import("./lib/formatters/colors.js"); @@ -693,7 +693,7 @@ export async function startCli(): Promise { await preloadProjectContext(process.cwd()); } catch (err) { if (err instanceof CliError) { - process.stderr.write(`${err.format()}\n`); + process.stderr.write(`${formatError(err)}\n`); process.exitCode = err.exitCode; return; } @@ -701,7 +701,7 @@ export async function startCli(): Promise { } return runCli(args).catch((err) => { - process.stderr.write(`Fatal: ${err}\n`); + process.stderr.write(`Fatal: ${formatError(err)}\n`); process.exitCode = 1; }); } diff --git a/packages/cli/src/lib/credential-redaction.ts b/packages/cli/src/lib/credential-redaction.ts new file mode 100644 index 000000000..a35fcc68f --- /dev/null +++ b/packages/cli/src/lib/credential-redaction.ts @@ -0,0 +1,125 @@ +/** + * Stateless credential redaction for CLI diagnostics and telemetry. + * Kept separate from API data formatting so successful responses stay intact. + */ + +import { type Envelope, normalize } from "@sentry/core"; + +const INVALID_BEARER_HEADER_START = + /(\bHeaders\.(?:set|append):[ \t]*)(\\*["'])Bearer[ \t]+/gi; +const INVALID_HEADER_END = /(?(); + for (const match of text.matchAll(INVALID_HEADER_END)) { + const quote = match[1]; + if (quote) { + lastEnds.set(quote, match.index); + } + } + if (lastEnds.size === 0) { + return text; + } + + const parts: string[] = []; + let cursor = 0; + for (const match of text.matchAll(INVALID_BEARER_HEADER_START)) { + if (match.index < cursor) { + continue; + } + const [, prefix, quote] = match; + if (!(prefix && quote)) { + continue; + } + const end = lastEnds.get(quote); + if (end === undefined || end < match.index + match[0].length) { + continue; + } + parts.push( + text.slice(cursor, match.index), + `${prefix}${quote}Bearer [REDACTED]${quote}` + ); + cursor = end + quote.length; + } + parts.push(text.slice(cursor)); + return parts.join(""); +} + +/** Remove recognizable credentials from diagnostics without retaining secrets. */ +export function redactCredentialText(text: string): string { + return redactInvalidBearerHeaders(text) + .replace(QUOTED_CREDENTIAL, (_match, quote: string, prefix: string) => + prefix.toLowerCase().startsWith("bearer") + ? `${quote}Bearer [REDACTED]${quote}` + : `${quote}[REDACTED]${quote}` + ) + .replace(BEARER_CREDENTIAL, "Bearer [REDACTED]") + .replace(SENTRY_CREDENTIAL, "[REDACTED]"); +} + +/** Materialize the same JSON as the SDK before redacting a detached copy. */ +function redactJson(value: T): T { + let serialized: string | undefined; + try { + serialized = JSON.stringify(value); + } catch { + // This is the SDK's envelope serialization fallback for cycles and BigInt. + serialized = JSON.stringify(normalize(value)); + } + const copy: T = JSON.parse(serialized ?? "null"); + if (typeof copy === "string") { + return redactCredentialText(copy) as T; + } + const pending: unknown[] = [copy]; + while (pending.length > 0) { + const current = pending.pop(); + if (current === null || typeof current !== "object") { + continue; + } + for (const [key, nested] of Object.entries(current)) { + if (typeof nested === "string") { + (current as Record)[key] = + redactCredentialText(nested); + } else { + pending.push(nested); + } + } + } + return copy; +} + +/** + * Scrub the final envelope, after SDK metadata and log attributes are resolved. + * JSON materialization preserves boxed values/toJSON without mutating live + * scopes, client options, or caller-owned objects. Binary attachments stay intact. + */ +export function redactTelemetryEnvelope(envelope: Envelope): Envelope { + return [ + redactJson(envelope[0]), + envelope[1].map(([headers, payload]) => { + const safeHeaders = redactJson(headers); + const safePayload = + payload instanceof Uint8Array ? payload : redactJson(payload); + if (typeof safePayload === "string" && headers.length !== undefined) { + safeHeaders.length = Buffer.byteLength(safePayload, "utf8"); + } + return [safeHeaders, safePayload]; + }), + ] as Envelope; +} diff --git a/packages/cli/src/lib/error-reporting.ts b/packages/cli/src/lib/error-reporting.ts index 99e3a8f2f..b8856107f 100644 --- a/packages/cli/src/lib/error-reporting.ts +++ b/packages/cli/src/lib/error-reporting.ts @@ -27,6 +27,7 @@ // biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import import * as Sentry from "@sentry/node-core/light"; +import { redactCredentialText } from "./credential-redaction.js"; import { ApiError, AuthError, @@ -255,7 +256,7 @@ export function extractResourceKind(resource: string): string { * `"Invalid trace ID \"abc\". Expected ..."` → `"Invalid trace ID"` (with maxWords=3) */ export function extractMessagePrefix(message: string, maxWords = 3): string { - const firstLine = message.split("\n", 1)[0] ?? ""; + const firstLine = redactCredentialText(message).split("\n", 1)[0] ?? ""; return firstLine .replace(/'[^']*'/g, "") .replace(/"[^"]*"/g, "") diff --git a/packages/cli/src/lib/errors.ts b/packages/cli/src/lib/errors.ts index 92c1ca771..ac7e7e1b3 100644 --- a/packages/cli/src/lib/errors.ts +++ b/packages/cli/src/lib/errors.ts @@ -23,6 +23,7 @@ * @see https://cli.sentry.dev/exit-codes/ for full reference */ +import { redactCredentialText } from "./credential-redaction.js"; import { buildBillingUrl, buildOrgSettingsUrl, @@ -803,9 +804,9 @@ export function stringifyUnknown(value: unknown): string { */ export function formatError(error: unknown): string { if (error instanceof CliError) { - return error.format(); + return redactCredentialText(error.format()); } - return stringifyUnknown(error); + return redactCredentialText(stringifyUnknown(error)); } /** diff --git a/packages/cli/src/lib/sdk-types.ts b/packages/cli/src/lib/sdk-types.ts index 5ad09e6b9..c930df1eb 100644 --- a/packages/cli/src/lib/sdk-types.ts +++ b/packages/cli/src/lib/sdk-types.ts @@ -7,6 +7,8 @@ * @module */ +import { redactCredentialText } from "./credential-redaction.js"; + /** Options for programmatic CLI invocation. */ export type SentryOptions = { /** @@ -73,13 +75,13 @@ export class SentryError extends Error { /** CLI exit code (non-zero). */ readonly exitCode: number; - /** Raw stderr output from the command. */ + /** Captured stderr output with recognizable credentials redacted. */ readonly stderr: string; constructor(message: string, exitCode: number, stderr: string) { - super(message); + super(redactCredentialText(message)); this.name = "SentryError"; this.exitCode = exitCode; - this.stderr = stderr; + this.stderr = redactCredentialText(stderr); } } diff --git a/packages/cli/src/lib/telemetry.ts b/packages/cli/src/lib/telemetry.ts index 18109ed3d..0970a7053 100644 --- a/packages/cli/src/lib/telemetry.ts +++ b/packages/cli/src/lib/telemetry.ts @@ -23,6 +23,7 @@ import { getConfiguredSentryUrl, SENTRY_CLI_DSN, } from "./constants.js"; +import { redactTelemetryEnvelope } from "./credential-redaction.js"; import { getCustomCaCerts } from "./custom-ca.js"; import { getTelemetryPreference } from "./db/defaults.js"; import { isReadonlyError, tryRepairAndRetry } from "./db/schema.js"; @@ -595,7 +596,15 @@ export function initSentry( // smaller payloads, faster compress/decompress on both sides. // Automatic gzip fallback when running on Node < 22.15, where // `node:zlib`'s zstd support is unavailable. - transport: makeCompressedTransport, + transport: (transportOptions) => { + const transport = makeCompressedTransport(transportOptions); + return { + // The SDK adds log scope attributes after beforeSendLog and skips + // beforeSend for internal errors. Redact at the final delivery boundary. + send: (envelope) => transport.send(redactTelemetryEnvelope(envelope)), + flush: (timeout) => transport.flush(timeout), + }; + }, // Pass custom CA certificates to the transport for corporate TLS proxies. // The zstd-transport reads `caCerts` and passes it as `ca:` to // `http.request()`, and the SDK's fallback `makeNodeTransport` does the same. diff --git a/packages/cli/test/e2e/auth.test.ts b/packages/cli/test/e2e/auth.test.ts index efd9a2f9a..260f834a8 100644 --- a/packages/cli/test/e2e/auth.test.ts +++ b/packages/cli/test/e2e/auth.test.ts @@ -17,7 +17,7 @@ import { } from "vitest"; import { Database } from "../../src/lib/db/sqlite.js"; import { EXIT } from "../../src/lib/errors.js"; -import { createE2EContext, type E2EContext } from "../fixture.js"; +import { createE2EContext, type E2EContext, runCli } from "../fixture.js"; import { cleanupTestDir, createTestConfigDir } from "../helpers.js"; import { createSentryMockServer, TEST_TOKEN } from "../mocks/routes.js"; import type { MockServer } from "../mocks/server.js"; @@ -250,3 +250,26 @@ describe("sentry auth logout", () => { expect(result.exitCode).toBe(0); }); }); + +describe("command error redaction", () => { + test("redacts unexpected command errors handled inside Stricli", async () => { + const result = await runCli(["auth", "whoami", "--json"], { + env: { + SENTRY_CONFIG_DIR: testConfigDir, + SENTRY_URL: mockServer.url, + SENTRY_AUTH_TOKEN: TEST_TOKEN, + SENTRY_FORCE_ENV_TOKEN: "1", + SENTRY_CUSTOM_HEADERS: + "X-Proxy: Bearer SYNTHETIC-PREFIX\rSYNTHETIC-SECRET-TAIL", + SENTRY_CLI_NO_TELEMETRY: "1", + }, + }); + const output = result.stdout + result.stderr; + + expect(result.exitCode).toBe(EXIT.GENERAL); + expect(output).toContain("Unexpected error: TypeError:"); + expect(output).toContain("[REDACTED]"); + expect(output).not.toContain("SYNTHETIC-PREFIX"); + expect(output).not.toContain("SYNTHETIC-SECRET-TAIL"); + }); +}); diff --git a/packages/cli/test/lib/credential-redaction.test.ts b/packages/cli/test/lib/credential-redaction.test.ts new file mode 100644 index 000000000..30c755e4c --- /dev/null +++ b/packages/cli/test/lib/credential-redaction.test.ts @@ -0,0 +1,182 @@ +import { describe, expect, test } from "vitest"; +import { redactCredentialText } from "../../src/lib/credential-redaction.js"; +import { ApiError, formatError, getExitCode } from "../../src/lib/errors.js"; +import { SentryError } from "../../src/lib/sdk-types.js"; + +describe("credential redaction", () => { + test.each([ + "sntrys_SYNTHETIC_PAYLOAD\n_SYNTHETIC_SECRET", + "sntryu_SYNTHETIC_PAYLOAD\r\n_SYNTHETIC_SECRET", + "legacy_SYNTHETIC_PAYLOAD\n_SYNTHETIC_SECRET", + "legacy_SYNTHETIC_PAYLOAD\\n_SYNTHETIC_SECRET", + 'legacy_SYNTHETIC_"PAYLOAD\n_SYNTHETIC_SECRET', + ])("redacts the entire invalid Bearer header: %j", (token) => { + const message = `Headers.set: "Bearer ${token}" is an invalid header value.`; + expect(redactCredentialText(message)).toBe( + 'Headers.set: "Bearer [REDACTED]" is an invalid header value.' + ); + }); + + test("handles quotes escaped by JSON serialization", () => { + const message = JSON.stringify({ + error: 'Headers.set: "Bearer legacy_SYNTHETIC\n_SECRET" is invalid.', + }); + expect(JSON.parse(redactCredentialText(message))).toEqual({ + error: 'Headers.set: "Bearer [REDACTED]" is invalid.', + }); + }); + + test.each([ + 1, 2, 3, + ])("preserves %i levels of JSON escaping around an invalid header", (levels) => { + let input = + 'Headers.set: "Bearer legacy_SYNTHETIC_"PAYLOAD\n_SECRET" is an invalid header value.'; + let expected = + 'Headers.set: "Bearer [REDACTED]" is an invalid header value.'; + for (let level = 0; level < levels; level++) { + input = JSON.stringify({ error: input }); + expected = JSON.stringify({ error: expected }); + } + expect(redactCredentialText(input)).toBe(expected); + }); + + test("preserves nested JSON escaping around quoted Sentry credentials", () => { + const input = JSON.stringify({ + error: JSON.stringify({ error: 'Rejected "sntryu_SYNTHETIC_SECRET".' }), + }); + const redacted = redactCredentialText(input); + expect(JSON.parse(JSON.parse(redacted).error)).toEqual({ + error: 'Rejected "[REDACTED]".', + }); + }); + + test("handles many incomplete runtime diagnostics", () => { + const diagnostic = 'Headers.set: "Bearer SYNTHETIC_SECRET"; '; + const expected = 'Headers.set: "Bearer [REDACTED]"; '; + expect(redactCredentialText(diagnostic.repeat(10_000))).toBe( + expected.repeat(10_000) + ); + }); + + test.each([ + "\t", + "\v", + "\f", + "\0", + "\b", + "\u0085", + "\u00a0", + "\u2028", + "\u2029", + "\\t", + "\\b", + "\\u0000", + "\\u2028", + ])("redacts token fragments separated by %j", (separator) => { + for (const prefix of ["sntryu_", "sntrys_", "Bearer opaque_"]) { + const input = `Rejected ${prefix}SYNTHETIC_FIRST${separator}SYNTHETIC_TAIL`; + expect(redactCredentialText(input)).not.toContain("SYNTHETIC"); + expect( + redactCredentialText(JSON.stringify({ error: input })) + ).not.toContain("SYNTHETIC"); + } + }); + + test.each([ + [ + "sntrys_SYNTHETIC_PAYLOAD\n_SYNTHETIC_SECRET", + "Rejected [REDACTED]; try again.", + ], + [ + "sntryu_SYNTHETIC_PAYLOAD\\n_SYNTHETIC_SECRET", + "Rejected [REDACTED]; try again.", + ], + [ + "Bearer legacy_SYNTHETIC_PAYLOAD\n_SYNTHETIC_SECRET", + "Rejected Bearer [REDACTED] try again.", + ], + ])("redacts recognizable unquoted credentials: %j", (token, expected) => { + const redacted = redactCredentialText(`Rejected ${token}; try again.`); + expect(redacted).not.toContain("SYNTHETIC"); + expect(redacted).toBe(expected); + expect(redactCredentialText(redacted)).toBe(redacted); + }); + + test.each([ + "opaque:SYNTHETIC_SECRET", + "opaque!SYNTHETIC_SECRET", + "opaque@SYNTHETIC_SECRET", + "opaque;SYNTHETIC_SECRET", + "opaque,[SYNTHETIC_SECRET]{}", + "opaque\\SYNTHETIC_SECRET", + "opaque:SYNTHETIC_PAYLOAD\n !SYNTHETIC_SECRET", + "opaque@SYNTHETIC_PAYLOAD\\n :SYNTHETIC_SECRET", + ])("redacts punctuation in an unquoted Bearer value: %j", (token) => { + const text = `Authorization: Bearer ${token}`; + const expected = "Authorization: Bearer [REDACTED]"; + expect(redactCredentialText(text)).toBe(expected); + expect(redactCredentialText(expected)).toBe(expected); + + const serialized = JSON.stringify({ error: text, status: 401 }); + const redacted = redactCredentialText(serialized); + expect(JSON.parse(redacted)).toEqual({ error: expected, status: 401 }); + expect(redactCredentialText(redacted)).toBe(redacted); + }); + + test("preserves escaped JSON quote delimiters after an unquoted Bearer", () => { + const nested = JSON.stringify({ + error: "Authorization: Bearer opaque:SECRET", + }); + const serialized = JSON.stringify({ nested }); + const redacted = redactCredentialText(serialized); + expect(JSON.parse(JSON.parse(redacted).nested)).toEqual({ + error: "Authorization: Bearer [REDACTED]", + }); + expect(redactCredentialText(redacted)).toBe(redacted); + }); + + test("redacts a quoted header truncated before its closing quote", () => { + expect( + redactCredentialText('Headers.set: "Bearer legacy_PAYLOAD\n_SECRET...') + ).toBe('Headers.set: "Bearer [REDACTED]"'); + }); + + test("leaves ordinary diagnostic text intact", () => { + const text = 'GET /api/0/projects/ failed: "Not found" (HTTP 404).'; + expect(redactCredentialText(text)).toBe(text); + }); +}); + +describe("error output boundaries", () => { + const token = "sntrys_SYNTHETIC_PAYLOAD\n_SYNTHETIC_SECRET"; + const message = `Headers.set: "Bearer ${token}" is an invalid header value.`; + + test("formats errors safely without changing the error or exit code", () => { + const error = new ApiError(message, 500, `Details: ${token}`); + expect(formatError(error)).not.toContain("SYNTHETIC"); + expect(error).toBeInstanceOf(ApiError); + expect(error.message).toBe(message); + expect(error.status).toBe(500); + expect(getExitCode(error)).toBe(30); + }); + + test("formats non-Error thrown objects safely", () => { + const formatted = formatError({ error: message }); + expect(formatted).not.toContain("SYNTHETIC"); + expect(JSON.parse(formatted)).toEqual({ + error: 'Headers.set: "Bearer [REDACTED]" is an invalid header value.', + }); + }); + + test("SDK error message, stack, stderr, and JSON are safe", () => { + const error = new SentryError(message, 12, `${message}\n`); + expect(error.name).toBe("SentryError"); + expect(error.exitCode).toBe(12); + expect(error.message).not.toContain("SYNTHETIC"); + expect(error.stack).not.toContain("SYNTHETIC"); + expect(error.stderr).not.toContain("SYNTHETIC"); + expect(JSON.stringify({ message: error.message, ...error })).not.toContain( + "SYNTHETIC" + ); + }); +}); diff --git a/packages/cli/test/lib/telemetry-credentials.test.ts b/packages/cli/test/lib/telemetry-credentials.test.ts new file mode 100644 index 000000000..2822d770b --- /dev/null +++ b/packages/cli/test/lib/telemetry-credentials.test.ts @@ -0,0 +1,191 @@ +import { + _INTERNAL_flushLogsBuffer, + type Envelope, + type ErrorEvent, + type Transport, +} from "@sentry/core"; +import { + captureEvent, + captureException, + getCurrentScope, + getIsolationScope, + logger, + setContext, + startSpan, + withScope, +} from "@sentry/node-core/light"; +import { afterEach, describe, expect, test, vi } from "vitest"; +import { redactTelemetryEnvelope } from "../../src/lib/credential-redaction.js"; +import { extractMessagePrefix } from "../../src/lib/error-reporting.js"; +// biome-ignore lint/performance/noNamespaceImport: spy on the outbound transport factory +import * as transportModule from "../../src/lib/telemetry/zstd-transport.js"; +import { initSentry } from "../../src/lib/telemetry.js"; + +const token = "sntrys_SYNTHETIC_PAYLOAD\n_SYNTHETIC_SECRET"; +const headerError = `Headers.set: "Bearer ${token}" is an invalid header value.`; +let client: ReturnType; + +/** Intercept the delegated delivery boundary while keeping the real SDK pipeline. */ +function captureTelemetry() { + const send = vi.fn().mockResolvedValue({}); + const flush = vi.fn().mockResolvedValue(true); + vi.spyOn(transportModule, "makeCompressedTransport").mockReturnValue({ + send, + flush, + }); + client = initSentry(false, { libraryMode: true }); + if (!client) { + throw new Error("Expected the telemetry client"); + } + // Enable capture only after every delivery route is intercepted. + client.getOptions().enabled = true; + client.getOptions().enableLogs = true; + client.init(); + return { client, send, flush }; +} + +afterEach(async () => { + await client?.close(0); + getCurrentScope().clear(); + getIsolationScope().clear(); + vi.restoreAllMocks(); +}); + +describe("telemetry credential boundaries", () => { + test("sanitizes exceptions and related fields in the outgoing envelope", async () => { + const capture = captureTelemetry(); + const event: ErrorEvent = { + type: undefined, + message: headerError, + exception: { + values: [ + { type: "TypeError", value: headerError }, + { type: "Error", value: `Wrapping ${token}` }, + ], + }, + tags: { "cli_error.kind": token }, + contexts: { cli_error: { detail: headerError, status: 500 } }, + extra: { stack: headerError }, + breadcrumbs: [{ message: headerError, data: { token } }], + }; + captureEvent(event); + await capture.client.flush(1000); + + const envelope = capture.send.mock.calls.find(([entry]) => + entry[1].some(([header]) => header.type === "event") + )?.[0]; + expect(envelope).toBeDefined(); + expect(JSON.stringify(envelope)).not.toContain("SYNTHETIC"); + expect(JSON.stringify(envelope)).toContain("[REDACTED]"); + expect(JSON.stringify(envelope)).toContain('"status":500'); + expect(event.contexts?.cli_error?.detail).toBe(headerError); + }); + + test("scrubs exception causes in the outgoing Sentry envelope", async () => { + const capture = captureTelemetry(); + captureException( + new TypeError(headerError, { cause: new Error(`Rejected ${token}`) }) + ); + await capture.client.flush(1000); + + const envelope = capture.send.mock.calls.find(([entry]) => + entry[1].some(([header]) => header.type === "event") + )?.[0]; + expect(envelope).toBeDefined(); + const serialized = JSON.stringify(envelope); + expect(serialized).toContain("[REDACTED]"); + expect(serialized).toContain("TypeError"); + expect(serialized).toContain("Rejected"); + expect(serialized).not.toContain("SYNTHETIC"); + }); + + test("scrubs logs after the SDK adds scope attributes and fmt parameters", async () => { + const capture = captureTelemetry(); + const frozen = Object.freeze({ token }); + // biome-ignore lint/style/useConsistentBuiltinInstantiation: regression coverage for boxed log parameters + const boxed = new String(token); + withScope((scope) => { + scope.setAttribute("scope.token", token); + logger.error(logger.fmt`Rejected ${boxed}`, { + nested: frozen, + serialized: { toJSON: () => token }, + status: 500, + }); + }); + _INTERNAL_flushLogsBuffer(capture.client); + await capture.client.flush(1000); + + const envelope = capture.send.mock.calls.find(([entry]) => + entry[1].some(([header]) => header.type === "log") + )?.[0]; + expect(envelope).toBeDefined(); + const serialized = JSON.stringify(envelope); + expect(serialized).not.toContain("SYNTHETIC"); + expect(serialized).toContain("scope.token"); + expect(serialized).toContain("sentry.message.parameter.0"); + expect(serialized).toContain("Rejected [REDACTED]"); + expect(frozen.token).toBe(token); + expect(String(boxed)).toBe(token); + }); + + test("does not traverse live scopes or client options when scrubbing a trace", async () => { + const capture = captureTelemetry(); + const context = Object.freeze({ token }); + setContext("synthetic", context); + capture.client.getOptions().release = token; + startSpan({ name: "synthetic-review", forceTransaction: true }, (span) => { + span.setAttribute("diagnostic", headerError); + }); + await capture.client.flush(1000); + + const envelopes = capture.send.mock.calls.map(([envelope]) => envelope); + expect(envelopes).toHaveLength(1); + expect(envelopes[0]?.[1][0]?.[0].type).toBe("transaction"); + expect(JSON.stringify(envelopes)).not.toContain("SYNTHETIC"); + expect(context.token).toBe(token); + expect(capture.client.getOptions().release).toBe(token); + expect(capture.flush).toHaveBeenCalledWith(1000); + }); + + test("scrubs SDK internal errors that bypass beforeSend", async () => { + const capture = captureTelemetry(); + setContext("synthetic", { token }); + capture.client.captureException(new Error(headerError), { + data: { __sentry__: true }, + }); + await capture.client.flush(1000); + + expect(capture.send).toHaveBeenCalledOnce(); + const serialized = JSON.stringify(capture.send.mock.calls); + expect(serialized).toContain("[REDACTED]"); + expect(serialized).not.toContain("SYNTHETIC"); + }); + + test("redacts before deriving a grouping key from the first line", () => { + expect(extractMessagePrefix(headerError, 4)).toBe( + "Headers.set: is an invalid" + ); + }); + + test("uses SDK serialization fallback for cycles and preserves binary attachments", () => { + const data: { token: string; circular?: unknown } = { token }; + data.circular = data; + const bytes = new Uint8Array([1, 2, 3]); + const envelope: Envelope = [ + {}, + [ + [{ type: "event" }, { extra: data }], + [ + { type: "attachment", length: bytes.length, filename: "test.bin" }, + bytes, + ], + ], + ]; + const redacted = redactTelemetryEnvelope(envelope); + expect(JSON.stringify(redacted)).not.toContain("SYNTHETIC"); + expect(JSON.stringify(redacted)).toContain("[Circular ~]"); + expect(redacted[1][1]?.[1]).toBe(bytes); + expect(data.token).toBe(token); + expect(data.circular).toBe(data); + }); +}); From c060c540bf913ad50f71f4374c9c0d3328f9384a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Beteg=C3=B3n?= Date: Tue, 29 Sep 2026 07:44:06 +0200 Subject: [PATCH 2/3] refactor(telemetry): simplify credential redaction and tests --- packages/cli/src/lib/credential-redaction.ts | 19 ++++-- packages/cli/src/lib/errors.ts | 7 +-- .../cli/test/lib/credential-redaction.test.ts | 61 ++++++------------- .../test/lib/telemetry-credentials.test.ts | 46 +++++++------- 4 files changed, 57 insertions(+), 76 deletions(-) diff --git a/packages/cli/src/lib/credential-redaction.ts b/packages/cli/src/lib/credential-redaction.ts index a35fcc68f..908117cf7 100644 --- a/packages/cli/src/lib/credential-redaction.ts +++ b/packages/cli/src/lib/credential-redaction.ts @@ -12,17 +12,24 @@ const INVALID_HEADER_END = /(? { "legacy_SYNTHETIC_PAYLOAD\n_SYNTHETIC_SECRET", "legacy_SYNTHETIC_PAYLOAD\\n_SYNTHETIC_SECRET", 'legacy_SYNTHETIC_"PAYLOAD\n_SYNTHETIC_SECRET', - ])("redacts the entire invalid Bearer header: %j", (token) => { - const message = `Headers.set: "Bearer ${token}" is an invalid header value.`; - expect(redactCredentialText(message)).toBe( - 'Headers.set: "Bearer [REDACTED]" is an invalid header value.' - ); + 'legacy_SYNTHETIC_" is an invalid header value\n_SYNTHETIC_SECRET', + ])("redacts invalid Bearer headers through JSON escaping: %j", (token) => { + let input = `Headers.set: "Bearer ${token}" is an invalid header value.`; + let expected = + 'Headers.set: "Bearer [REDACTED]" is an invalid header value.'; + for (let level = 0; level <= 3; level++) { + expect(redactCredentialText(input)).toBe(expected); + input = JSON.stringify({ error: input }); + expected = JSON.stringify({ error: expected }); + } }); test("handles quotes escaped by JSON serialization", () => { @@ -26,20 +31,6 @@ describe("credential redaction", () => { }); }); - test.each([ - 1, 2, 3, - ])("preserves %i levels of JSON escaping around an invalid header", (levels) => { - let input = - 'Headers.set: "Bearer legacy_SYNTHETIC_"PAYLOAD\n_SECRET" is an invalid header value.'; - let expected = - 'Headers.set: "Bearer [REDACTED]" is an invalid header value.'; - for (let level = 0; level < levels; level++) { - input = JSON.stringify({ error: input }); - expected = JSON.stringify({ error: expected }); - } - expect(redactCredentialText(input)).toBe(expected); - }); - test("preserves nested JSON escaping around quoted Sentry credentials", () => { const input = JSON.stringify({ error: JSON.stringify({ error: 'Rejected "sntryu_SYNTHETIC_SECRET".' }), @@ -97,7 +88,6 @@ describe("credential redaction", () => { ], ])("redacts recognizable unquoted credentials: %j", (token, expected) => { const redacted = redactCredentialText(`Rejected ${token}; try again.`); - expect(redacted).not.toContain("SYNTHETIC"); expect(redacted).toBe(expected); expect(redactCredentialText(redacted)).toBe(redacted); }); @@ -112,27 +102,15 @@ describe("credential redaction", () => { "opaque:SYNTHETIC_PAYLOAD\n !SYNTHETIC_SECRET", "opaque@SYNTHETIC_PAYLOAD\\n :SYNTHETIC_SECRET", ])("redacts punctuation in an unquoted Bearer value: %j", (token) => { - const text = `Authorization: Bearer ${token}`; - const expected = "Authorization: Bearer [REDACTED]"; - expect(redactCredentialText(text)).toBe(expected); - expect(redactCredentialText(expected)).toBe(expected); - - const serialized = JSON.stringify({ error: text, status: 401 }); - const redacted = redactCredentialText(serialized); - expect(JSON.parse(redacted)).toEqual({ error: expected, status: 401 }); - expect(redactCredentialText(redacted)).toBe(redacted); - }); - - test("preserves escaped JSON quote delimiters after an unquoted Bearer", () => { - const nested = JSON.stringify({ - error: "Authorization: Bearer opaque:SECRET", - }); - const serialized = JSON.stringify({ nested }); - const redacted = redactCredentialText(serialized); - expect(JSON.parse(JSON.parse(redacted).nested)).toEqual({ - error: "Authorization: Bearer [REDACTED]", - }); - expect(redactCredentialText(redacted)).toBe(redacted); + let input = `Authorization: Bearer ${token}`; + let expected = "Authorization: Bearer [REDACTED]"; + for (let level = 0; level <= 2; level++) { + const redacted = redactCredentialText(input); + expect(redacted).toBe(expected); + expect(redactCredentialText(redacted)).toBe(expected); + input = JSON.stringify({ error: input, status: 401 }); + expected = JSON.stringify({ error: expected, status: 401 }); + } }); test("redacts a quoted header truncated before its closing quote", () => { @@ -162,7 +140,6 @@ describe("error output boundaries", () => { test("formats non-Error thrown objects safely", () => { const formatted = formatError({ error: message }); - expect(formatted).not.toContain("SYNTHETIC"); expect(JSON.parse(formatted)).toEqual({ error: 'Headers.set: "Bearer [REDACTED]" is an invalid header value.', }); diff --git a/packages/cli/test/lib/telemetry-credentials.test.ts b/packages/cli/test/lib/telemetry-credentials.test.ts index 2822d770b..ed60bd0b2 100644 --- a/packages/cli/test/lib/telemetry-credentials.test.ts +++ b/packages/cli/test/lib/telemetry-credentials.test.ts @@ -41,7 +41,16 @@ function captureTelemetry() { client.getOptions().enabled = true; client.getOptions().enableLogs = true; client.init(); - return { client, send, flush }; + function serializedEnvelope(type: string): string { + const envelope = send.mock.calls.find(([entry]) => + entry[1].some(([header]) => header.type === type) + )?.[0]; + if (!envelope) { + throw new Error(`Expected a ${type} envelope`); + } + return JSON.stringify(envelope); + } + return { client, send, flush, serializedEnvelope }; } afterEach(async () => { @@ -71,13 +80,10 @@ describe("telemetry credential boundaries", () => { captureEvent(event); await capture.client.flush(1000); - const envelope = capture.send.mock.calls.find(([entry]) => - entry[1].some(([header]) => header.type === "event") - )?.[0]; - expect(envelope).toBeDefined(); - expect(JSON.stringify(envelope)).not.toContain("SYNTHETIC"); - expect(JSON.stringify(envelope)).toContain("[REDACTED]"); - expect(JSON.stringify(envelope)).toContain('"status":500'); + const serialized = capture.serializedEnvelope("event"); + expect(serialized).not.toContain("SYNTHETIC"); + expect(serialized).toContain("[REDACTED]"); + expect(serialized).toContain('"status":500'); expect(event.contexts?.cli_error?.detail).toBe(headerError); }); @@ -88,11 +94,7 @@ describe("telemetry credential boundaries", () => { ); await capture.client.flush(1000); - const envelope = capture.send.mock.calls.find(([entry]) => - entry[1].some(([header]) => header.type === "event") - )?.[0]; - expect(envelope).toBeDefined(); - const serialized = JSON.stringify(envelope); + const serialized = capture.serializedEnvelope("event"); expect(serialized).toContain("[REDACTED]"); expect(serialized).toContain("TypeError"); expect(serialized).toContain("Rejected"); @@ -115,11 +117,7 @@ describe("telemetry credential boundaries", () => { _INTERNAL_flushLogsBuffer(capture.client); await capture.client.flush(1000); - const envelope = capture.send.mock.calls.find(([entry]) => - entry[1].some(([header]) => header.type === "log") - )?.[0]; - expect(envelope).toBeDefined(); - const serialized = JSON.stringify(envelope); + const serialized = capture.serializedEnvelope("log"); expect(serialized).not.toContain("SYNTHETIC"); expect(serialized).toContain("scope.token"); expect(serialized).toContain("sentry.message.parameter.0"); @@ -136,12 +134,12 @@ describe("telemetry credential boundaries", () => { startSpan({ name: "synthetic-review", forceTransaction: true }, (span) => { span.setAttribute("diagnostic", headerError); }); - await capture.client.flush(1000); + capture.flush.mockResolvedValueOnce(false); + await expect(capture.client.flush(1000)).resolves.toBe(false); - const envelopes = capture.send.mock.calls.map(([envelope]) => envelope); - expect(envelopes).toHaveLength(1); - expect(envelopes[0]?.[1][0]?.[0].type).toBe("transaction"); - expect(JSON.stringify(envelopes)).not.toContain("SYNTHETIC"); + expect(capture.send).toHaveBeenCalledOnce(); + const serialized = capture.serializedEnvelope("transaction"); + expect(serialized).not.toContain("SYNTHETIC"); expect(context.token).toBe(token); expect(capture.client.getOptions().release).toBe(token); expect(capture.flush).toHaveBeenCalledWith(1000); @@ -156,7 +154,7 @@ describe("telemetry credential boundaries", () => { await capture.client.flush(1000); expect(capture.send).toHaveBeenCalledOnce(); - const serialized = JSON.stringify(capture.send.mock.calls); + const serialized = capture.serializedEnvelope("event"); expect(serialized).toContain("[REDACTED]"); expect(serialized).not.toContain("SYNTHETIC"); }); From abb01d218c45f347e59cb710a09bd292e4834601 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Beteg=C3=B3n?= Date: Tue, 29 Sep 2026 07:59:46 +0200 Subject: [PATCH 3/3] fix(cli): keep startup light and preserve fatal error names --- packages/cli/src/cli.ts | 3 +- packages/cli/src/lib/credential-redaction.ts | 56 +------------------ packages/cli/src/lib/telemetry.ts | 2 +- .../src/lib/telemetry/credential-redaction.ts | 55 ++++++++++++++++++ packages/cli/test/lib/cli-startup.test.ts | 37 ++++++++++++ .../test/lib/telemetry-credentials.test.ts | 2 +- packages/cli/test/script/cli-startup.test.ts | 46 +++++++++++++++ 7 files changed, 144 insertions(+), 57 deletions(-) create mode 100644 packages/cli/src/lib/telemetry/credential-redaction.ts create mode 100644 packages/cli/test/lib/cli-startup.test.ts create mode 100644 packages/cli/test/script/cli-startup.test.ts diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 2b95fd8aa..1b8580abf 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -10,6 +10,7 @@ * stream error handlers and calls `startCli()`. */ +import { redactCredentialText } from "./lib/credential-redaction.js"; import { getEnv } from "./lib/env.js"; import { CliError, formatError } from "./lib/errors.js"; import { initTimezone } from "./lib/timezone.js"; @@ -701,7 +702,7 @@ export async function startCli(): Promise { } return runCli(args).catch((err) => { - process.stderr.write(`Fatal: ${formatError(err)}\n`); + process.stderr.write(`Fatal: ${redactCredentialText(String(err))}\n`); process.exitCode = 1; }); } diff --git a/packages/cli/src/lib/credential-redaction.ts b/packages/cli/src/lib/credential-redaction.ts index 908117cf7..445f59627 100644 --- a/packages/cli/src/lib/credential-redaction.ts +++ b/packages/cli/src/lib/credential-redaction.ts @@ -1,10 +1,9 @@ /** * Stateless credential redaction for CLI diagnostics and telemetry. - * Kept separate from API data formatting so successful responses stay intact. + * Kept free of SDK imports for CLI startup and the completion fast path. + * Successful API responses are not passed through this redactor. */ -import { type Envelope, normalize } from "@sentry/core"; - const INVALID_BEARER_HEADER_START = /(\bHeaders\.(?:set|append):[ \t]*)(\\*["'])Bearer[ \t]+/gi; const INVALID_HEADER_END = /(?(value: T): T { - let serialized: string | undefined; - try { - serialized = JSON.stringify(value); - } catch { - // This is the SDK's envelope serialization fallback for cycles and BigInt. - serialized = JSON.stringify(normalize(value)); - } - const copy: T = JSON.parse(serialized ?? "null"); - if (typeof copy === "string") { - return redactCredentialText(copy) as T; - } - const pending: unknown[] = [copy]; - while (pending.length > 0) { - const current = pending.pop(); - if (current === null || typeof current !== "object") { - continue; - } - for (const [key, nested] of Object.entries(current)) { - if (typeof nested === "string") { - (current as Record)[key] = - redactCredentialText(nested); - } else { - pending.push(nested); - } - } - } - return copy; -} - -/** - * Scrub the final envelope, after SDK metadata and log attributes are resolved. - * JSON materialization preserves boxed values/toJSON without mutating live - * scopes, client options, or caller-owned objects. Binary attachments stay intact. - */ -export function redactTelemetryEnvelope(envelope: Envelope): Envelope { - return [ - redactJson(envelope[0]), - envelope[1].map(([headers, payload]) => { - const safeHeaders = redactJson(headers); - const safePayload = - payload instanceof Uint8Array ? payload : redactJson(payload); - if (typeof safePayload === "string" && headers.length !== undefined) { - safeHeaders.length = Buffer.byteLength(safePayload, "utf8"); - } - return [safeHeaders, safePayload]; - }), - ] as Envelope; -} diff --git a/packages/cli/src/lib/telemetry.ts b/packages/cli/src/lib/telemetry.ts index 0970a7053..4175e7cbe 100644 --- a/packages/cli/src/lib/telemetry.ts +++ b/packages/cli/src/lib/telemetry.ts @@ -23,7 +23,6 @@ import { getConfiguredSentryUrl, SENTRY_CLI_DSN, } from "./constants.js"; -import { redactTelemetryEnvelope } from "./credential-redaction.js"; import { getCustomCaCerts } from "./custom-ca.js"; import { getTelemetryPreference } from "./db/defaults.js"; import { isReadonlyError, tryRepairAndRetry } from "./db/schema.js"; @@ -41,6 +40,7 @@ import { import { ApiError, isUserError } from "./errors.js"; import { attachSentryReporter, logger } from "./logger.js"; import { getSentryBaseUrl, isSentrySaasUrl } from "./sentry-urls.js"; +import { redactTelemetryEnvelope } from "./telemetry/credential-redaction.js"; import { makeCompressedTransport } from "./telemetry/zstd-transport.js"; import { getRealUsername } from "./utils.js"; diff --git a/packages/cli/src/lib/telemetry/credential-redaction.ts b/packages/cli/src/lib/telemetry/credential-redaction.ts new file mode 100644 index 000000000..63aefe36c --- /dev/null +++ b/packages/cli/src/lib/telemetry/credential-redaction.ts @@ -0,0 +1,55 @@ +/** Redact outgoing SDK envelopes without modifying live scopes or caller data. */ + +import { type Envelope, normalize } from "@sentry/core"; +import { redactCredentialText } from "../credential-redaction.js"; + +/** Materialize the same JSON as the SDK before redacting a detached copy. */ +function redactJson(value: T): T { + let serialized: string | undefined; + try { + serialized = JSON.stringify(value); + } catch { + // This is the SDK's envelope serialization fallback for cycles and BigInt. + serialized = JSON.stringify(normalize(value)); + } + const copy: T = JSON.parse(serialized ?? "null"); + if (typeof copy === "string") { + return redactCredentialText(copy) as T; + } + const pending: unknown[] = [copy]; + while (pending.length > 0) { + const current = pending.pop(); + if (current === null || typeof current !== "object") { + continue; + } + for (const [key, nested] of Object.entries(current)) { + if (typeof nested === "string") { + (current as Record)[key] = + redactCredentialText(nested); + } else { + pending.push(nested); + } + } + } + return copy; +} + +/** + * Scrub the final envelope, after SDK metadata and log attributes are resolved. + * JSON materialization preserves boxed values/toJSON without mutating live + * scopes, client options, or caller-owned objects. Binary attachments stay intact. + */ +export function redactTelemetryEnvelope(envelope: Envelope): Envelope { + return [ + redactJson(envelope[0]), + envelope[1].map(([headers, payload]) => { + const safeHeaders = redactJson(headers); + const safePayload = + payload instanceof Uint8Array ? payload : redactJson(payload); + if (typeof safePayload === "string" && headers.length !== undefined) { + safeHeaders.length = Buffer.byteLength(safePayload, "utf8"); + } + return [safeHeaders, safePayload]; + }), + ] as Envelope; +} diff --git a/packages/cli/test/lib/cli-startup.test.ts b/packages/cli/test/lib/cli-startup.test.ts new file mode 100644 index 000000000..96c66f175 --- /dev/null +++ b/packages/cli/test/lib/cli-startup.test.ts @@ -0,0 +1,37 @@ +import { expect, test, vi } from "vitest"; +import { startCli } from "../../src/cli.js"; +// biome-ignore lint/performance/noNamespaceImport: spy on the startup dependency +import * as upgrade from "../../src/lib/upgrade.js"; +import { useTestConfigDir } from "../helpers.js"; + +useTestConfigDir("cli-startup-"); + +test("fatal errors preserve their name while redacting credentials", async () => { + const argv = process.argv; + const exitCode = process.exitCode; + const stderr = vi + .spyOn(process.stderr, "write") + .mockImplementation(() => true); + const cleanup = vi + .spyOn(upgrade, "startCleanupOldBinary") + .mockImplementation(() => { + throw new TypeError( + 'Headers.set: "Bearer SYNTHETIC_PREFIX\nSYNTHETIC_SECRET" is an invalid header value.' + ); + }); + + try { + process.argv = ["node", "sentry", "--help"]; + await startCli(); + + expect(cleanup).toHaveBeenCalledOnce(); + expect(stderr).toHaveBeenLastCalledWith( + 'Fatal: TypeError: Headers.set: "Bearer [REDACTED]" is an invalid header value.\n' + ); + expect(process.exitCode).toBe(1); + } finally { + process.argv = argv; + process.exitCode = exitCode; + vi.restoreAllMocks(); + } +}); diff --git a/packages/cli/test/lib/telemetry-credentials.test.ts b/packages/cli/test/lib/telemetry-credentials.test.ts index ed60bd0b2..191376e92 100644 --- a/packages/cli/test/lib/telemetry-credentials.test.ts +++ b/packages/cli/test/lib/telemetry-credentials.test.ts @@ -15,8 +15,8 @@ import { withScope, } from "@sentry/node-core/light"; import { afterEach, describe, expect, test, vi } from "vitest"; -import { redactTelemetryEnvelope } from "../../src/lib/credential-redaction.js"; import { extractMessagePrefix } from "../../src/lib/error-reporting.js"; +import { redactTelemetryEnvelope } from "../../src/lib/telemetry/credential-redaction.js"; // biome-ignore lint/performance/noNamespaceImport: spy on the outbound transport factory import * as transportModule from "../../src/lib/telemetry/zstd-transport.js"; import { initSentry } from "../../src/lib/telemetry.js"; diff --git a/packages/cli/test/script/cli-startup.test.ts b/packages/cli/test/script/cli-startup.test.ts new file mode 100644 index 000000000..fa4a02e82 --- /dev/null +++ b/packages/cli/test/script/cli-startup.test.ts @@ -0,0 +1,46 @@ +/** Keep Sentry SDK dependencies out of the completion startup path. */ + +import { join } from "node:path"; +import { build } from "esbuild"; +import { expect, test } from "vitest"; + +const ANY_IMPORT = /.*/; + +test.each([ + "cli.ts", + "index.ts", +])("%s does not eagerly import the Sentry SDK", async (entry) => { + await expect( + build({ + entryPoints: [join(import.meta.dirname, "../../src", entry)], + bundle: true, + write: false, + platform: "node", + format: "esm", + logLevel: "silent", + plugins: [ + { + name: "check-startup-imports", + setup(builder) { + builder.onResolve({ filter: ANY_IMPORT }, (args) => { + // Deferred imports do not execute when an entry point loads. + if (args.kind === "dynamic-import") { + return { path: args.path, external: true }; + } + if (args.path.startsWith("@sentry/")) { + return { + errors: [ + { + text: `Eager SDK dependency ${args.path} from ${args.importer}`, + }, + ], + }; + } + return; + }); + }, + }, + ], + }) + ).resolves.toMatchObject({ errors: [] }); +});