From 74bff21fc5adf2630d6c660d89058ba888c7d683 Mon Sep 17 00:00:00 2001 From: Lucinda Zhou Date: Tue, 19 May 2026 17:35:22 -0400 Subject: [PATCH] [CRED-2625] Re-apply Authorization header redaction in debug logRequest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 6954a1cd5d2a3d2b9f9a8e3f4d5c6b7a8f9e0d1c (#4207), which itself reverted the original fix in #4168. The revert was prompted by a failing test on the auto-generated spec-sync PR #4156 (branch `datadog-api-spec/generated/5684`). At the time of that failure the branch contained the test from #4168 but not the production-code edit from #4168. That looked like the redaction was broken; it actually meant `isomorphic-fetch.ts` is template-managed. The template lives in a separate repo, `DataDog/datadog-api-client-generator`, under both: - `src/.../typescript/templates/common_package/http/isomorphic-fetch.ts.j2` - `src/.../typescript_unified/templates/http/isomorphic-fetch.ts.j2` The original PR #4168 edited the generated file but never touched the templates, so the next `api-clients-generation-pipeline[bot]` regen rewrote `isomorphic-fetch.ts` from template — silently deleting the redaction. The split-diff branch state (test present, production code reverted) then failed the redaction assertion, and the revert was applied as an unblock instead of root-causing the template gap. A companion PR against `datadog-api-client-generator` adds the Authorization redaction to both templates so this fix is durable. Once both land, the next regen reproduces this generated file unchanged. Restores: - `packages/datadog-api-client-common/http/isomorphic-fetch.ts`: adds Authorization to the `headersToRedact` list alongside DD-API-KEY / DD-APPLICATION-KEY, preserving the existing per-character `x` mask. - `tests/api/log-redaction.test.ts`: asserts the raw bearer token does not appear anywhere in the joined debug-log output and that Authorization is masked to an `x+` run. Tracking ticket: CRED-2625. Surfaced in terraform-provider-datadog#3757 (the first Terraform code path that sets ContextAccessToken). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../http/isomorphic-fetch.ts | 19 ++++---- tests/api/log-redaction.test.ts | 47 +++++++++++++++++++ 2 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 tests/api/log-redaction.test.ts diff --git a/packages/datadog-api-client-common/http/isomorphic-fetch.ts b/packages/datadog-api-client-common/http/isomorphic-fetch.ts index 67a1e3957a4a..f296b0b856d2 100644 --- a/packages/datadog-api-client-common/http/isomorphic-fetch.ts +++ b/packages/datadog-api-client-common/http/isomorphic-fetch.ts @@ -171,14 +171,17 @@ export class IsomorphicFetchHttpLibrary implements HttpLibrary { for (const header in originalHeaders) { headers[header] = originalHeaders[header]; } - if (headers["DD-API-KEY"]) { - headers["DD-API-KEY"] = headers["DD-API-KEY"].replace(/./g, "x"); - } - if (headers["DD-APPLICATION-KEY"]) { - headers["DD-APPLICATION-KEY"] = headers["DD-APPLICATION-KEY"].replace( - /./g, - "x" - ); + // Mask credential headers before serializing for the debug log. + // Authorization carries Bearer tokens (delegated tokens, PATs). See CRED-2625. + const headersToRedact = [ + "DD-API-KEY", + "DD-APPLICATION-KEY", + "Authorization", + ]; + for (const header of headersToRedact) { + if (headers[header]) { + headers[header] = headers[header].replace(/./g, "x"); + } } const headersJSON = JSON.stringify(headers, null, 2).replace(/\n/g, "\n\t"); diff --git a/tests/api/log-redaction.test.ts b/tests/api/log-redaction.test.ts new file mode 100644 index 000000000000..454d8eee5547 --- /dev/null +++ b/tests/api/log-redaction.test.ts @@ -0,0 +1,47 @@ +import { IsomorphicFetchHttpLibrary } from "../../packages/datadog-api-client-common/http/isomorphic-fetch"; +import { + HttpMethod, + RequestContext, +} from "../../packages/datadog-api-client-common/http/http"; +import { logger } from "../../logger"; + +// TestLogRequestRedactsAuthorization verifies that when debug logging is on, +// the IsomorphicFetchHttpLibrary masks the Authorization header (Bearer +// tokens — delegated tokens, PATs) before sending the request log line to +// the logger. Without this, callers running with debug logging and access- +// token auth leak the bearer to stderr / CI artifacts / log shippers. +// See CRED-2625. +test("logRequest masks Authorization Bearer token", () => { + const debugMessages: string[] = []; + const originalDebug = logger.debug.bind(logger); + logger.debug = (...args: unknown[]) => { + debugMessages.push(args.map(String).join("")); + }; + + try { + const http = new IsomorphicFetchHttpLibrary(); + http.debug = true; + + const ctx = new RequestContext("https://api.example.com/ping", HttpMethod.GET); + ctx.setHeaderParam("DD-API-KEY", "api-key-secret-value"); + ctx.setHeaderParam("DD-APPLICATION-KEY", "app-key-secret-value"); + ctx.setHeaderParam("Authorization", "Bearer ddpat_supersecret_should_not_leak"); + + // logRequest is private; exercise it indirectly by reaching through the + // class. Acceptable in a test file since the redaction surface is what + // we're asserting on, not the public send() pipeline. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (http as any).logRequest(ctx); + + const joined = debugMessages.join("\n"); + expect(joined).not.toContain("api-key-secret-value"); + expect(joined).not.toContain("app-key-secret-value"); + expect(joined).not.toContain("ddpat_supersecret_should_not_leak"); + // Each redacted value is replaced with `x` repeated to its original length. + expect(joined).toMatch(/DD-API-KEY":\s*"x+"/); + expect(joined).toMatch(/DD-APPLICATION-KEY":\s*"x+"/); + expect(joined).toMatch(/Authorization":\s*"x+"/); + } finally { + logger.debug = originalDebug; + } +});