[CRED-2625] Re-apply Authorization header redaction in debug logRequest#4208
Closed
luczhou wants to merge 1 commit into
Closed
[CRED-2625] Re-apply Authorization header redaction in debug logRequest#4208luczhou wants to merge 1 commit into
luczhou wants to merge 1 commit into
Conversation
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Re-applies the fix from #4168 that was reverted by #4207. The original revert was a misdirected unblock: the test was failing on a separate, regen-only branch where the production-code edit had been silently stripped out by the spec-sync regen, while the new test file persisted. That looked like a broken fix; it was actually a template-vs-generated-file mismatch.
Investigation summary:
test (16, ubuntu-latest)andtest (18, ubuntu-latest). The fix worked.datadog-api-spec/generated/5684) ran tests at commit659162b798e7. Diff of that commit vs its branch base (e4ed866f0f80, which already contained [CRED-2625] Redact Authorization (Bearer) header in debug logRequest #4168) showspackages/datadog-api-client-common/http/isomorphic-fetch.tswith+8/-11— the exact inverse of [CRED-2625] Redact Authorization (Bearer) header in debug logRequest #4168's+11/-8. The regen reverted that file from template.tests/api/log-redaction.test.ts(a hand-written test, untouched by regen). With the production code reverted and the test still present, the test correctly fails — but the failure isn't a "broken fix", it's a "missing template change".isomorphic-fetch.tsis not regenerated from the in-repo.generator/directory (which only holdsexample.j2); it's regenerated from templates in the separate private repoDataDog/datadog-api-client-generator, by theapi-clients-generation-pipeline[bot]GitHub App. Bothsrc/.../typescript/templates/common_package/http/isomorphic-fetch.ts.j2andsrc/.../typescript_unified/templates/http/isomorphic-fetch.ts.j2had only the DD-API-KEY / DD-APPLICATION-KEY redaction, never Authorization.A companion patch against
datadog-api-client-generatorupdates both templates so this fix is durable. Once that lands, the next regen reproduces the generated file in this PR byte-for-byte.Changes
packages/datadog-api-client-common/http/isomorphic-fetch.ts: restores theheadersToRedactarray (DD-API-KEY, DD-APPLICATION-KEY, Authorization) and the loop that masks each value withxrepeated to its original length.tests/api/log-redaction.test.ts: restored. DriveslogRequestdirectly with a capturedlogger.debug, asserts the raw bearer string does not appear in the joined output, and asserts Authorization is masked to anx+run.Cross-language status
datadog-api-client-goAdd OpenAPI spec for dataset restrictions endpoints #4098: merged. Templates and generated file both updated in one PR (templates are local to the SDK repo there, so the issue did not arise).datadog-api-client-rubyUpdate permissions in docs api for apm retention filters #3340: merged. Same — local templates updated alongside the generated.rb.datadog-api-client-java: still has no header redaction inLoggingFeature.PAYLOAD_ANY; tracked separately.datadog-api-client-python: no equivalent debug code path; no fix needed.Testing
git revertof Revert "[CRED-2625] Redact Authorization (Bearer) header in debug logRequest" #4207, so the diff vsmasteris identical to [CRED-2625] Redact Authorization (Bearer) header in debug logRequest #4168, which passed CI when originally merged.test (16),test (18)) before merge.datadog-api-client-generatorPR should land first (or together). Without it, the next spec-sync regen will strip these changes again and we will repeat the loop.Tracking ticket: CRED-2625. Surfaced in terraform-provider-datadog#3757.
🤖 Generated with Claude Code