Fix comments

config should be boolean
leave room to test each argument based on command type

Signed-off-by: naglera <[email protected]>

src/config.c

@@ -3106,6 +3106,7 @@ standardConfig static_configs[] = {
     createBoolConfig("extended-redis-compatibility", NULL, MODIFIABLE_CONFIG, server.extended_redis_compat, 0, NULL, updateExtendedRedisCompat),
     createBoolConfig("enable-debug-assert", NULL, IMMUTABLE_CONFIG | HIDDEN_CONFIG, server.enable_debug_assert, 0, NULL, NULL),
     createBoolConfig("cluster-slot-stats-enabled", NULL, MODIFIABLE_CONFIG, server.cluster_slot_stats_enabled, 0, NULL, NULL),
+    createBoolConfig("hide-user-data-from-log", NULL, MODIFIABLE_CONFIG, server.hide_user_data_from_log, 0, NULL, NULL),
 
     /* String Configs */
     createStringConfig("aclfile", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.acl_filename, "", NULL, NULL),
@@ -3198,7 +3199,6 @@ standardConfig static_configs[] = {
     createIntConfig("watchdog-period", NULL, MODIFIABLE_CONFIG | HIDDEN_CONFIG, 0, INT_MAX, server.watchdog_period, 0, INTEGER_CONFIG, NULL, updateWatchdogPeriod),
     createIntConfig("shutdown-timeout", NULL, MODIFIABLE_CONFIG, 0, INT_MAX, server.shutdown_timeout, 10, INTEGER_CONFIG, NULL, NULL),
     createIntConfig("repl-diskless-sync-max-replicas", NULL, MODIFIABLE_CONFIG, 0, INT_MAX, server.repl_diskless_sync_max_replicas, 0, INTEGER_CONFIG, NULL, NULL),
-    createIntConfig("hide-user-data-from-log", NULL, MODIFIABLE_CONFIG, -1, INT_MAX, server.hide_user_data_from_log, 0, INTEGER_CONFIG, NULL, NULL),
 
     /* Unsigned int configs */
     createUIntConfig("maxclients", NULL, MODIFIABLE_CONFIG, 1, UINT_MAX, server.maxclients, 10000, INTEGER_CONFIG, NULL, updateMaxclients),

src/debug.c

@@ -1031,12 +1031,14 @@ __attribute__((noinline)) void _serverAssert(const char *estr, const char *file,
     bugReportEnd(0, 0);
 }
 
-/* Checks if logging the provided argument is permitted based on the current
- * configuration for hiding user data from logs. */
-int canLogClientArg(const client *c, int idx) {
+/* Checks if the argument at the given index should be redacted from logs. */
+int shouldRedactArg(const client *c, int idx) {
     serverAssert(idx < c->argc);
-    if (server.hide_user_data_from_log < 0) return 1;
-    return idx < server.hide_user_data_from_log;
+    /* Don't redact if the config is disabled */
+    if (!server.hide_user_data_from_log) return 0;
+    /* first_sensitive_arg_idx value should be changed based on the command type. */
+    int first_sensitive_arg_idx = 1;
+    return idx >= first_sensitive_arg_idx;
 }
 
 void _serverAssertPrintClientInfo(const client *c) {
@@ -1049,7 +1051,7 @@ void _serverAssertPrintClientInfo(const client *c) {
     serverLog(LL_WARNING, "client->conn = %s", connGetInfo(c->conn, conninfo, sizeof(conninfo)));
     serverLog(LL_WARNING, "client->argc = %d", c->argc);
     for (j = 0; j < c->argc; j++) {
-        if (!canLogClientArg(c, j)) {
+        if (!shouldRedactArg(c, j)) {
             serverLog(LL_WARNING | LL_RAW, "client->argv[%d]: %zu bytes ", j, sdslen((sds)c->argv[j]->ptr));
             continue;
         }
@@ -1253,7 +1255,7 @@ static void *getAndSetMcontextEip(ucontext_t *uc, void *eip) {
 
 VALKEY_NO_SANITIZE("address")
 void logStackContent(void **sp) {
-    if (server.hide_user_data_from_log > 0) {
+    if (server.hide_user_data_from_log) {
         serverLog(LL_NOTICE, "hide-user-data-from-log is on, skip logging stack content to avoid spilling PII.");
         return;
     }
@@ -1872,7 +1874,7 @@ void logCurrentClient(client *cc, const char *title) {
     sdsfree(client);
     serverLog(LL_WARNING | LL_RAW, "argc: '%d'\n", cc->argc);
     for (j = 0; j < cc->argc; j++) {
-        if (!canLogClientArg(cc, j)) {
+        if (!shouldRedactArg(cc, j)) {
             serverLog(LL_WARNING | LL_RAW, "client->argv[%d]: %zu bytes ", j, sdslen((sds)cc->argv[j]->ptr));
             continue;
         }

src/networking.c

@@ -3277,14 +3277,13 @@ sds catClientInfoString(sds s, client *client) {
         used_blocks_of_repl_buf = last->id - cur->id + 1;
     }
 
-    int hide_data = server.hide_user_data_from_log > 0;
     /* clang-format off */
     sds ret = sdscatfmt(s, FMTARGS(
         "id=%U", (unsigned long long) client->id,
         " addr=%s", getClientPeerId(client),
         " laddr=%s", getClientSockname(client),
         " %s", connGetInfo(client->conn, conninfo, sizeof(conninfo)),
-        " name=%s", hide_data ? "*redacted*" : (client->name ? (char*)client->name->ptr : ""),
+        " name=%s", server.hide_user_data_from_log ? "*redacted*" : (client->name ? (char*)client->name->ptr : ""),
         " age=%I", (long long)(commandTimeSnapshot() / 1000 - client->ctime),
         " idle=%I", (long long)(server.unixtime - client->last_interaction),
         " flags=%s", flags,
@@ -3306,7 +3305,7 @@ sds catClientInfoString(sds s, client *client) {
         " tot-mem=%U", (unsigned long long) total_mem,
         " events=%s", events,
         " cmd=%s", client->lastcmd ? client->lastcmd->fullname : "NULL",
-        " user=%s", hide_data ? "*redacted*" : (client->user ? client->user->name : "(superuser)"),
+        " user=%s", server.hide_user_data_from_log ? "*redacted*" : (client->user ? client->user->name : "(superuser)"),
         " redir=%I", (client->flag.tracking) ? (long long) client->client_tracking_redirection : -1,
         " resp=%i", client->resp,
         " lib-name=%s", client->lib_name ? (char*)client->lib_name->ptr : "",

src/replication.c

@@ -614,7 +614,7 @@ void replicationFeedReplicas(int dictid, robj **argv, int argc) {
 void showLatestBacklog(void) {
     if (server.repl_backlog == NULL) return;
     if (listLength(server.repl_buffer_blocks) == 0) return;
-    if (server.hide_user_data_from_log > 0) {
+    if (server.hide_user_data_from_log) {
         serverLog(LL_NOTICE, "hide-user-data-from-log is on, skip logging backlog content to avoid spilling user data.");
         return;
     }
@@ -649,7 +649,7 @@ void replicationFeedStreamFromPrimaryStream(char *buf, size_t buflen) {
     /* Debugging: this is handy to see the stream sent from primary
      * to replicas. Disabled with if(0). */
     if (0) {
-        if (server.hide_user_data_from_log < 0) {
+        if (server.hide_user_data_from_log) {
             printf("%zu:", buflen);
             for (size_t j = 0; j < buflen; j++) {
                 printf("%c", isprint(buf[j]) ? buf[j] : '.');

valkey.conf

@@ -389,8 +389,8 @@ always-show-logo no
 # To prevent sensitive user information, such as PII, from being recorded in the 
 # server log file, this feature is enabled by default. If you need to log user 
 # data for debugging or troubleshooting purposes, you can disable this feature
-# by changing the config value to -1.
-hide-user-data-from-log 1
+# by changing the config value to no.
+hide-user-data-from-log yes
 
 # By default, the server modifies the process title (as seen in 'top' and 'ps') to
 # provide some runtime information. It is possible to disable this and leave