fix(boto3): Fix botocore SigV4 failures caused by post-sign trace propagation by pabloDeputter · Pull Request #7050 · getsentry/sentry-python

pabloDeputter · 2026-08-05T14:51:07Z

Description

Summary of issue

pre-4.12.0 ddtrace basically behaved like Sentry before: propagation headers were added by the HTTP client after botocore already had calculated the SigV4 signature. So baggage was not included in SignedHeaders. Any later modifications to the value did not invalidate the request.
4.12.0 moves the header injection to botocore's before-sign event. It adds baggage, ... and x-datadog-* before signing. Any later modifications to the value DO invalidate the request, thus later HTTP-client injection is suppressed to avoid duplicate headers.
How failure happens:
1. incoming FastAPI request contains Sentry or other W3C baggage
2. datadog extracts that baggage into its active context
3. boto3 request is created
4. datadog’s before-sign handler writes the baggage to the AWS request
5. botocore includes baggage in the SigV4 signature
6. sentry’s httplib runs afterwards and updates or adds another baggage value
7. AWS receives a different baggage value from the one that was signed. Because request was "tampered" with after signing, AWS rejects the request with 403 Forbidden or SignatureDoesNotMatch.

Changes

inject sentry propagation through before-sign handler, so final baggage and sentry-trace values are created before SigV4 signing.
existing third-party baggage is merged into one header.
http.client propagation is delayed until endheaders(), when the complete request headers and SigV4 SignedHeaders are available. Existing baggage header is never mutated after it already was signed.
propagation is skipped for presigned requests, since adding headers could require a future caller to reproduce these exact headers.

Issues

Resolves: #7031 & #7031
Related issues in dd-trace-py: #19477 & #19358

Reminders

Please add tests to validate your changes, and lint your code using uv run ruff.
Add GH Issue ID & Linear ID (if applicable)
PR title should use conventional commit style (feat:, fix:, ref:, meta:)
For external contributors: CONTRIBUTING.md, Sentry SDK development docs, Discord community

- merge Sentry baggage with existing vendor (e.g. Datadog) baggage in botocore's`before-sign` hook; avoiding post-sign header tampering that invalidates the SigV4 signature. - Skip propagation for presigned requests Fixes: #7031 & PY-2667

- trace-header injection is delayed until `endheaders()` after all request headers are available. - avoid duplicate propagation headers. - no tampering/modifying of already signed baggage Fixes: #7031 & PY-2667

Refs: #7031 & PY-2667

# Refs: #7031 & PY-2667

Refs: #7031 & PY-2667

… SigV4 headers Refs: #7031 & PY-2667

alexander-alderman-webb · 2026-08-07T11:29:24Z

I haven't forgot about this, it's just complex so I'll likely only re-review fully at the start of next week.

…` + support for SigV4 query/presigned authentication Refs: #7031 & PY-2667

Refs: #7031 & PY-2667

…3.9 tests succeed Refs: #7031 & PY-2667

…ssues Refs: #7031 & PY-2667

alexander-alderman-webb

This looks really good broadly as fixes the issue without disrupting existing users (including those that not using boto3).

My comments are mainly concerned with the patching approach. It looks more complex than needed to achieve the result.

…` since it's not used anymore

…ion` string instead of all headers

- Record existing and signed headers in `putheader()` so trace propagation can avoid reparsing `_buffer` on every request. Refs: #7031 & PY-2667

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit 3ce7587. Configure here.}

alexander-alderman-webb

Next round 😃.
I've also pushed directly to your branch to limit the behavior to the affected HTTPConnection subclasses. I remove the changes to aiohttp as well, they can be in a separate PR when we get around to it (this one is big enough).

…aws_sigv4_signed_headers`

