Skip to content

api,sink: mask sink uri secrets - #5093

Merged
ti-chi-bot[bot] merged 3 commits into
pingcap:masterfrom
asddongmen:fix/mask-sink-uri-secrets
May 19, 2026
Merged

api,sink: mask sink uri secrets#5093
ti-chi-bot[bot] merged 3 commits into
pingcap:masterfrom
asddongmen:fix/mask-sink-uri-secrets

Conversation

@asddongmen

@asddongmen asddongmen commented May 19, 2026

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: close #5094

What is changed and how it works?

This PR masks sensitive sink URI data before sink validation errors reach the OpenAPI request log. It covers changefeed create, update, resume, verify-table, and shared sink parse/unknown scheme paths. It also avoids logging parse errors inside URI masking helpers and adds tests for masked query secrets and malformed
URIs containing userinfo passwords.

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)

Questions

Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?

Release note

None

Summary by CodeRabbit

  • Security

    • Enhanced error handling to automatically mask sensitive sink URI credentials including passwords and API keys in error messages, preventing accidental exposure in system logs and error responses.
  • Tests

    • Added comprehensive test coverage validating credential masking across all changefeed operations and sink configuration validation endpoints.

Review Change Stack

@ti-chi-bot ti-chi-bot Bot added do-not-merge/needs-linked-issue do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels May 19, 2026
@ti-chi-bot

ti-chi-bot Bot commented May 19, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5d26ab36-46b3-4616-aa4e-7bc46e309c62

📥 Commits

Reviewing files that changed from the base of the PR and between 7b68b70 and 71275a7.

📒 Files selected for processing (9)
  • .gitignore
  • api/v2/changefeed.go
  • api/v2/changefeed_test.go
  • downstreamadapter/sink/sink.go
  • downstreamadapter/sink/sink_test.go
  • pkg/check/active_active_tso_indexes.go
  • pkg/check/cluster.go
  • pkg/util/uri.go
  • pkg/util/uri_test.go

📝 Walkthrough

Walkthrough

This PR adds utility functions to mask sensitive sink URI parameters and integrates them throughout changefeed validation paths to prevent credential exposure in error logs and API responses. Changes span utility functions, API handlers, sink adapters, and validation layers.

Changes

Sink URI Secret Masking

Layer / File(s) Summary
URI masking utility functions and tests
pkg/util/uri.go, pkg/util/uri_test.go
Add MaskSensitiveDataInURIForError for safe error display and MaskSensitiveDataInURLError to rewrite *url.Error with masked URLs. Remove logging dependencies from existing helpers.
API handler masking and integration
api/v2/changefeed.go, api/v2/changefeed_test.go
Add maskSinkURIForError and genSinkURIInvalidError helpers. Integrate masking in CreateChangefeed, VerifyTable, ResumeChangefeed, and UpdateChangefeed error paths.
Sink adapter masking integration
downstreamadapter/sink/sink.go, downstreamadapter/sink/sink_test.go
Mask sensitive data in New and Verify error paths when URI parsing fails or sink scheme is unsupported.
Validation layer masking integration
pkg/check/active_active_tso_indexes.go, pkg/check/cluster.go
Update sink URI error wrapping to mask sensitive data in validation error reporting.
Development configuration
.gitignore
Add .design ignore pattern.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Secrets hidden in URIs bright,
Now masked from logs' revealing light,
Passwords swaddled, keys wrapped tight,
Sensitive data gone from sight!
Hop hop, the errors shine so right! 🔐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 35.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'api,sink: mask sink uri secrets' is clear and directly describes the main changes: masking sensitive data in sink URI error handling across API and sink modules.
Description check ✅ Passed The description follows the template with Issue Number clearly linked (#5094), explains what is changed and how (masking sink URI data before validation errors), lists appropriate tests (Unit test, Manual test), and includes a release note (None).
Linked Issues check ✅ Passed The PR implements the core requirement from issue #5094: masking sensitive sink URI data before errors reach OpenAPI logs. Changes cover changefeed handlers, sink validation paths, and error-handling utilities with corresponding test coverage.
Out of Scope Changes check ✅ Passed The .gitignore change (ignoring .design path) is a minor housekeeping item unrelated to the primary objective of masking sink URI secrets, though minimally disruptive.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ti-chi-bot ti-chi-bot Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 19, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request implements sensitive data masking for SinkURI in error messages across several components, including OpenAPI v2 handlers, sink adapters, and cluster checks. It introduces utility functions to redact credentials from URIs and adds comprehensive unit tests for this logic. The reviewer feedback highlights that the current implementation often discards the underlying error context when masking the URI, which can hinder debuggability for non-sensitive parsing failures. It is recommended to wrap the original error while still masking the URI string to preserve diagnostic information.

Comment thread api/v2/changefeed.go Outdated
Comment thread api/v2/changefeed.go Outdated
Comment thread downstreamadapter/sink/sink.go Outdated
Comment thread downstreamadapter/sink/sink.go Outdated
@ti-chi-bot ti-chi-bot Bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. labels May 19, 2026
@asddongmen
asddongmen marked this pull request as ready for review May 19, 2026 03:51
@ti-chi-bot ti-chi-bot Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 19, 2026
@ti-chi-bot ti-chi-bot Bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels May 19, 2026
@ti-chi-bot ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels May 19, 2026
@ti-chi-bot

ti-chi-bot Bot commented May 19, 2026

Copy link
Copy Markdown

[LGTM Timeline notifier]

Timeline:

  • 2026-05-19 03:54:31.620924499 +0000 UTC m=+236401.125055176: ☑️ agreed by flowbehappy.
  • 2026-05-19 03:55:09.460351188 +0000 UTC m=+236438.964481863: ☑️ agreed by 3AceShowHand.

@ti-chi-bot

ti-chi-bot Bot commented May 19, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 3AceShowHand, flowbehappy, wk989898

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [3AceShowHand,flowbehappy,wk989898]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Comment thread api/v2/changefeed.go
return changefeedDisplayName, true
}

func maskSinkURIForError(sinkURI string) string {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This 2 method is only a think wrapper about other method, useless.

@ti-chi-bot
ti-chi-bot Bot merged commit d38ea0a into pingcap:master May 19, 2026
25 of 31 checks passed
@asddongmen

Copy link
Copy Markdown
Collaborator Author

/cherry-pick release-8.5

@ti-chi-bot

Copy link
Copy Markdown
Member

@asddongmen: new pull request created to branch release-8.5: #5762.
But this PR has conflicts, please resolve them!

Details

In response to this:

/cherry-pick release-8.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot Bot pushed a commit that referenced this pull request Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mask sink URI secrets in OpenAPI error logs

5 participants