From 2d452471dbc171ea2c7be3ad92be7a8ba480f165 Mon Sep 17 00:00:00 2001 From: "sentry[bot]" <39604003+sentry[bot]@users.noreply.github.com> Date: Sun, 4 Oct 2026 10:50:43 +0000 Subject: [PATCH] fix(cli): Guard baggage header against non-ASCII characters --- packages/cli/src/lib/sentry-client.ts | 23 ++- .../lib/sentry-client.baggage.mocked.test.ts | 135 ++++++++++++++++++ 2 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 packages/cli/test/lib/sentry-client.baggage.mocked.test.ts diff --git a/packages/cli/src/lib/sentry-client.ts b/packages/cli/src/lib/sentry-client.ts index 893b459d8..a0b443a4e 100644 --- a/packages/cli/src/lib/sentry-client.ts +++ b/packages/cli/src/lib/sentry-client.ts @@ -54,6 +54,15 @@ import { const log = logger.withTag("http"); +/** + * Matches any character outside the printable ASCII range that is valid in an + * HTTP header-field-value (tab + 0x20-0x7E). Node.js's undici enforces the + * ByteString constraint (≤ 255) on Headers.set(), but HTTP requires ASCII, so + * we guard at the ASCII boundary to be both correct and safe. + */ +// biome-ignore lint/suspicious/noControlCharactersInRegex: \x09 is the tab character, intentionally included as a valid HTTP header byte. +const NON_HTTP_HEADER_CHAR_RE = /[^\x09\x20-\x7e]/; + /** Default request timeout in milliseconds */ const REQUEST_TIMEOUT_MS = 30_000; @@ -170,7 +179,19 @@ function prepareHeaders( headers.set("sentry-trace", traceData["sentry-trace"]); } if (traceData.baggage) { - headers.set("baggage", traceData.baggage); + // Node.js undici enforces the HTTP ByteString constraint: every character + // in a header value must have a code point ≤ 255. The Sentry SDK embeds + // the release name verbatim into the baggage value, so a release name + // containing a non-Latin-1 character (e.g. Turkish dotless ı = U+0131) + // triggers a TypeError from Headers.set. Skip the header rather than + // crashing — distributed tracing is best-effort telemetry. + if (NON_HTTP_HEADER_CHAR_RE.test(traceData.baggage)) { + log.debug( + "Skipping baggage header: value contains non-ASCII characters that are not valid HTTP header bytes" + ); + } else { + headers.set("baggage", traceData.baggage); + } } // Inject user-configured custom headers for self-hosted proxies (IAP, diff --git a/packages/cli/test/lib/sentry-client.baggage.mocked.test.ts b/packages/cli/test/lib/sentry-client.baggage.mocked.test.ts new file mode 100644 index 000000000..54d634929 --- /dev/null +++ b/packages/cli/test/lib/sentry-client.baggage.mocked.test.ts @@ -0,0 +1,135 @@ +/** + * Tests for the baggage header ASCII guard in prepareHeaders() — CLI-3A8 + * regression coverage. + * + * Kept as a mocked sibling file because vi.mock() on @sentry/node-core/light + * must precede all module imports to take effect. + */ + +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; + +// Mock Setup — must precede all imports of the module under test + +const { getTraceDataMock } = vi.hoisted(() => ({ + getTraceDataMock: vi.fn<() => Record>(() => ({})), +})); + +vi.mock("@sentry/node-core/light", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...(actual as object), + getTraceData: getTraceDataMock, + }; +}); + +// Import AFTER mock setup +import { setAuthToken } from "../../src/lib/db/auth.js"; +import { + getSdkConfig, + resetAuthenticatedFetch, +} from "../../src/lib/sentry-client.js"; +import { mockFetch, useTestConfigDir } from "../helpers.js"; + +useTestConfigDir("sentry-client-baggage-"); + +const REGION_URL = "https://us.sentry.io"; + +function getAuthenticatedFetch(): typeof fetch { + return getSdkConfig(REGION_URL).fetch as typeof fetch; +} + +let originalFetch: typeof globalThis.fetch; + +beforeEach(async () => { + originalFetch = globalThis.fetch; + await setAuthToken("test-token"); + resetAuthenticatedFetch(); + getTraceDataMock.mockReturnValue({}); +}); + +afterEach(() => { + globalThis.fetch = originalFetch; + resetAuthenticatedFetch(); + vi.clearAllMocks(); +}); + +describe("prepareHeaders baggage guard", () => { + test("omits the baggage header when it contains non-ASCII characters", async () => { + // U+0131 is Turkish dotless ı, decimal 305 — the character seen in CLI-3A8. + // It appears in a release name embedded in the sentry-release baggage field. + const nonAsciiBaggage = + "sentry-environment=production,sentry-release=my-rel\u0131ase"; + + getTraceDataMock.mockReturnValue({ + "sentry-trace": "abc123", + baggage: nonAsciiBaggage, + }); + + const capturedHeaders: Record = {}; + globalThis.fetch = mockFetch(async (_input, init) => { + const headers = new Headers(init?.headers); + headers.forEach((value, key) => { + capturedHeaders[key] = value; + }); + return new Response("{}", { status: 200 }); + }); + + const authFetch = getAuthenticatedFetch(); + await authFetch(`${REGION_URL}/api/0/organizations/`, { method: "GET" }); + + // sentry-trace is pure ASCII so it should be forwarded + expect(capturedHeaders["sentry-trace"]).toBe("abc123"); + // baggage contains U+0131 — must be dropped to avoid a ByteString TypeError + expect(capturedHeaders.baggage).toBeUndefined(); + }); + + test("forwards the baggage header when it is valid ASCII", async () => { + const asciiBaggage = + "sentry-environment=production,sentry-release=1.2.3,sentry-public_key=abc"; + + getTraceDataMock.mockReturnValue({ + "sentry-trace": "def456", + baggage: asciiBaggage, + }); + + const capturedHeaders: Record = {}; + globalThis.fetch = mockFetch(async (_input, init) => { + const headers = new Headers(init?.headers); + headers.forEach((value, key) => { + capturedHeaders[key] = value; + }); + return new Response("{}", { status: 200 }); + }); + + const authFetch = getAuthenticatedFetch(); + await authFetch(`${REGION_URL}/api/0/organizations/`, { method: "GET" }); + + expect(capturedHeaders["sentry-trace"]).toBe("def456"); + expect(capturedHeaders.baggage).toBe(asciiBaggage); + }); + + test("omits the baggage header when it contains Latin-1 non-ASCII characters (> 0x7f, <= 0xff)", async () => { + // Characters in the range 0x80-0xFF are valid Latin-1 but not ASCII. + // undici (Node.js fetch) accepts them as ByteStrings (≤255) but they are + // still outside the valid HTTP header-field-value range. Guard them too. + const latin1Baggage = "sentry-release=v\xe9\xe0\xfc"; + + getTraceDataMock.mockReturnValue({ + baggage: latin1Baggage, + }); + + const capturedHeaders: Record = {}; + globalThis.fetch = mockFetch(async (_input, init) => { + const headers = new Headers(init?.headers); + headers.forEach((value, key) => { + capturedHeaders[key] = value; + }); + return new Response("{}", { status: 200 }); + }); + + const authFetch = getAuthenticatedFetch(); + await authFetch(`${REGION_URL}/api/0/organizations/`, { method: "GET" }); + + expect(capturedHeaders.baggage).toBeUndefined(); + }); +});