…ction`

sentry-warden · 2026-08-24T10:24:18Z

+        )
+
+
 def _install_httplib() -> None:


AWS connection wrapper bypasses stdlib putrequest instrumentation

_patch_aws_connection() runs before HTTPConnection.putrequest is replaced. Because the AWS connection classes inherit putrequest, their wrapper captures the original uninstrumented method and then shadows the later base-class patch. As a result, botocore requests using only StdlibIntegration do not execute the stdlib instrumentation or set _sentrysdk_trace_url, so the deferred endheaders() path never injects Sentry propagation headers. Requests with Boto3Integration may still receive headers from its before-sign handler, but lose stdlib-level instrumentation.

Evidence

_install_httplib() calls _patch_aws_connection() before assigning the instrumented HTTPConnection.putrequest.

_patch_aws_connection() assigns an AWS-class putrequest wrapper that captures the then-current inherited, uninstrumented method.

The AWS-class method shadows the later HTTPConnection.putrequest assignment, so AWS requests skip the stdlib span setup and never set _sentrysdk_trace_url.

_get_wrapped_endheaders() injects propagation only when _sentrysdk_trace_url is present; therefore the Stdlib-only botocore path omits Sentry headers, as exercised by test_botocore_without_boto3_integration_preserves_signed_baggage.

_{Identified by Warden · code-review, find-bugs · 8UW-Y37}

no they call the superclass

From looking at the botocore code, I think this is legit. Here's a sample script of what it's describing. If you reverse the order of the Sub/Base patches, you'll see that when Sub is called that it also calls the patch on Base as well (right now it doesn't).

Edit: the solution would be to move the _patch_aws_connection call to below the HTTPConnection.* patches

demo_shadowing_bug.py

ericapisani

Still working through the changes but have some initial comments

ericapisani · 2026-08-24T18:13:36Z

+            return
+
+        def _replace_header(request: "AWSRequest", key: str, value: str) -> None:
+            # HTTPHeaders appends on assignment, so delete existing values first.


Because there's subtly different ways that headers can be handled when multiple values are set on the same header key, I think we should add a bit more context to this comment.

It'd be worth clarifying what "append" (ideally with a concrete example) means as it could be interpreted as:

adding to a list (e.g: [foo, bar, baz])

appending to a comma separated string (e.g. foo, bar, baz)

And I think it may also be worth pointing out that this is coming from email.message.Message from the standard library as this is what the botocore package is using under the hood.

Updated the comment to clarify that it creates another header field rather than extending, @alexander-alderman-webb was also confused about this 😄

ericapisani · 2026-08-24T18:31:47Z

    "build": sys.version,
 }

+_SENTRY_HEADER_NAMES = frozenset((BAGGAGE_HEADER_NAME, "sentry-trace"))


We also have a constant for sentry-trace that you can use here

Suggested change

_SENTRY_HEADER_NAMES = frozenset((BAGGAGE_HEADER_NAME, "sentry-trace"))

_SENTRY_HEADER_NAMES = frozenset((BAGGAGE_HEADER_NAME, SENTRY_TRACE_HEADER_NAME))

Yup good idea, didn't know this existed

ericapisani · 2026-08-24T18:45:38Z

+    original_putheader: "Callable[..., Any]",
+) -> "Callable[..., Any]":
+    """
+    Responsible for tracking request and signed headers.


Similar to the other comment, I think we should point future devs reading this to the specific "signed headers" that are being referred to here (in this case botocore's SignedHeaders).

Suggested change

Responsible for tracking request and signed headers.

Responsible for tracking request and `SignedHeaders`.

Yup, changed it to "Responsible for tracking which sentry headers are present and whether they are listed in AWS SigV4 SignedHeaders."

ericapisani · 2026-08-24T19:10:37Z

    )


+def _get_aws_sigv4_signed_headers(


Assuming I'm understanding this correctly, we've got 2 distinct code paths here:

searching for the signed headers in the Authorization header (lines 1705-1716)

searching for the signed headers in the URL's query string (lines 1718-1734)

If this were a public API and we wanted to optimize for user convenience, I can see the value of supporting the passing in both authorization and url and then searching in the correct place based on if the value is present or not, but because this is strictly for internal use, I think it'd be cleaner to have two distinct helper functions for each path:

_get_aws_sigv4_signed_headers_from_authorization_header

_get_aws_sigv4_signed_headers_from_url_query_string

since at the calling sites, we already know where these headers are.

agreed, I left it like that because originally we didn't read from the query string. I split it into 2 separate functions.

ericapisani · 2026-08-24T19:18:24Z

+    if authorization is not None:
+        # only AWS SigV4 authorization has the SignedHeaders parameter.
+        value = authorization.lstrip()
+        if value.startswith(("AWS4-HMAC-SHA256", "AWS4-ECDSA-P256-SHA256")):


Although these aren't used very widely in the broader SDK codebase, what are your thoughts on doing something similar to the _SENTRY_HEADER_NAMES constant with the SigV4 algorithms?

_AWS_SIGV4_SIGNING_ALGORITHMS = frozenset(("AWS4-HMAC-SHA256", "AWS4-ECDSA-P256-SHA256"))

I think that's okay. I'm doing it in stdlib.py with _SENTRY_HEADER_NAMES as well. Not sure whether we have a separate file for such constants, but I put it at the top of utils.py.

ericapisani · 2026-08-24T19:33:02Z

+            with capture_internal_exceptions():
+                authorization = values[0]
+                if isinstance(authorization, bytes):
+                    authorization = authorization.decode("ascii", "ignore")


To mirror the codecs used in the putheader method - it looks like we want latin-1 decoding instead of ascii here.

Suggested change

authorization = authorization.decode("ascii", "ignore")

authorization = authorization.decode("latin-1", "ignore")

ericapisani

I'm going to hold off on reviewing the tests for now until we've addressed all the questions around the core logic.

Overall lots of great stuff here - I particularly appreciate the docstrings/comments explaining what's happening.

Don't hesitate to reach out if there's any notes that I've left here that you want to chat about! 😄

ericapisani · 2026-08-25T12:54:50Z

+    """
+
+    def endheaders(self: "HTTPConnection", *args: "Any", **kwargs: "Any") -> "Any":
+        real_url = getattr(self, "_sentrysdk_trace_url", None)


Does real_url represent the original URL in the request? Or should this instead be called trace_url?

yes, it represents the whole URL, but its mainly used for trace propagation. _sentrysdk_real_url is better though.

ericapisani · 2026-08-25T13:02:49Z

+                        self, "_sentrysdk_request_headers", {}
+                    )
+                    if request_headers is not None:


If _sentrysdk_request_headers is not set on self and the empty dict default is returned, the if request_headers is not None conditional will still pass.

If this is the intent, it'd be clearer to have the default value for getattr be None as the way that this is currently written, this looks like a potential bug 😅

ericapisani · 2026-08-25T13:29:31Z

+                        for (
+                            signed_header
+                        ) in _get_aws_sigv4_signed_headers_from_url_query_string(
+                            real_url
+                        ):
+                            if signed_header in _SENTRY_HEADER_NAMES:
+                                is_present, _ = request_headers.get(
+                                    signed_header, (False, False)
+                                )
+                                request_headers[signed_header] = (is_present, True)


I'm finding this block of code (and the related one further up that deals with the authorization header) a bit confusing.

In what circumstances would the signed_header not be found in request_headers but is present on the query string, such that is_present would be False (lines 160-62)?

Related to that - do we need the request_headers.get() part that sets is_present? If the signed header is in the query string, would that not mean that, at this point in the code, is_present is always true?

ericapisani · 2026-08-25T13:34:28Z

+                                normalized_header, (False, False)
+                            )
+                            if is_signed or (
+                                is_present and normalized_header != BAGGAGE_HEADER_NAME


If we're looking to avoid a duplicate sentry-trace (the comment on line 172), should BAGGAGE_HEADER_NAME instead be SENTRY_TRACE_HEADER_NAME? Do we potentially need to avoid duplicating both of these headers?

ericapisani · 2026-08-25T13:37:32Z

+        # track which propagation headers are present and signed e.g. {"sentry-trace": (is_present, is_signed)}
+        request_headers: "Optional[Dict[str, Tuple[bool, bool]]]" = {}
+        self._sentrysdk_request_headers = request_headers  # type: ignore[attr-defined]


Was there code that was previously here that used request_headers? This looks like we could remove request_headers and assign self._sentrysdk_request_headers to an empty dict instead

Suggested change

# track which propagation headers are present and signed e.g. {"sentry-trace": (is_present, is_signed)}

request_headers: "Optional[Dict[str, Tuple[bool, bool]]]" = {}

self._sentrysdk_request_headers = request_headers # type: ignore[attr-defined]

self._sentrysdk_request_headers = {}

I was trying to fix a mypy error, but just did type: ignore[attr-defined] in the end and forgot to remove that part.

This comment was marked as outdated.

Sign in to view

pabloDeputter added 5 commits August 5, 2026 17:46

fix(stdlib): Preserve signed headers during trace propagation

78f1ee6

- trace-header injection is delayed until `endheaders()` after all request headers are available. - avoid duplicate propagation headers. - no tampering/modifying of already signed baggage Fixes: #7031 & PY-2667

test(boto3): add tests covering trace propagation + SigV4 signing

88fdd7d

Refs: #7031 & PY-2667

test(boto3): inline span_streaming logic

a1e5f9c

Refs: #7031 & PY-2667

test(stdlib): cover httplib SigV4 trace header preservation

4ba8901

Refs: #7031 & PY-2667

test(boto3): Add server_close() in test cleanup

92672ca

# Refs: #7031 & PY-2667

pabloDeputter marked this pull request as ready for review August 6, 2026 11:12

pabloDeputter requested a review from a team as a code owner August 6, 2026 11:12

sentry Bot reviewed Aug 6, 2026

View reviewed changes

Comment thread sentry_sdk/integrations/stdlib.py Outdated

alexander-alderman-webb reviewed Aug 6, 2026

View reviewed changes

Comment thread sentry_sdk/integrations/stdlib.py Outdated

pabloDeputter added 2 commits August 6, 2026 14:41

fix(stdlib): clarify comment

37fa9ff

Refs: #7031 & PY-2667

fix(stdlib): Move trace header generation into endheaders()

e675673

Refs: #7031 & PY-2667

sentry-warden Bot reviewed Aug 6, 2026

View reviewed changes

Comment thread sentry_sdk/integrations/stdlib.py Outdated

fix(stdlib): Replace return set() with continue to avoid skipping…

14bad29

… SigV4 headers Refs: #7031 & PY-2667

sentry-warden Bot reviewed Aug 6, 2026

View reviewed changes

Comment thread sentry_sdk/integrations/stdlib.py Outdated

fix(stdlib): Move get_aws_sigv4_signed_headers to `sentry_sdk.utils…

90a77d6

…` + support for SigV4 query/presigned authentication Refs: #7031 & PY-2667

sentry Bot reviewed Aug 11, 2026

View reviewed changes

Comment thread sentry_sdk/integrations/stdlib.py Outdated

pabloDeputter added 8 commits August 11, 2026 13:16

fix(stdlib): Make mypy happy

f2d0cee

Refs: #7031 & PY-2667

fix(stdlib): Capture internal exceptions in endheaders()

e6e9e5e

Refs: #7031 & PY-2667

test(utils): Add test for get_aws_sigv4_signed_headers

3f4dacc

Refs: #7031 & PY-2667

fix(aiohttp): Preserve SigV4 signed headers

6d7301c

Refs: #7031 & PY-2667

test(aiohttp): Add tests related to SigV4 issues

a0c1ad2

Refs: #7031 & PY-2667

test(boto3): Add signed header assertions to tests

e40e931

Refs: #7031 & PY-2667

test(httplib): Add tests for SigV4 signed headers

af905db

Refs: #7031 & PY-2667

fix(aiohttp): Do request.rel_url instead of request.url so py3.7-…

09f8c44

…3.9 tests succeed Refs: #7031 & PY-2667

sentry-warden Bot reviewed Aug 11, 2026

View reviewed changes

Comment thread tests/integrations/boto3/test_trace_propagation.py

fix(aiohttp): Use raw_path instead of rel_url to avoid encoding i…

e07978e

…ssues Refs: #7031 & PY-2667

alexander-alderman-webb reviewed Aug 12, 2026

View reviewed changes

Comment thread sentry_sdk/integrations/stdlib.py Outdated

Comment thread sentry_sdk/integrations/stdlib.py Outdated

alexander-alderman-webb reviewed Aug 12, 2026

View reviewed changes

Comment thread sentry_sdk/integrations/boto3.py

pabloDeputter added 5 commits August 12, 2026 11:12

ref(boto3): Clarify logic in replace_header()

08dc4d2

ref(utils): Remove bytes parsing from `get_aws_sigv4_signed_headers()…

