-
Notifications
You must be signed in to change notification settings - Fork 293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Resources.Azure] NugetAudit - fix dependencies with known vulnerabilities #2056
[Resources.Azure] NugetAudit - fix dependencies with known vulnerabilities #2056
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2056 +/- ##
==========================================
+ Coverage 73.91% 82.35% +8.43%
==========================================
Files 267 6 -261
Lines 9615 136 -9479
==========================================
- Hits 7107 112 -6995
+ Misses 2508 24 -2484
Flags with carried forward coverage won't be shown. Click here to find out more. |
@@ -15,6 +15,8 @@ | |||
<ItemGroup> | |||
<PackageReference Include="OpenTelemetry" Version="$(OpenTelemetryCoreLatestVersion)" /> | |||
<PackageReference Include="System.Text.Json" Version="4.7.2" /> | |||
<!-- System.Text.Encodings.Web is indirect reference. It is needed to upgrade it directly to avoid https://github.com/advisories/GHSA-ghhp-997w-qr28 --> | |||
<PackageReference Include="System.Text.Encodings.Web" Version="4.7.2" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .NET Standard 2.0 version of System.Text.Json
has an indirect dependency on System.Text.Encodings.Web
. It is better to upgrade System.Text.Json
, as was done in OpenTelemetry.Exporter.OneCollector
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may need to re-evaluate whether this is still a challenge with auto-instrumentation - #1279 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible, I would be more conservative with versioning. This solution is applied in Exporter.Console. I do not see any reason we cannot follow this pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will explore it outside of this PR to understand System.Text.Json version could be bumped.
Follow up to #2034
Changes
Changelog based on https://github.com/open-telemetry/opentelemetry-dotnet/blob/37535a5607ee7e4056c0e274ec01d1e0111a64be/src/OpenTelemetry.Exporter.Console/CHANGELOG.md#L112-L114
Merge requirement checklist
[ ] Unit tests added/updatedCHANGELOG.md
files updated for non-trivial changes