fix(llc): sanitize SystemEnvironment updates against the SDK-owned baseline - #163
Open
VelikovPetar wants to merge 2 commits into
Open
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #163 +/- ##
==========================================
+ Coverage 60.32% 60.64% +0.31%
==========================================
Files 192 192
Lines 7834 7857 +23
==========================================
+ Hits 4726 4765 +39
+ Misses 3108 3092 -16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…t-in-stream_core-to-prevent-customers
VelikovPetar
marked this pull request as ready for review
August 24, 2026 16:09
xsahil03x
approved these changes
Aug 24, 2026
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.
Submit a pull request
Linear: FLU-727
Github Issue: #
CLA
Description of the pull request
Ports the
X-Stream-Clientsanitization from FLU-565 / GetStream/stream-chat-flutter#2790 intostream_core.Why
SystemEnvironmentManager.updateEnvironmentassigned a consumer-suppliedSystemEnvironmentwholesale, andHeadersInterceptorre-reads the value on every request.stream_feedsexposes that path publicly asStreamFeedsClient.updateSystemEnvironment, so any integrator could rewrite the fields Stream uses to identify the SDK —sdkName,sdkIdentifier,sdkVersionandosName.Those fields drive SDK-usage analytics and support triage. An integrator reporting
stream-chat-android v99.0.0from a Dart Feeds app silently corrupts both, and spoofed traffic is indistinguishable from real traffic after the fact.Chat fixed this in its own copy of the class.
stream_coreis a generalized fork of Chat's pre-fix version —SystemEnvironmentis field-for-field identical andxStreamClientHeaderis character-identical — so the same hole is here, and every SDK built on core inherits it.What
Core can't lock these to internal constants the way Chat does, because
sdkNameandsdkVersiondiffer per product (stream-chat,stream-feeds,stream-video). Instead the environment passed to the constructor is treated as the SDK-owned baseline — only the product SDK constructs the manager; consumers only ever reachupdateEnvironment— and every later update is sanitized against it:sdkName,sdkVersion,osNamesdkIdentifierdart→flutterpromotion is acceptedappName,appVersion,osVersion,deviceModelThe SDK-owned values are snapshotted as scalars at construction rather than held as a
SystemEnvironmentreference, since the type isn'tfinaland a subtype could otherwise return a different value on every getter read.The promotion path has to stay. The package that promotes is never the package that constructs the manager:
stream_chat_flutter_corepromotes frominitStateon a client the app already built, soupdateEnvironmentis its only channel, and a futurestream_feeds_flutterwould be identical. LockingsdkIdentifieroutright would silently regress every Flutter app to-dart-.Scope of impact
stream_feedsis the only consumer of the manager today — Video doesn't depend onstream_core, andstream_core_flutterdepends on it but never uses it. Nothing inside this repo constructs it exceptHeadersInterceptor.Non-breaking: no signature changes, so
^0.4.0admits it. It is a runtime behavior change to a public method, so flagging the judgment call here rather than burying it — happy to move the entry under a breaking heading if reviewers disagree.Testing
22 new tests in
packages/stream_core/test/api/, covering each sanitization branch plus the promotion ratchet, unrecognized identifiers, and baseline drift. Also ports Chat's 9 header-format tests, sincexStreamClientHeaderis public API of this package and had zero coverage.Verified beyond green tests:
updateEnvironmentto the old wholesale assignment fails 6 tests; removing only the unrecognized-identifier guard fails exactly the 1 test written for it.stream_feedspointed at this branch, observing the header a local server actually received:sdkName: 'stream-chat-android',sdkIdentifier: 'android',sdkVersion: '99.9.9',osName: 'spoofed-os'— none of it reached the wire, while the legitimate app/device enrichment did.stream_coresuite (364) and fullstream_feedssuite (404) pass against this branch.Screenshots / Videos
N/A — no UI surface.