Skip to content

[8.18](backport #45857) [metricbeat] [sql] sanitizeError: replace sensitive info even if it is escaped, add pattern-based sanitization#46185

Merged
mykola-elastic merged 3 commits into8.18from
mergify/bp/8.18/pr-45857
Aug 22, 2025
Merged

[8.18](backport #45857) [metricbeat] [sql] sanitizeError: replace sensitive info even if it is escaped, add pattern-based sanitization#46185
mykola-elastic merged 3 commits into8.18from
mergify/bp/8.18/pr-45857

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Aug 22, 2025

Suggested fix for the connection string leak described in the issue: #45852

After this change not only sensitive string will be replaced in the error message, but the escaped and url-encoded representations of it will be replaced. (Escaped as in fmt.Sprintf("%q", sensitive)). Additionally, this PR also improves sanitizeError by adding pattern-based sanitization.

What triggered the original issue: net/url library (in case of the issue, invoked by mssql driver) has its own error type which uses %q when printing URLs:

// go/src/net/url/url.go
type Error struct {
	Op  string
	URL string
	Err error
}

func (e *Error) Error() string { return fmt.Sprintf("%s %q: %s", e.Op, e.URL, e.Err) }

Additional refactoring:

  • moved SanitizeError to the "github.com/elastic/beats/v7/metricbeat/helper/sql" package as it is a generic helper function, which is also related targeted at errors returned by sql driver libs
  • implemented custom error type sqlSanitizedError so that errors.Is() and errors.As() still work with the errors returned by SanitizeError
  • adding error checks or //nolint:errcheck comments in "github.com/elastic/beats/v7/metricbeat/helper/sql" package, as changes there triggered more CI checks

Proposed commit message

See title.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

  1. Use metricbeat module sql with driver set to driver: mssql
  2. Set hosts to some invalid string with symbols (", \, etc.)
  3. The parse error shouldn't contain the host connection string, but contain (redacted) in the place of it

Related issues


This is an automatic backport of pull request #45857 done by [Mergify](https://mergify.com).
…s escaped, add pattern-based sanitization (#45857)

(cherry picked from commit bf63860)

# Conflicts:
#	x-pack/metricbeat/module/sql/query/dsn.go
#	x-pack/metricbeat/module/sql/query/query_test.go
@mergify mergify bot added backport conflicts There is a conflict in the backported pull request labels Aug 22, 2025
@mergify mergify bot requested review from a team as code owners August 22, 2025 11:36
@mergify
Copy link
Contributor Author

mergify bot commented Aug 22, 2025

Cherry-pick of bf63860 has failed:

On branch mergify/bp/8.18/pr-45857
Your branch is up to date with 'origin/8.18'.

You are currently cherry-picking commit bf63860f1.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   CHANGELOG.next.asciidoc
	modified:   metricbeat/helper/sql/sql.go
	modified:   metricbeat/helper/sql/sql_test.go
	modified:   x-pack/metricbeat/module/sql/query/query.go

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	both modified:   x-pack/metricbeat/module/sql/query/dsn.go
	deleted by them: x-pack/metricbeat/module/sql/query/query_test.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify bot requested review from AndersonQ and VihasMakwana and removed request for a team August 22, 2025 11:36
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Aug 22, 2025
@github-actions github-actions bot added Metricbeat Metricbeat Team:Obs-InfraObs Label for the Observability Infrastructure Monitoring team bugfix labels Aug 22, 2025
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Aug 22, 2025
Copy link
Contributor

@mykola-elastic mykola-elastic left a comment

Choose a reason for hiding this comment

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

Manual testing - PASS

@mykola-elastic mykola-elastic removed the conflicts There is a conflict in the backported pull request label Aug 22, 2025
@mykola-elastic mykola-elastic merged commit e996b38 into 8.18 Aug 22, 2025
36 checks passed
@mykola-elastic mykola-elastic deleted the mergify/bp/8.18/pr-45857 branch August 22, 2025 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport bugfix Metricbeat Metricbeat Team:Obs-InfraObs Label for the Observability Infrastructure Monitoring team

1 participant