Mask secrets for XDBC and NATS engines#99344
Conversation
|
Workflow [PR], commit [6f7022d] Summary: ✅ AI ReviewSummaryThis PR extends secret masking to ClickHouse Rules
Final Verdict
|
clickhouse-gh
Bot
added
the
pr-bugfix
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
antaljanosbenjamin
changed the title
|
|
||
| if (ds_idx >= 0 && cs_idx >= 0) | ||
| { | ||
| /// Both present — hide all named arguments starting from index 1. |
There was a problem hiding this comment.
Why we decide to hide all arguments here and not just those two?
There was a problem hiding this comment.
Because FunctionSecretArgumentsFinder::Result can only hide arguments next to each other as it has start and count only. So in case the arguments are not next to each other, we should either extend this logic or hide arguments in between. As specifying both connection_settings and datasource is not a valid option, I opted to go with the easiest solution that shouldn't cause any harm. Basically the query shouldn't succeed, so hiding more values is okay in my opinion.
Because if the default destruction order `execute_tasks_scheduler` got destroyed before `loop`, but `loop` closes all still registered handles in its destructor.
| NATSHandler::~NATSHandler() | ||
| { | ||
| /// Close the async handle before UVLoop destructor runs, | ||
| /// otherwise uv_loop_close reads from already-destroyed memory. | ||
| uv_close(reinterpret_cast<uv_handle_t *>(&execute_tasks_scheduler), nullptr); | ||
| uv_run(loop.getLoop(), UV_RUN_NOWAIT); | ||
| } | ||
|
|
|
|
LLVM Coverage Report
PR changed lines: PR changed-lines coverage: 98.70% (76/77, 0 noise lines excluded) |
robot-ch-test-poll1
added
the
pr-synced-to-cloud
antaljanosbenjamin
added
pr-backport
robot-ch-test-poll
added
the
pr-must-backport-synced
Backport #99344 to 26.2: Mask secrets for XDBC and NATS engines
robot-ch-test-poll2
added
the
pr-backports-created
Backport #99344 to 25.8: Mask secrets for XDBC and NATS engines
Backport #99344 to 25.12: Mask secrets for XDBC and NATS engines
6 participants
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Credentials in JDBC, ODBC, and NATS connection strings are now masked in query logs and
SHOW CREATEoutput, preventing accidental exposure of sensitive information. For URI-style connection strings (e.g. {scheme}://{user}:{password}@{host}), only the password portion is masked while the rest remains visible for easier debugging. Thenats_tokensetting is now also masked.Documentation entry for user-facing changes
Note
Medium Risk
Touches centralized secret-masking logic for function/table-engine argument rendering; mistakes could either leak credentials or over-mask user-visible queries, but scope is limited to JDBC/ODBC and NATS settings.
Overview
Prevents credential leakage by extending secret masking to
JDBC/ODBCtable engines andjdbc/odbctable functions, treating the DSN/connection string as sensitive.For URI-style connection strings, it now rewrites only the password portion (via
maskURIPassword), while non-URI DSNs are fully replaced with'[HIDDEN]'; named-collection calls also handledatasource/connection_settings(and defensively hide all named args if both are provided).Additionally,
NATSengine masking now hidesnats_token, and integration tests are expanded to assert the new masking behavior in logs andSHOW CREATEoutput.Written by Cursor Bugbot for commit b0dedc8. This will update automatically on new commits. Configure here.