Skip to content

Conversation

@marevers
Copy link

This PR upgrades golang.org/x/crypto to version v0.45.0. Versions prior to it contain two CVEs (medium severity):

I also ran go mod tidy to fix some issues in the go.mod file:

  • toolchain 1.24 while go version 1.23
  • github.com/shopspring/decimal as direct dependency, not indirect

@marevers
Copy link
Author

@microsoft-github-policy-service agree company="GK Software SE"

@shueybubbles
Copy link
Collaborator

shueybubbles commented Dec 23, 2025

thx for the PR!

Updating the tool chain creates some build errors

# [github.com/microsoft/go-mssqldb]
.\bulkcopy.go:144:74: non-constant format string in call to (*github.com/microsoft/go-mssqldb.Bulk).dlogf
.\token.go:1199:39: non-constant format string in call to (*github.com/microsoft/go-mssqldb.tdsSession).LogF
.\error_test.go:55:20: non-constant format string in call to fmt.Errorf
.\log_test.go:69:46: non-constant format string in call to fmt.Errorf
FAIL	github.com/microsoft/go-mssqldb [build failed]
Command exited with code 1
``` #Closed

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR upgrades golang.org/x/crypto from v0.38.0 to v0.45.0 to address two medium-severity CVEs (CVE-2025-47914 and CVE-2025-58181). The upgrade also includes related golang.org/x dependencies and fixes issues in go.mod by upgrading the Go version to 1.24.0 and properly classifying github.com/shopspring/decimal as a direct dependency.

Key Changes:

  • Upgrades golang.org/x/crypto to v0.45.0 to address security vulnerabilities
  • Upgrades Go version from 1.23.0 to 1.24.0 to match the existing toolchain go1.24.4
  • Moves github.com/shopspring/decimal from indirect to direct dependencies (correctly reflects actual usage in the codebase)

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.

File Description
go.mod Upgrades Go version to 1.24.0, updates golang.org/x dependencies to latest versions, and properly classifies shopspring/decimal as a direct dependency
go.sum Updates checksums for golang.org/x/crypto (v0.45.0), golang.org/x/net (v0.47.0), golang.org/x/sys (v0.38.0), and golang.org/x/text (v0.31.0)

Copy link
Collaborator

@shueybubbles shueybubbles left a comment

Choose a reason for hiding this comment

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

pls fix the build breaks and update the appveyor action per comments

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.53%. Comparing base (0bbbaf2) to head (5ab3793).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #301      +/-   ##
==========================================
- Coverage   75.66%   75.53%   -0.14%     
==========================================
  Files          34       34              
  Lines        6580     6580              
==========================================
- Hits         4979     4970       -9     
- Misses       1317     1324       +7     
- Partials      284      286       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@marevers
Copy link
Author

marevers commented Jan 2, 2026

@shueybubbles thanks for your comment, these issues should be resolved now. Build breaks were due to incorrect use of some logging statements, I would have expected the build to have already failed on 1.23 - but maybe this is due to more strict enforcement in 1.24 and up? At any rate, this should be fine now.

@shueybubbles
Copy link
Collaborator

@marevers thx. What do you think of the upcoming cert handling break in Go 1.25 per #302 ? Are app developers/enterprises more interesting in having secure clients on modern code or do they need old clients that handle sha-1 certs on their old SQL Servers because they don't want to bother getting new certs?

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.

err, more := tok.(error)
if more {
t.sess.LogF(t.ctx, msdsn.LogDebug, "nextToken returned an error:"+err.Error())
t.sess.LogF(t.ctx, msdsn.LogDebug, "%s", "nextToken returned an error:"+err.Error())
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

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

This code change (fixing format string usage) appears unrelated to the PR description, which focuses on upgrading golang.org/x/crypto to address CVEs. While this is a good fix for potential format string vulnerabilities, it should ideally be in a separate PR or at least mentioned in the PR description. If this change is required by linting tools or was discovered during the dependency upgrade process, please update the PR description to mention it.

Copilot uses AI. Check for mistakes.
errMsg := "Retryable Test Error"
retryPrefix := "RETRY: "
inErr := StreamError{InnerError: fmt.Errorf(errMsg)}
inErr := StreamError{InnerError: errors.New(errMsg)}
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

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

This code change (using errors.New instead of fmt.Errorf for non-formatted errors) appears unrelated to the PR description, which focuses on upgrading golang.org/x/crypto to address CVEs. While this is a good improvement, it should ideally be in a separate PR or at least mentioned in the PR description.

Copilot uses AI. Check for mistakes.
@shueybubbles shueybubbles merged commit 232fa60 into microsoft:main Jan 4, 2026
11 checks passed
@marevers
Copy link
Author

marevers commented Jan 5, 2026

@marevers thx. What do you think of the upcoming cert handling break in Go 1.25 per #302 ? Are app developers/enterprises more interesting in having secure clients on modern code or do they need old clients that handle sha-1 certs on their old SQL Servers because they don't want to bother getting new certs?

@shueybubbles I checked the issue you linked, in our case I don't believe we have any servers that are using SHA-1 so this should not influence us. Users still using SHA-1 should strongly consider switching to something else as it is cryptographically broken.

@marevers marevers deleted the crypto-0-45 branch January 6, 2026 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants