From 14ac53379a9dfa5d06cd335c2eae17305cdce1dc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 01:01:05 +0000 Subject: [PATCH] test: harden admin credential logging regression Co-authored-by: renecannao <3645227+renecannao@users.noreply.github.com> --- lib/Admin_Handler.cpp | 32 +++++++++++-------- .../tests/admin_set_credentials_logging-t.cpp | 27 ++++++++++++---- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/lib/Admin_Handler.cpp b/lib/Admin_Handler.cpp index 2ea438171..7e747bed3 100644 --- a/lib/Admin_Handler.cpp +++ b/lib/Admin_Handler.cpp @@ -1114,9 +1114,12 @@ static char* mask_sensitive_values_in_query(const char* query) { } static bool is_sensitive_set_variable_name(const char* var_name) { - return strstr(var_name, (char*)"password") || - strcmp(var_name, (char*)"mysql-default_authentication_plugin") == 0 || - strcmp(var_name, (char*)"admin-admin_credentials") == 0; + if (var_name == NULL) { + return false; + } + return strstr(var_name, "password") || + strcmp(var_name, "mysql-default_authentication_plugin") == 0 || + strcmp(var_name, "admin-admin_credentials") == 0; } // Returns true if the given name is either a know mysql or admin global variable. @@ -1157,16 +1160,10 @@ bool is_valid_global_variable(const char *var_name) { // It modifies the original query. template bool admin_handler_command_set(char *query_no_space, unsigned int query_no_space_length, S* sess, ProxySQL_Admin *pa, char **q, unsigned int *ql) { - // Get a pointer to the beginnig of var=value entry and split to get var name and value - char *set_entry = query_no_space + strlen("SET "); - char *untrimmed_var_name=NULL; - char *var_value=NULL; - c_split_2(set_entry, "=", &untrimmed_var_name, &var_value); - - // Trim spaces from var name to allow writing like 'var = value' - char *var_name = trim_spaces_in_place(untrimmed_var_name); - - if (!strstr(query_no_space, (char *)"password") && !is_sensitive_set_variable_name(var_name)) { // issue #599 + bool skip_raw_query_log = strstr(query_no_space, (char *)"password") || + strstr(query_no_space, (char *)"admin-admin_credentials") || + strstr(query_no_space, (char *)"mysql-default_authentication_plugin"); + if (!skip_raw_query_log) { // issue #599 proxy_debug(PROXY_DEBUG_ADMIN, 4, "Received command %s\n", query_no_space); if (strncasecmp(query_no_space,(char *)"set autocommit",strlen((char *)"set autocommit"))) { if (strncasecmp(query_no_space,(char *)"SET @@session.autocommit",strlen((char *)"SET @@session.autocommit"))) { @@ -1177,6 +1174,15 @@ bool admin_handler_command_set(char *query_no_space, unsigned int query_no_space } } + // Get a pointer to the beginning of var=value entry and split to get var name and value + char *set_entry = query_no_space + strlen("SET "); + char *untrimmed_var_name=NULL; + char *var_value=NULL; + c_split_2(set_entry, "=", &untrimmed_var_name, &var_value); + + // Trim spaces from var name to allow writing like 'var = value' + char *var_name = trim_spaces_in_place(untrimmed_var_name); + if (is_sensitive_set_variable_name(var_name)) { proxy_info("Received SET command for %s\n", var_name); } diff --git a/test/tap/tests/admin_set_credentials_logging-t.cpp b/test/tap/tests/admin_set_credentials_logging-t.cpp index 2beca9d03..f81956b8c 100644 --- a/test/tap/tests/admin_set_credentials_logging-t.cpp +++ b/test/tap/tests/admin_set_credentials_logging-t.cpp @@ -10,7 +10,13 @@ using std::fstream; using std::string; +static constexpr int MAX_LOG_CHECK_ATTEMPTS = 20; +static constexpr useconds_t LOG_CHECK_RETRY_DELAY_US = 100000; + static string escape_sql_string(MYSQL* mysql, const string& value) { + if (value.empty()) { + return ""; + } string escaped(value.size() * 2 + 1, '\0'); unsigned long escaped_len = mysql_real_escape_string(mysql, &escaped[0], value.c_str(), value.size()); escaped.resize(escaped_len); @@ -20,7 +26,7 @@ static string escape_sql_string(MYSQL* mysql, const string& value) { int main() { CommandLine cl; - plan(6); + plan(7); if (cl.getEnv()) { return exit_status(); @@ -42,19 +48,26 @@ int main() { int show_res = show_admin_global_variable(admin, "admin-admin_credentials", original_admin_credentials); ok(show_res == 0, "Fetched original admin-admin_credentials"); - const string log_path { get_env("REGULAR_INFRA_DATADIR") + "/proxysql.log" }; + const string log_dir { get_env("REGULAR_INFRA_DATADIR") }; + ok(!log_dir.empty(), "REGULAR_INFRA_DATADIR is set"); + const string log_path { log_dir + "/proxysql.log" }; fstream proxysql_log {}; - int log_res = open_file_and_seek_end(log_path, proxysql_log); + int log_res = log_dir.empty() ? EXIT_FAILURE : open_file_and_seek_end(log_path, proxysql_log); ok(log_res == EXIT_SUCCESS, "Opened ProxySQL log"); bool leaked_secret = false; bool logged_sensitive_set = false; if (show_res == 0 && log_res == EXIT_SUCCESS) { + struct timespec ts {}; + clock_gettime(CLOCK_REALTIME, &ts); const string unique_secret { - "copilot-secret-" + std::to_string(getpid()) + "-" + std::to_string(static_cast(time(NULL))) + "admin-test-secret-" + std::to_string(getpid()) + "-" + std::to_string(ts.tv_sec) + + "-" + std::to_string(ts.tv_nsec) }; const string updated_admin_credentials { original_admin_credentials + ";copilot:" + unique_secret }; - const string set_query { "SET admin-admin_credentials='" + updated_admin_credentials + "'" }; + const string set_query { + "SET admin-admin_credentials='" + escape_sql_string(admin, updated_admin_credentials) + "'" + }; int set_res = mysql_query(admin, set_query.c_str()); ok(set_res == 0, "Updated admin-admin_credentials"); @@ -67,7 +80,7 @@ int main() { "Stored admin-admin_credentials contains the new secret"); string log_line {}; - for (int attempt = 0; attempt < 20; ++attempt) { + for (int attempt = 0; attempt < MAX_LOG_CHECK_ATTEMPTS; ++attempt) { proxysql_log.clear(proxysql_log.rdstate() & ~std::ios_base::failbit); while (getline(proxysql_log, log_line)) { if (log_line.find(unique_secret) != string::npos) { @@ -80,7 +93,7 @@ int main() { if (leaked_secret || logged_sensitive_set) { break; } - usleep(100000); + usleep(LOG_CHECK_RETRY_DELAY_US); } } else { ok(false, "Updated admin-admin_credentials");