fix: Enforce baggage limits on the Jaeger extract path - #2299
Open
serhiy-bzhezytskyy wants to merge 3 commits into
Open
fix: Enforce baggage limits on the Jaeger extract path#2299serhiy-bzhezytskyy wants to merge 3 commits into
serhiy-bzhezytskyy wants to merge 3 commits into
Conversation
The Jaeger propagator extracts every uberctx- header into baggage with no bound. The Jaeger format defines no limits, so borrow the W3C Baggage spec limits (180 entries / 4096 bytes per entry / 8192 bytes total, byte-denominated): over-limit members are dropped once a limit is reached and the earlier ones are kept. opentelemetry-java caps its Jaeger propagator the same way, for the same reason (CVE-2026-45292). Adds four extract tests (entry count, per-entry bytes, byte-vs-char, total bytes); each fails without the change. Assisted-By: Claude Fable 5
serhiy-bzhezytskyy
requested review from
a team,
dazuma,
fbogsany,
kaylareopelle,
mwear,
robbkidd,
robertlaurin and
xuan-cao-swi
as code owners
August 18, 2026 13:30
simi
approved these changes
Aug 18, 2026
simi
left a comment
simi
left a comment
Contributor
There was a problem hiding this comment.
not sure we need all those comments
Keep only the byte-vs-character note, which is not obvious from the code; the rationale for the limit values lives in the PR description. Assisted-By: Claude Fable 5
Author
|
Thanks — dropped the explanatory comments and kept one line where the byte-vs-character distinction is not obvious from the code. @simi |
| baggage_key = carrier_key.start_with?(BAGGAGE_KEY_PREFIX) && carrier_key[BAGGAGE_KEY_PREFIX.length..] | ||
| next unless baggage_key | ||
|
|
||
| raw_value = getter.get(carrier, carrier_key) |
Contributor
There was a problem hiding this comment.
Maybe add next unless raw_value to ignore the case when raw_value=nil?
This can also prevent the undefined method 'bytesize' issue when raw_value is nil
Author
There was a problem hiding this comment.
Thanks, you're right. Fixed and covered it with a test.
Rubocop then hit 8/7 on Metrics/CyclomaticComplexity and 101/100 on Metrics/ClassLength, so the limit check moved into within_baggage_limits? and the private_constant list is reflowed. bundle exec rake is green.
getter.get returns nil for a carrier key whose value is nil, so the byte accounting raised NoMethodError before any limit was applied. Guard on the value and cover it with a test that fails without the guard. The extra branch tripped Metrics/CyclomaticComplexity (8/7), and extracting the limit check tripped Metrics/ClassLength (101/100), so the check now lives in within_baggage_limits? and the private_constant list is reflowed. Assisted-By: Claude Fable 5
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
3 participants
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.
The Jaeger propagator extracts every
uberctx-header into baggage with no bound:context_with_extracted_baggagewalks all carrier keys and sets each one, so an inbound carrier decides how much baggage enters the context.The Jaeger format defines no limits, so this borrows the W3C Baggage spec limits — 180 entries, 4096 bytes per entry, 8192 bytes total, byte-denominated — dropping members once a limit is reached and keeping the earlier ones.
opentelemetry-javacaps its own Jaeger propagator the same way and for the same reason (CVE-2026-45292); the W3C propagator in this repo is being brought to parity in #2298.Adds four extract tests: entry count, per-entry bytes, per-entry bytes measured in bytes rather than characters, and the total-bytes cap. Each fails without the change.
Assisted-By: Claude Fable 5