Skip to content

fix: sanitize credentials from connection string parsing errors#319

Open
Copilot wants to merge 1 commit intomainfrom
copilot/fix-credential-leak-in-connection-errors
Open

fix: sanitize credentials from connection string parsing errors#319
Copilot wants to merge 1 commit intomainfrom
copilot/fix-credential-leak-in-connection-errors

Conversation

Copy link

Copilot AI commented Feb 2, 2026

When url.Parse fails on malformed connection URLs, Go's standard library includes the full URL (including credentials) in the error message. This error was propagated directly to callers, leaking usernames and passwords into application logs.

Changes

  • msdsn/conn_str.go: Return sanitized error from splitConnectionStringURL when URL parsing fails
  • msdsn/conn_str_test.go: Add test coverage verifying credentials are not exposed in error messages

Example

Before:

parse "sqlserver://user:password@host:1433\x00invalid": net/url: invalid control character in URL

After:

unable to parse connection string: invalid URL format

The error provides actionable context without exposing sensitive data.

Original prompt

This section details on the original issue you should resolve

<issue_title>conn.Query* might return error that contains connection credentials</issue_title>
<issue_description>### Describe the bug

When the connection string is malformed, go-mssqldb might return an error that contains the username and password, which might be logged by the caller and cause credentials leak.

This is caused by a late url.Parse call which might include the entire connection URL in the error message.

Stack trace

url.Parse (url.go:478) net/url
msdsn.splitConnectionStringURL (conn_str.go:631) github.com/microsoft/go-mssqldb/msdsn
msdsn.getDsnParams (conn_str.go:272) github.com/microsoft/go-mssqldb/msdsn
msdsn.Parse (conn_str.go:291) github.com/microsoft/go-mssqldb/msdsn
mssql.(*Driver).open (mssql.go:410) github.com/microsoft/go-mssqldb
mssql.(*Driver).Open (mssql.go:77) github.com/microsoft/go-mssqldb
sql.dsnConnector.Connect (sql.go:791) database/sql
<autogenerated>:2
sql.(*DB).conn (sql.go:1415) database/sql
sql.(*DB).query (sql.go:1749) database/sql
sql.(*DB).QueryContext.func1 (sql.go:1732) database/sql
sql.(*DB).retry (sql.go:1566) database/sql
sql.(*DB).QueryContext (sql.go:1731) database/sql
sql.(*DB).Query (sql.go:1745) database/sql

Steps to reproduce

func (suite *MssqlDBTestSuite) TestConnectionCredentialsLeak() {
	conn, err := sql.Open("mssql", "sqlserver://username:password@[foo].bar:1433?database=foo&encrypt=true&ssl=require")
	suite.NoError(err)

	_, err = conn.Query("select 1")
	suite.NoError(err)
}

Actual behaviour

the error returned from conn.Query reveals the connection credentials

Error:      	Received unexpected error:
        	            	parse "sqlserver://username:password@[foo].bar:1433?database=foo&encrypt=true&ssl=require": invalid port ".bar:1433" after host

Expected behaviour

The error does not include credentials (username/password)

Further technical details

SQL Server version: doesn't matter. this error happens on the client side.
Operating system: macOS 14.5 on M1 CPU
Table schema: doesn't matter. this error happens on the client side.

Notes

The issue happens when the caller provides a malformed connection string. However, it's still undesirable for go-mssqldb to return an error that could lead to the leak of connection credentials. Especially that the error isn't returned in sql.Open, but later on conn.Query, so the error can not be easily handled centrally.

</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Fix connection error to prevent credential leakage fix: sanitize credentials from connection string parsing errors Feb 2, 2026
Copilot AI requested a review from dlevy-msft-sql February 2, 2026 16:13
@codecov-commenter
Copy link

codecov-commenter commented Feb 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.00%. Comparing base (1c8ea5b) to head (236e400).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #319      +/-   ##
==========================================
+ Coverage   75.45%   79.00%   +3.54%     
==========================================
  Files          34       34              
  Lines        6597     6648      +51     
==========================================
+ Hits         4978     5252     +274     
+ Misses       1333     1109     -224     
- Partials      286      287       +1     

☔ 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.

@dlevy-msft-sql dlevy-msft-sql added bug Something isn't working Size: S Small issue (less than one week effort, less than 250 lines of code) security needs-work labels Feb 2, 2026
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 pull request fixes a security issue where malformed connection URLs cause url.Parse to fail with error messages that include the full URL, potentially leaking usernames and passwords into application logs. The fix sanitizes these errors to return a generic message without sensitive information.

Changes:

  • Sanitized error handling in splitConnectionStringURL to prevent credential leakage when URL parsing fails
  • Added comprehensive test coverage to verify credentials are not exposed in error messages
  • Minor whitespace formatting fix for consistency

Reviewed changes

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

File Description
msdsn/conn_str.go Replaced direct error propagation from url.Parse with a sanitized generic error message; minor whitespace fix
msdsn/conn_str_test.go Added test function TestCredentialNotLeakedInError with two test cases verifying credentials are not present in error messages

@dlevy-msft-sql dlevy-msft-sql marked this pull request as ready for review February 2, 2026 22:09
@dlevy-msft-sql dlevy-msft-sql added in-review Size: XS Less than 50 lines of code and removed needs-work Size: S Small issue (less than one week effort, less than 250 lines of code) labels Feb 2, 2026
@dlevy-msft-sql dlevy-msft-sql added Priority: 1 High priority/impact and removed in-review labels Feb 3, 2026
Prevent leaking usernames and passwords in error messages when URL parsing fails.
The original url.Parse error could include the full connection string with credentials.

- Replace url.Parse error with generic message
- Add tests using testify to verify credentials are not leaked
@dlevy-msft-sql dlevy-msft-sql force-pushed the copilot/fix-credential-leak-in-connection-errors branch from c86e503 to 236e400 Compare February 5, 2026 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Priority: 1 High priority/impact security Size: XS Less than 50 lines of code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

conn.Query* might return error that contains connection credentials

3 participants