079cf17

…` since it's not used anymore

ref(utils): get_aws_sigv4_signed_headers() now takes in `authorizat…

ebcce02

…ion` string instead of all headers

chore(stdlib): Track headers during request construction

484a941

- Record existing and signed headers in `putheader()` so trace propagation can avoid reparsing `_buffer` on every request. Refs: #7031 & PY-2667

linting

35ff7b4

cursor Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread sentry_sdk/integrations/stdlib.py Outdated

pabloDeputter and others added 8 commits August 12, 2026 13:57

fix(stdlib): Fix renaming issue

355be07

ref(utils): Remove unused Mapping import

5ab6ccb

ref(stdlib): Fix another typo

f04bd51

revert aiohttp changes

28e33cc

limit changes to botocore connection classes

6ff70c4

merge master

8003db0

formatting

b68e9f7

make mypy happy

3ce7587

cursor Bot reviewed Aug 24, 2026

View reviewed changes

Comment thread sentry_sdk/integrations/stdlib.py

Comment thread sentry_sdk/integrations/stdlib.py

alexander-alderman-webb reviewed Aug 24, 2026

View reviewed changes

Comment thread sentry_sdk/utils.py Outdated

Comment thread tests/test_utils.py Outdated

Comment thread tests/integrations/stdlib/test_httplib.py Outdated

Comment thread sentry_sdk/integrations/stdlib.py Outdated

alexander-alderman-webb and others added 3 commits August 24, 2026 10:48

clean up trace_url on endheaders

628543e

(ref): Make get_aws_sigv4_signed_headers private; renamed to `_get_…

