Skip to content

[API] Isolate ambient baggage updates across execution contexts - #8042

Open
Kielek wants to merge 11 commits into
open-telemetry:mainfrom
Kielek:codex/ant-2026-yxedg1g8-baggage-isolation
Open

Kielek wants to merge 11 commits into
open-telemetry:mainfrom
Kielek:codex/ant-2026-yxedg1g8-baggage-isolation

Conversation

@Kielek

@Kielek Kielek commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

This fix addresses a ANT-2026-YXEDG1G8 finding from Anthropic's OSS Scanner.
Project enrollment: anthropics/oss-scanner#117.

Fixes #7449, it is 3rd attempt to solve this. This time, with AppContext opt-out switch.
Is is similar to was done directly in .NET for https://learn.microsoft.com/en-us/dotnet/core/compatibility/serialization/10/xmlserializer-obsolete-properties

Changes

Ambient baggage writes now publish a replacement holder in the current execution
context, so they no longer change baggage observed by previously forked contexts.
The correction covers Baggage.Current and every static baggage mutator, with
both explicit and omitted baggage arguments. It addresses the behavior tracked
in #7449.

This is a breaking behavior change: enrichment performed in an awaited
asynchronous helper no longer flows back to its caller. Enable the
OpenTelemetry.Baggage.AllowUnsafeLegacyFlow AppContext switch at startup to
temporarily restore the original shared holder and locking behavior. An unset
or false switch keeps isolation. The selected mode is cached when Baggage
initializes; later configuration changes do not update it.
The API documentation explains the unsafe sharing risk and shows how to migrate
by returning updated baggage to the caller. No public signatures or dependencies
change.

The compatibility option is a migration aid, not a claim of backward compatibility
or an approved 1.x release exception. The history in #5208, #5227, and #7191.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, successful build/tests, etc.)
  • Tests added/updated
  • CHANGELOG.md file(s) updated for non-trivial changes
  • Changes in public API reviewed (if applicable)
  • AI usage disclosure:
    • No AI used
    • AI-assisted
    • Bulk AI-generated
  • I have the experience and knowledge necessary to understand, review, and validate all content in this PR1

Note

This PR description was generated with OpenAI Codex.

Footnotes

  1. Yes, I can answer maintainer questions about the content of this PR without using AI. ↩

Scope baggage replacement and every static mutation to the current execution
context by default. Add a startup AppContext switch to restore unsafe legacy
sharing temporarily, with migration guidance and regression coverage for both
modes and cached configuration. The default behavior change still requires
maintainer release approval.

Discovered by Anthropic's OSS Scanner, as bug ANT-2026-YXEDG1G8.
@github-actions github-actions Bot added the pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package label Oct 26, 2026
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Pull request dashboard status

Waiting on the author · refreshed 2026-11-10 13:04 UTC

Respond to 2 review items (e.g. link a commit, explain why not, ask a follow-up):

  • Top-level threads: 1, 2
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.
  • Should this be with reviewers? Comment /dashboard route:reviewers to route it to them.
  • Anything wrong — including the routing? Report it with what you expected; it helps us improve the dashboard.

@codecov

codecov Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 53.33333% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.01%. Comparing base (f7eaaab) to head (c15cf9c).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/OpenTelemetry.Api/Baggage.cs 53.33% 2 Missing and 5 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8042      +/-   ##
==========================================
- Coverage   93.03%   93.01%   -0.03%     
==========================================
  Files         398      400       +2     
  Lines       23473    23734     +261     
  Branches     4389     4439      +50     
