Skip to content

opentelemetry-util-http: add redact_query_string and apply it in the wsgi instrumentation - #4944

Open
henry3260 wants to merge 2 commits into
open-telemetry:mainfrom
henry3260:wsgi-redact-query-string
Open

opentelemetry-util-http: add redact_query_string and apply it in the wsgi instrumentation#4944
henry3260 wants to merge 2 commits into
open-telemetry:mainfrom
henry3260:wsgi-redact-query-string

Conversation

@henry3260

@henry3260 henry3260 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Description

opentelemetry-instrumentation-wsgi writes the raw query string into http.target and url.query without redaction, so presigned-URL credentials such as Signature and AWSAccessKeyId reach the trace backend. redact_url is only called on the else branch of collect_request_attributes, which is taken when neither RAW_URI nor REQUEST_URI is set. gunicorn sets RAW_URI and uWSGI sets REQUEST_URI, so in production that branch is never reached; the werkzeug development server is what exercises it, which is why the gap does not show up locally.

The semantic conventions say query string values for AWSAccessKeyId, Signature, sig and X-Goog-Signature SHOULD be redacted by default and replaced by REDACTED, with case-sensitive matching. tornado and aiohttp-server already redact before calling _set_http_target; this brings wsgi in line.

The existing redact_query_parameters is not usable here: it calls urlparse on what is an attacker-controlled target, returning it unchanged when parsing fails, and its parse_qs/urlencode round trip drops valueless parameters and rewrites %20 as +. This PR adds redact_query_string to opentelemetry-util-http, which takes a bare query string, splits on & and = without parsing a URL, percent-decodes parameter names before matching (so %53ignature is redacted like Signature), and leaves everything it does not match byte for byte. wsgi redacts the target's own query and QUERY_STRING separately, since PEP 3333 does not require a server to supply both. url.path still comes from PATH_INFO and is unchanged.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • uv run tox -e py312-test-util-http — 74 passed. 15 new tests for redact_query_string covering each parameter in PARAMS_TO_REDACT, multiple matches in one query, case-sensitive matching, percent-encoded parameter names (%53ignature, %53%69gnature), parity with redact_query_parameters on a percent-encoded name, repeated parameters, valueless and blank parameters, a value containing [ that would break urlparse, and byte-for-byte round-tripping when nothing matches.
  • uv run tox -e py312-test-instrumentation-wsgi — 61 passed. 9 new tests covering default, http and http/dup semconv modes, RAW_URI and REQUEST_URI as the target source, a target that is not a parseable URL, an environ with no QUERY_STRING, a percent-encoded parameter name, url.path still reporting the PATH_INFO value, and a query with no sensitive parameters round-tripping unchanged. Each redaction test also asserts that no secret value appears in any attribute of the returned dict.

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@henry3260
henry3260 requested a review from a team as a code owner August 10, 2026 17:53
@henry3260
henry3260 force-pushed the wsgi-redact-query-string branch from 8d760d4 to 2d38b63 Compare August 10, 2026 18:46
…wsgi instrumentation so sensitive query parameter values do not leak through http.target and url.query
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Aug 14, 2026

Copy link
Copy Markdown

Pull request dashboard status

Waiting on reviewers · refreshed 2026-08-21 22:20 UTC

Review the latest changes.

Status above doesn't look right?
  • Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

@xrmx
xrmx requested a balanced review from Copilot August 18, 2026 13:42
@xrmx xrmx moved this to Ready for review in Python PR digest Aug 18, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Adds safer query-string redaction to prevent presigned-URL credentials from leaking into WSGI span attributes (http.target / url.query), aligning WSGI instrumentation with other server instrumentations and semantic conventions.

Changes:

  • Introduces redact_query_string() in opentelemetry-util-http to redact sensitive keys while preserving non-redacted bytes verbatim.
  • Applies query-value redaction to raw WSGI targets (RAW_URI / REQUEST_URI) and to QUERY_STRING before _set_http_target.
  • Adds unit tests for both the new utility function and WSGI attribute collection behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
util/opentelemetry-util-http/src/opentelemetry/util/http/init.py Adds redact_query_string() and supporting import for safe redaction without URL parsing.
util/opentelemetry-util-http/tests/test_redact_query_string.py Adds direct unit coverage for edge cases and byte-preserving behavior of new redaction helper.
instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/init.py Redacts sensitive query values in request target and QUERY_STRING to prevent attribute leaks.
instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py Adds tests ensuring WSGI does not leak secrets across semconv modes and target sources.
.changelog/4944.fixed Notes the WSGI leak fix in changelog.
.changelog/4944.added Notes the new util helper in changelog.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.


def _setup_sensitive_request(self, target_key="RAW_URI"):
self.environ["PATH_INFO"] = "/download"
self.environ["QUERY_STRING"] = self._SENSITIVE_QUERY
…h redaction test exercises the target source it names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready for review

Development

Successfully merging this pull request may close these issues.

3 participants