Skip to content

Hide secrets inside query in client command (in top/ps)#79507

Open
aaaengel wants to merge 7 commits intoClickHouse:masterfrom
aaaengel:issue_65549
Open

Hide secrets inside query in client command (in top/ps)#79507
aaaengel wants to merge 7 commits intoClickHouse:masterfrom
aaaengel:issue_65549

Conversation

@aaaengel
Copy link
Copy Markdown
Contributor

@aaaengel aaaengel commented Apr 23, 2025

Changelog category:

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Hide secrets inside query in client command (in top/ps) for s3, s3Cluster, remote, remoteSecure, mysql, postgresql, mongodb functions.

Documentation entry for user-facing changes

SELECT * FROM s3('http://localhost:9000/test/hello.csv', 'secret', 'secret', 'CSV', 'value String');

After ps appear as

SELECT * FROM s3('http://localhost:9000/test/hello.csv', '', '', 'CSV', 'value String');

Closes #65549

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 23, 2025

CLA assistant check
All committers have signed the CLA.

@aaaengel aaaengel changed the title Issue 65549 Secrets in queries are hidden now Apr 23, 2025
@nikitamikhaylov nikitamikhaylov added the can be tested Allows running workflows for external contributors label Apr 24, 2025
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Apr 24, 2025

Workflow [PR], commit [2ca252b]

@clickhouse-gh clickhouse-gh bot added the pr-improvement Pull request with some product improvements label Apr 24, 2025
@azat azat self-assigned this Apr 25, 2025
@azat azat changed the title Secrets in queries are hidden now Hide secrets from client command (in top/ps) May 25, 2025
@azat azat changed the title Hide secrets from client command (in top/ps) Hide secrets inside query in client command (in top/ps) May 25, 2025
static const SecretRule rules[] = {
{"s3"sv, {1, 2}},
{"s3Cluster"sv, {2, 3}},
{"remote"sv, {4}},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For remote/remoteSecure we can have two options -

/// remote('addresses_expr', db.table, 'user', 'password')
/// remote('addresses_expr', 'db', 'table', 'user', 'password')

I guess it is OK to simply hide both

Comment on lines +6 to +8
bool isIdentChar(char c);
char * mask(char * begin, char * end);
void shrederSecretInQuery(char* query);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This functions used only internally, we don't need to expose them, let's declare them either static on .cpp and remove from .h

Comment on lines +58 to +63
for (const auto & r : rules)
if (ident == r.func)
{
secret = r.secret_pos;
break;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
for (const auto & r : rules)
if (ident == r.func)
{
secret = r.secret_pos;
break;
}
for (const auto & r : rules)
{
if (ident == r.func)
{
secret = r.secret_pos;
break;
}
}

return begin;
}

void shrederSecretInQuery(char * query)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess it does not worth to use Lexer here, so OK


bool in_quote = false;
size_t arg_idx = 0;
char * quote_beg = nullptr;
Copy link
Copy Markdown
Member

@azat azat May 25, 2025

Choose a reason for hiding this comment

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

Please do not abuse abbrevs for variable names, i.e. no need to use beg over begin (maybe I missed something as well)

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Jul 1, 2025

Dear @azat, this PR hasn't been updated for a while. You will be unassigned. Will you continue working on it? If so, please feel free to reassign yourself.

@clickhouse-gh clickhouse-gh bot unassigned azat Jul 1, 2025
@azat azat self-assigned this Sep 17, 2025
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Oct 21, 2025

Dear @azat, this PR hasn't been updated for a while. You will be unassigned. Will you continue working on it? If so, please feel free to reassign yourself.

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

Labels

can be tested Allows running workflows for external contributors pr-improvement Pull request with some product improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cleanup passwords from inside the query in the command line parameter.

4 participants