Skip to content

Conversation

@asmyasnikov
Copy link
Member

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: YDBAPPTEAM-453

What is the new behavior?

Other information

… data source name) from error messages for security reasons.
@github-actions
Copy link

github-actions bot commented Dec 5, 2025

summary

Inferred base version: v3.121.0
Suggested version: v3.121.1

@codecov-commenter
Copy link

codecov-commenter commented Dec 5, 2025

Codecov Report

❌ Patch coverage is 84.61538% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.90%. Comparing base (fbf3502) to head (962b353).

Files with missing lines Patch % Lines
sql.go 0.00% 4 Missing ⚠️
internal/secret/dsn.go 93.93% 1 Missing and 1 partial ⚠️
driver.go 0.00% 1 Missing ⚠️
options.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1973      +/-   ##
==========================================
- Coverage   73.98%   73.90%   -0.08%     
==========================================
  Files         392      394       +2     
  Lines       34417    34454      +37     
==========================================
  Hits        25462    25462              
- Misses       7834     7863      +29     
- Partials     1121     1129       +8     
Flag Coverage Δ
experiment 73.57% <84.61%> (-0.06%) ⬇️
go-1.21.x 72.66% <84.61%> (-0.05%) ⬇️
go-1.25.x 73.85% <84.61%> (-0.10%) ⬇️
integration 54.85% <19.23%> (-0.46%) ⬇️
macOS 47.30% <84.61%> (+0.03%) ⬆️
ubuntu 73.89% <84.61%> (-0.09%) ⬇️
unit 47.30% <84.61%> (+0.02%) ⬆️
windows 47.27% <84.61%> (+0.01%) ⬆️
ydb-24.4 54.05% <19.23%> (-0.51%) ⬇️
ydb-25.2 54.76% <19.23%> (-0.21%) ⬇️
ydb-latest 54.41% <19.23%> (-0.21%) ⬇️
ydb-nightly 73.57% <84.61%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copilot finished reviewing on behalf of asmyasnikov December 5, 2025 15:42
Copy link
Contributor

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 implements security improvements to prevent sensitive credential data from being exposed in error messages by sanitizing DSN (Data Source Name) strings. The implementation introduces a new secret.DSN() function that removes credentials from connection strings before they are included in error messages.

Key Changes

  • Created a new internal/secret package with a DSN() function that removes sensitive credentials from connection strings
  • Applied the sanitization to error messages in three key locations: sql.go, options.go, and driver.go
  • Added test coverage for the DSN sanitization functionality

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
internal/secret/dsn.go New implementation of DSN sanitization function that removes credentials from connection strings
internal/secret/dsn_test.go Test cases for DSN sanitization covering various credential formats
sql.go Applied DSN sanitization to SQL driver error messages
options.go Applied DSN sanitization to connection string parsing errors
driver.go Applied DSN sanitization to driver initialization errors
CHANGELOG.md Documented the security enhancement

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

asmyasnikov and others added 2 commits December 5, 2025 19:21
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@asmyasnikov asmyasnikov added the SLO label Dec 5, 2025
@github-actions
Copy link

github-actions bot commented Dec 5, 2025

🌋 SLO Test Results

Status: 🟡 6 workloads tested • 5 workloads with warnings

Workload Metrics Regressions Improvements Links
🟡 database-sql-table 8 0 0 ReportCheck
🟡 native-table-over-query-service 8 1 0 ReportCheck
🟡 native-table 8 0 0 ReportCheck
🟡 database-sql-query 8 0 0 ReportCheck
🟡 native-bulk-upsert 8 0 0 ReportCheck
🟢 native-query 8 1 0 ReportCheck

Generated by ydb-slo-action

@github-actions github-actions bot removed the SLO label Dec 5, 2025
@asmyasnikov asmyasnikov requested a review from Copilot December 6, 2025 09:05
@asmyasnikov asmyasnikov changed the title Excluded the sensitive credential data in the connection string (DSN,data source name) from error messages for security reasons. Masked the sensitive credential data in the connection string (DSN,data source name) from error messages for security reasons. Dec 6, 2025
Copy link
Contributor

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 12 out of 12 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants