This is an automated email from the ASF dual-hosted git repository.
hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new d0a0d7124 fix(command): MONITOR command should redact sensitive tokens
(#3193)
d0a0d7124 is described below
commit d0a0d712434e7fc56b8a4f0acb8ea2e0159cd85a
Author: hulk <[email protected]>
AuthorDate: Sat Sep 20 21:39:15 2025 +0800
fix(command): MONITOR command should redact sensitive tokens (#3193)
---
src/commands/cmd_function.cc | 4 +-
src/commands/cmd_script.cc | 12 ++-
src/commands/cmd_server.cc | 4 +-
src/commands/commander.h | 5 +
src/server/redis_connection.cc | 4 +-
src/server/server.cc | 36 +++++++-
src/server/server.h | 1 +
src/storage/scripting.cc | 4 +-
.../unit/introspection/introspection_test.go | 11 ---
tests/gocase/unit/monitor/monitor_test.go | 102 +++++++++++++++++++++
10 files changed, 160 insertions(+), 23 deletions(-)
diff --git a/src/commands/cmd_function.cc b/src/commands/cmd_function.cc
index 73fc24c4b..3709a74ab 100644
--- a/src/commands/cmd_function.cc
+++ b/src/commands/cmd_function.cc
@@ -130,7 +130,7 @@ uint64_t GenerateFCallFlags(uint64_t flags, const
std::vector<std::string> &, co
REDIS_REGISTER_COMMANDS(
Function, MakeCmdAttr<CommandFunction>("function", -2, "exclusive
no-script", NO_KEY, GenerateFunctionFlags),
- MakeCmdAttr<CommandFCall<>>("fcall", -3, "write no-script",
GetScriptEvalKeyRange, GenerateFCallFlags),
- MakeCmdAttr<CommandFCall<true>>("fcall_ro", -3, "read-only no-script",
GetScriptEvalKeyRange));
+ MakeCmdAttr<CommandFCall<>>("fcall", -3, "write no-script skip-monitor",
GetScriptEvalKeyRange, GenerateFCallFlags),
+ MakeCmdAttr<CommandFCall<true>>("fcall_ro", -3, "read-only no-script
skip-monitor", GetScriptEvalKeyRange));
} // namespace redis
diff --git a/src/commands/cmd_script.cc b/src/commands/cmd_script.cc
index ea6a37d54..e9a05fecc 100644
--- a/src/commands/cmd_script.cc
+++ b/src/commands/cmd_script.cc
@@ -134,10 +134,12 @@ uint64_t GenerateEvalFlags(uint64_t flags, const
std::vector<std::string> &, con
}
REDIS_REGISTER_COMMANDS(
- Script, MakeCmdAttr<CommandEval>("eval", -3, "write no-script",
GetScriptEvalKeyRange, GenerateEvalFlags),
- MakeCmdAttr<CommandEvalSHA>("evalsha", -3, "write no-script",
GetScriptEvalKeyRange, GenerateEvalFlags),
- MakeCmdAttr<CommandEvalRO>("eval_ro", -3, "read-only no-script",
GetScriptEvalKeyRange),
- MakeCmdAttr<CommandEvalSHARO>("evalsha_ro", -3, "read-only no-script",
GetScriptEvalKeyRange),
- MakeCmdAttr<CommandScript>("script", -2, "exclusive no-script", NO_KEY,
GenerateScriptFlags), )
+ Script,
+ MakeCmdAttr<CommandEval>("eval", -3, "write no-script skip-monitor",
GetScriptEvalKeyRange, GenerateEvalFlags),
+ MakeCmdAttr<CommandEvalSHA>("evalsha", -3, "write no-script skip-monitor",
GetScriptEvalKeyRange,
+ GenerateEvalFlags),
+ MakeCmdAttr<CommandEvalRO>("eval_ro", -3, "read-only no-script
skip-monitor", GetScriptEvalKeyRange),
+ MakeCmdAttr<CommandEvalSHARO>("evalsha_ro", -3, "read-only no-script
skip-monitor", GetScriptEvalKeyRange),
+ MakeCmdAttr<CommandScript>("script", -2, "exclusive no-script
skip-monitor", NO_KEY, GenerateScriptFlags), )
} // namespace redis
diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc
index e90487d67..212d44a49 100644
--- a/src/commands/cmd_server.cc
+++ b/src/commands/cmd_server.cc
@@ -1555,8 +1555,8 @@ REDIS_REGISTER_COMMANDS(
MakeCmdAttr<CommandSelect>("select", 2, "read-only", NO_KEY),
MakeCmdAttr<CommandInfo>("info", -1, "read-only ok-loading", NO_KEY),
MakeCmdAttr<CommandRole>("role", 1, "read-only ok-loading", NO_KEY),
- MakeCmdAttr<CommandConfig>("config", -2, "read-only admin", NO_KEY,
GenerateConfigFlag),
- MakeCmdAttr<CommandNamespace>("namespace", -2, "read-only admin", NO_KEY,
GenerateNamespaceFlag),
+ MakeCmdAttr<CommandConfig>("config", -2, "read-only admin skip-monitor",
NO_KEY, GenerateConfigFlag),
+ MakeCmdAttr<CommandNamespace>("namespace", -2, "read-only admin
skip-monitor", NO_KEY, GenerateNamespaceFlag),
MakeCmdAttr<CommandKeys>("keys", 2, "read-only slow", NO_KEY),
MakeCmdAttr<CommandFlushDB>("flushdb", 1, "write no-dbsize-check
exclusive", NO_KEY),
MakeCmdAttr<CommandFlushAll>("flushall", 1, "write no-dbsize-check
exclusive admin", NO_KEY),
diff --git a/src/commands/commander.h b/src/commands/commander.h
index 61a3b79d5..7fede573f 100644
--- a/src/commands/commander.h
+++ b/src/commands/commander.h
@@ -87,6 +87,8 @@ enum CommandFlags : uint64_t {
kCmdAuth = 1ULL << 10,
// "admin" flag, for commands that require admin permission
kCmdAdmin = 1ULL << 11,
+ // "skip-monitor" flag, for commands that should skip monitor feed
+ kCmdSkipMonitor = 1ULL << 12,
};
enum class CommandCategory : uint8_t {
@@ -352,6 +354,8 @@ inline uint64_t ParseCommandFlags(const std::string
&description, const std::str
flags |= kCmdNoMulti;
else if (flag == "no-script")
flags |= kCmdNoScript;
+ else if (flag == "skip-monitor")
+ flags |= kCmdSkipMonitor;
else if (flag == "no-dbsize-check")
flags |= kCmdNoDBSizeCheck;
else if (flag == "slow")
@@ -388,6 +392,7 @@ inline std::vector<std::string>
CommandAttributes::FlagsToString(uint64_t flags)
if (flags & kCmdBlocking) res.emplace_back("blocking");
if (flags & kCmdAuth) res.emplace_back("auth");
if (flags & kCmdAdmin) res.emplace_back("admin");
+ if (flags & kCmdSkipMonitor) res.emplace_back("skip-monitor");
return res;
}
diff --git a/src/server/redis_connection.cc b/src/server/redis_connection.cc
index 06a1a4559..e61548689 100644
--- a/src/server/redis_connection.cc
+++ b/src/server/redis_connection.cc
@@ -573,7 +573,9 @@ void Connection::ExecuteCommands(std::deque<CommandTokens>
*to_process_cmds) {
}
}
- srv_->FeedMonitorConns(this, cmd_tokens);
+ if (!(cmd_flags & redis::kCmdSkipMonitor)) {
+ srv_->FeedMonitorConns(this, cmd_tokens);
+ }
// Break the execution loop when occurring the blocking command like BLPOP
or BRPOP,
// it will suspend the connection and wait for the wakeup signal.
diff --git a/src/server/server.cc b/src/server/server.cc
index 6ca401438..04e62febc 100644
--- a/src/server/server.cc
+++ b/src/server/server.cc
@@ -370,13 +370,47 @@ void Server::CleanupExitedSlaves() {
}
}
+std::vector<std::string> Server::RedactSensitiveTokens(const
std::vector<std::string> &tokens) {
+ if (tokens.empty()) return tokens;
+ std::string cmd = util::ToLower(tokens[0]);
+ if (cmd != "auth" && cmd != "hello") return tokens;
+
+ std::vector<std::string> redacted_tokens = tokens;
+ if (cmd == "auth" && tokens.size() >= 2) {
+ // AUTH password -> redact password (arg 1)
+ redacted_tokens[1] = "(redacted)";
+ } else if (cmd == "hello" && tokens.size() >= 3) {
+ // HELLO [version] [AUTH [username] password] [SETNAME name]
+ for (size_t i = 1; i < tokens.size(); ++i) {
+ std::string arg = util::ToLower(tokens[i]);
+ if (arg == "auth" && i + 1 < tokens.size()) {
+ size_t remaining_args = tokens.size() - i - 1;
+ if (remaining_args >= 2) {
+ // Check if this follows the pattern AUTH username password
+ // In this case, redact the password (arg i+2)
+ redacted_tokens[i + 2] = "(redacted)";
+ } else if (remaining_args == 1) {
+ // This follows the pattern AUTH password
+ // Redact the password (arg i+1)
+ redacted_tokens[i + 1] = "(redacted)";
+ }
+ break;
+ }
+ }
+ }
+ return redacted_tokens;
+}
+
void Server::FeedMonitorConns(redis::Connection *conn, const
std::vector<std::string> &tokens) {
if (monitor_clients_ <= 0) return;
auto now_us = util::GetTimeStampUS();
std::string output =
fmt::format("{}.{} [{} {}]", now_us / 1000000, now_us % 1000000,
conn->GetNamespace(), conn->GetAddr());
- for (const auto &token : tokens) {
+
+ auto redacted_tokens = RedactSensitiveTokens(tokens);
+
+ for (const auto &token : redacted_tokens) {
output += " \"";
output += util::EscapeString(token);
output += "\"";
diff --git a/src/server/server.h b/src/server/server.h
index 54f4c2cc4..7063a6632 100644
--- a/src/server/server.h
+++ b/src/server/server.h
@@ -203,6 +203,7 @@ class Server {
void CleanupExitedSlaves();
bool IsSlave() const { return !master_host_.empty(); }
void FeedMonitorConns(redis::Connection *conn, const
std::vector<std::string> &tokens);
+ static std::vector<std::string> RedactSensitiveTokens(const
std::vector<std::string> &tokens);
void IncrFetchFileThread() { fetch_file_threads_num_++; }
void DecrFetchFileThread() { fetch_file_threads_num_--; }
int GetFetchFileThreadNum() const { return fetch_file_threads_num_; }
diff --git a/src/storage/scripting.cc b/src/storage/scripting.cc
index ff97d253c..c8ddfc667 100644
--- a/src/storage/scripting.cc
+++ b/src/storage/scripting.cc
@@ -884,7 +884,9 @@ int RedisGenericCommand(lua_State *lua, int raise_error) {
return raise_error ? RaiseError(lua) : 1;
}
- srv->FeedMonitorConns(conn, args);
+ if (!(cmd_flags & redis::kCmdSkipMonitor)) {
+ srv->FeedMonitorConns(conn, args);
+ }
RedisProtocolToLuaType(lua, output.data());
return 1;
diff --git a/tests/gocase/unit/introspection/introspection_test.go
b/tests/gocase/unit/introspection/introspection_test.go
index 1b17a98ca..f8b0874b5 100644
--- a/tests/gocase/unit/introspection/introspection_test.go
+++ b/tests/gocase/unit/introspection/introspection_test.go
@@ -118,17 +118,6 @@ func TestIntrospection(t *testing.T) {
require.Regexp(t, "id=.* addr=.*:.* fd=.* name=.* age=.*
idle=.* flags=N namespace=.* qbuf=.* .*obuf=.* cmd=client.*", v)
})
- t.Run("MONITOR can log executed commands", func(t *testing.T) {
- c := srv.NewTCPClient()
- defer func() { require.NoError(t, c.Close()) }()
- require.NoError(t, c.WriteArgs("MONITOR"))
- c.MustRead(t, "+OK")
- require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
- require.NoError(t, rdb.Get(ctx, "foo").Err())
- c.MustMatch(t, ".*set.*foo.*bar.*")
- c.MustMatch(t, ".*get.*foo.*")
- })
-
t.Run("CLIENT GETNAME should return NIL if name is not assigned",
func(t *testing.T) {
require.EqualError(t, rdb.ClientGetName(ctx).Err(),
redis.Nil.Error())
})
diff --git a/tests/gocase/unit/monitor/monitor_test.go
b/tests/gocase/unit/monitor/monitor_test.go
new file mode 100644
index 000000000..a76a84a52
--- /dev/null
+++ b/tests/gocase/unit/monitor/monitor_test.go
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package monitor
+
+import (
+ "context"
+ "testing"
+
+ "github.com/redis/go-redis/v9"
+ "github.com/stretchr/testify/require"
+
+ "github.com/apache/kvrocks/tests/gocase/util"
+)
+
+func TestMonitor(t *testing.T) {
+ srv := util.StartServer(t, map[string]string{})
+ defer srv.Close()
+
+ ctx := context.Background()
+ rdb := srv.NewClient()
+ defer func() { require.NoError(t, rdb.Close()) }()
+
+ t.Run("MONITOR can log executed commands", func(t *testing.T) {
+ c := srv.NewTCPClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.WriteArgs("MONITOR"))
+ c.MustRead(t, "+OK")
+ require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
+ require.NoError(t, rdb.Get(ctx, "foo").Err())
+ c.MustMatch(t, ".*hello.*")
+ c.MustMatch(t, ".*set.*foo.*bar.*")
+ c.MustMatch(t, ".*get.*foo.*")
+ })
+
+ t.Run("MONITOR should skip commands with 'skip-monitor' flag", func(t
*testing.T) {
+ c := srv.NewTCPClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.WriteArgs("MONITOR"))
+ c.MustRead(t, "+OK")
+
+ // Execute commands that should be skipped
+ require.NoError(t, rdb.ConfigSet(ctx, "slave-read-only",
"yes").Err())
+ require.NoError(t, rdb.ConfigGet(ctx, "slave-read-only").Err())
+
+ require.NoError(t, rdb.Ping(ctx).Err())
+ require.NoError(t, rdb.FlushAll(ctx).Err())
+ require.NoError(t, rdb.Set(ctx, "test", "value", 0).Err())
+ require.NoError(t, rdb.Get(ctx, "test").Err())
+ require.NoError(t, rdb.Info(ctx, "server").Err())
+ c.MustMatch(t, ".*ping.*")
+ c.MustMatch(t, ".*flushall.*")
+ c.MustMatch(t, ".*set.*test.*value.*")
+ c.MustMatch(t, ".*get.*test.*")
+ c.MustMatch(t, ".*info.*server.*")
+ })
+}
+
+func TestMonitorRedactPassword(t *testing.T) {
+ srv := util.StartServer(t, map[string]string{"requirepass": "testpass"})
+ defer srv.Close()
+
+ c := srv.NewTCPClient()
+ defer func() { require.NoError(t, c.Close()) }()
+ require.NoError(t, c.WriteArgs("AUTH", "testpass"))
+ c.MustRead(t, "+OK")
+
+ require.NoError(t, c.WriteArgs("MONITOR"))
+ c.MustRead(t, "+OK")
+
+ rdb := srv.NewClientWithOption(&redis.Options{
+ Password: "testpass",
+ })
+ defer func() { require.NoError(t, rdb.Close()) }()
+
+ ctx := context.Background()
+ require.NoError(t, rdb.Do(ctx, "AUTH", "testpass").Err())
+ require.NoError(t, rdb.Do(ctx, "HELLO", "3", "AUTH", "testpass").Err())
+ require.NoError(t, rdb.Do(ctx, "HELLO", "3", "AUTH", "default",
"testpass").Err())
+
+ // Client uses HELLO to AUTH, so it will have an extra HELLO command
+ c.MustMatch(t, `.*hello.*3.*auth.*default.*\(redacted\).*`)
+ c.MustMatch(t, `.*AUTH.*\(redacted\).*`)
+ c.MustMatch(t, `.*HELLO.*3.*AUTH.*\(redacted\).*`)
+ c.MustMatch(t, `.*HELLO.*3.*AUTH.*default.*\(redacted\).*`)
+}