a739e84

…aws_sigv4_signed_headers`

(tests): Move tests using AWSHTTPConnection to `test_aws_http_conne…

3d44029

…ction`

sentry-warden Bot reviewed Aug 24, 2026

View reviewed changes

Comment thread sentry_sdk/integrations/boto3.py

pabloDeputter added 2 commits August 24, 2026 13:34

remove tracking of uncessary headers

c706b7b

ruff fix

1ee3b58

sentry-warden Bot reviewed Aug 24, 2026

View reviewed changes

Comment thread sentry_sdk/integrations/stdlib.py Outdated

ericapisani reviewed Aug 24, 2026

View reviewed changes

resolve PR comments

8762b88

ericapisani reviewed Aug 25, 2026

View reviewed changes

	_SENTRY_HEADER_NAMES = frozenset((BAGGAGE_HEADER_NAME, "sentry-trace"))
	_SENTRY_HEADER_NAMES = frozenset((BAGGAGE_HEADER_NAME, SENTRY_TRACE_HEADER_NAME))

	Responsible for tracking request and signed headers.
	Responsible for tracking request and `SignedHeaders`.

	authorization = authorization.decode("ascii", "ignore")
	authorization = authorization.decode("latin-1", "ignore")

Uh oh!

Conversation

pabloDeputter commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Summary of issue

Changes

Issues

Reminders

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexander-alderman-webb commented Aug 7, 2026

Uh oh!

Uh oh!

Uh oh!

alexander-alderman-webb left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alexander-alderman-webb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sentry-warden Bot Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericapisani Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ericapisani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericapisani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

pabloDeputter commented Aug 5, 2026 •

edited

Loading

alexander-alderman-webb left a comment •

edited

Loading

sentry-warden Bot Aug 24, 2026 •

edited

Loading

ericapisani Aug 25, 2026 •

edited

Loading