[Cosmos] Harden diagnostics logging redaction (Set-Cookie/auth-challenge headers + error-path URL) - #48827
Open
Anna Tchijova (annatchijova) wants to merge 2 commits into
Conversation
…rs + error-path URL) CosmosHttpLoggingPolicy redacted only a 3-entry denylist (Authorization, ProxyAuthorization, TransferEncoding), logging every other header in the clear when diagnostics logging is enabled. That re-exposes headers azure-core's HttpLoggingPolicy redacts by default -- notably Set-Cookie, Proxy-Authenticate, and WWW-Authenticate, which can carry session material. Separately, the success path (on_request) logged a query-redacted URL, but the error and exception paths (_log_diagnostics_error, on_exception, _populate_logger_attributes) logged the raw URL, so query-parameter values could be logged. - Add SetCookie, ProxyAuthenticate, WwwAuthenticate to _cosmos_disallow_list. - Add a _redact_url helper and apply it on the error/exception logging paths so the URL is redacted consistently with on_request. Deny-by-list logging remains fail-open by design; the comment now states that any newly added sensitive header must be added to the denylist. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VWmkpCnuhCPKFPD2fexY1a
github-actions
Bot
added
Community Contribution
Copilot started reviewing on behalf of
Anna Tchijova (annatchijova)
September 1, 2026 14:48
View session
Contributor
|
Thank you for your contribution Anna Tchijova (@annatchijova)! We will review the pull request and get back to you soon. |
|
Azure Pipelines: Successfully started running 2 pipeline(s). 6 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens Cosmos diagnostics logging against sensitive header and URL query-value exposure.
Changes:
- Expands sensitive-header redaction.
- Adds reusable URL redaction across error paths.
- Adds regression tests and changelog documentation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
CHANGELOG.md |
Documents logging hardening. |
_cosmos_http_logging_policy.py |
Redacts URLs in several error paths. |
http_constants.py |
Extends the header denylist. |
test_cosmos_log_redaction.py |
Adds redaction regression tests. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| logger_attributes["activity_id"] = http_request.headers.get(HttpHeaders.ActivityId, "") | ||
| logger_attributes["verb"] = http_request.method | ||
| logger_attributes["url"] = http_request.url | ||
| logger_attributes["url"] = _redact_url(http_request.url) |
Comment on lines
+499
to
+502
| # This includes credential/authorization headers and headers that can carry session material | ||
| # (cookies, authentication challenges), matching the headers azure-core's HttpLoggingPolicy | ||
| # redacts by default. Redaction here is deny-by-list, so any newly added sensitive header must be | ||
| # added below to avoid being logged in the clear. |
Comment on lines
+17
to
+18
| # These are redacted by azure-core's HttpLoggingPolicy by default and must | ||
| # not be logged by the Cosmos policy either. |
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
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.
What
CosmosHttpLoggingPolicyredacted only a 3-entry denylist (Authorization,ProxyAuthorization,TransferEncoding), logging every other header in the clear when diagnostics logging (enable_diagnostics_logging=True) is on. That re-exposes headers azure-core'sHttpLoggingPolicyredacts by default — notablySet-Cookie,Proxy-Authenticate, andWWW-Authenticate, which can carry session material.Separately, the success path (
on_request) logged a query-redacted URL, but the error/exception paths (_log_diagnostics_error,on_exception,_populate_logger_attributes) logged the raw URL, so query-parameter values could be logged.Changes
SetCookie,ProxyAuthenticate,WwwAuthenticateto_cosmos_disallow_list(http_constants.py), and note in the comment that deny-by-list is fail-open, so any new sensitive header must be added there._redact_urlhelper (_cosmos_http_logging_policy.py) and apply it on the error/exception logging paths, so the URL is redacted consistently withon_request.Scope / severity (stated honestly)
Defense-in-depth for diagnostics logging. Credential headers (
Authorizationfor both the master-key HMAC signature and AAD bearer,ProxyAuthorization) were already redacted and remain so; this closes the leakage of cookies / auth-challenge headers and query-parameter values that azure-core redacts by default. Requires opt-in diagnostics logging plus access to the logs; not remotely exploitable.The deny-by-list design is fail-open by nature (a newly introduced sensitive header would be logged until added to the list). This PR keeps the existing intent (the SDK deliberately logs
x-ms-*diagnostics headers) and makes the minimal, mergeable change; a maintainer wanting a stronger posture could switch to an allowlist, which the added comment flags.