==========================================
+ Hits        21838    22075     +237     
- Misses       1115     1125      +10     
- Partials      520      534      +14     
Flag Coverage Δ
unittests-Project-Experimental 92.98% <53.33%> (-0.15%) ⬇️
unittests-Project-Stable 92.97% <53.33%> (-0.16%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/OpenTelemetry.Api/Baggage.cs 92.20% <53.33%> (-4.25%) ⬇️

... and 9 files with indirect coverage changes

Use the existing TemporaryDirectory helper for isolated startup-mode test
directories and link its source into the API test project.

Discovered by Anthropic's OSS Scanner, as bug ANT-2026-YXEDG1G8.
@Kielek
Kielek marked this pull request as ready for review October 27, 2026 10:21
@Kielek
Kielek requested a review from a team as a code owner October 27, 2026 10:21
Remove startup-mode and caching tests and their unused helper include.
Keep the baggage regressions focused on the safe default behavior.

Discovered by Anthropic's OSS Scanner, as bug ANT-2026-YXEDG1G8.
Comment thread src/OpenTelemetry.Api/README.md Resolved
Comment thread src/OpenTelemetry.Api/README.md Resolved
Comment thread src/OpenTelemetry.Api/README.md Resolved
Link the AppContext documentation, add the ASP.NET Web.config example,
and clarify the warning about insecure legacy baggage flow.

Discovered by Anthropic's OSS Scanner, as bug ANT-2026-YXEDG1G8.

Co-authored-by: Martin Costello <martin@martincostello.com>
@Kielek Kielek added the review-priority Should be prioritized for review label Oct 27, 2026
@martincostello

Copy link
Copy Markdown
Member

Waiting on anything else to merge this?

@Kielek

Kielek commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

I would like to have opinion from @alanwest or @rajkumar-rangaraj here. I hope they will find some time to review.

@rajkumar-rangaraj rajkumar-rangaraj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM on the direction and the implementation. Isolating writes matches the spec's immutable-context model, and the opt-out switch gives users a migration path. A few things before merging:

Breaking default: this is essentially #5208, which was reverted in #5227 after user feedback that baggage set in awaited helpers no longer flowed back to the caller. Since this changes stable OpenTelemetry.Api 1.x behavior, can we confirm it at the SIG and call it out prominently in the release notes?

Timing: 1.21.0-rc.2 shipped today. I'd prefer this not go straight into 1.21.0 stable without an RC. Either cut another RC or merge after 1.21.0.

Contrib impact: ASP.NET TelemetryHttpModule saves the baggage holder object at request start and puts it back when the async context is lost (ActivityHelper.cs). With this change that restore rolls baggage back to its request-start value. If there was no incoming baggage, nothing is restored. I'll open a contrib follow-up so it saves the current Baggage value instead.

Nit: the legacy-switch path no longer has tests. That's fine since the code is unchanged, but worth noting.

@Kielek Kielek added the keep-open Prevents issues and pull requests being closed as stale label Nov 10, 2026
@Kielek

Kielek commented Nov 10, 2026 •

Copy link
Copy Markdown
Member Author

Contrib impact: ASP.NET TelemetryHttpModule saves the baggage holder object at request start and puts it back when the async context is lost (ActivityHelper.cs). With this change that restore rolls baggage back to its request-start value. If there was no incoming baggage, nothing is restored. I'll open a contrib follow-up so it saves the current Baggage value instead.

PR open-telemetry/opentelemetry-dotnet-contrib#5948 is ready. It can be merged even now, to avoid problems if anybody update only API and forgot to update instrumentation package.

Nit: the legacy-switch path no longer has tests. That's fine since the code is unchanged, but worth noting.

Intentionally omited.

@Kielek

Kielek commented Nov 10, 2026 •

Copy link
Copy Markdown
Member Author

Timing: 1.21.0-rc.2 shipped today. I'd prefer this not go straight into 1.21.0 stable without an RC. Either cut another RC or merge after 1.21.0.

I hope it will work for you:

  1. Stable release with .NET11
  2. Merge this PR.
  3. RC.1 release with the fix couple days later. Hopefully next week, then we can include it into stable in the next+1 version.

@martincostello martincostello added this to the v1.22.0 milestone Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

keep-open Prevents issues and pull requests being closed as stale pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package review-priority Should be prioritized for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] Baggage.Current can leak across async flows

4 participants