chore: Log connection context when issuing dangerous cmds (#3352)

* chore: Log connection context when issuing dangerous cmds

* raise VLOG level

src/server/command_registry.cc

@@ -176,6 +176,8 @@ const char* OptName(CO::CommandOpt fl) {
       return "fast";
     case LOADING:
       return "loading";
+    case DANGEROUS:
+      return "dangerous";
     case ADMIN:
       return "admin";
     case NOSCRIPT:

src/server/command_registry.h

@@ -27,7 +27,7 @@ enum CommandOpt : uint32_t {
   LOADING = 1U << 3,  // Command allowed during LOADING state.
   DENYOOM = 1U << 4,  // use-memory in redis.
 
-  // UNUSED = 1U << 5,
+  DANGEROUS = 1U << 5,  // Dangerous commands are logged when used
 
   VARIADIC_KEYS = 1U << 6,  // arg 2 determines number of keys. Relevant for ZUNIONSTORE, EVAL etc.
 

src/server/main_service.cc

@@ -244,7 +244,7 @@ void SendMonitor(const std::string& msg) {
   const auto& monitor_repo = ServerState::tlocal()->Monitors();
   const auto& monitors = monitor_repo.monitors();
   if (!monitors.empty()) {
-    VLOG(1) << "thread " << ProactorBase::me()->GetPoolIndex() << " sending monitor message '"
+    VLOG(2) << "Thread " << ProactorBase::me()->GetPoolIndex() << " sending monitor message '"
             << msg << "' for " << monitors.size();
 
     for (auto monitor_conn : monitors) {
@@ -258,7 +258,7 @@ void DispatchMonitor(ConnectionContext* cntx, const CommandId* cid, CmdArgList t
   //  We have connections waiting to get the info on the last command, send it to them
   string monitor_msg = MakeMonitorMessage(cntx, cid, tail_args);
 
-  VLOG(1) << "sending command '" << monitor_msg << "' to the clients that registered on it";
+  VLOG(2) << "Sending command '" << monitor_msg << "' to the clients that registered on it";
 
   shard_set->pool()->DispatchBrief(
       [msg = std::move(monitor_msg)](unsigned idx, util::ProactorBase*) { SendMonitor(msg); });
@@ -795,6 +795,13 @@ struct BorrowedInterpreter {
   bool owned_ = false;
 };
 
+string ConnectionLogContext(const facade::Connection* conn) {
+  if (conn == nullptr) {
+    return "(null-conn)";
+  }
+  return absl::StrCat("(", conn->RemoteEndpointStr(), ")");
+}
+
 }  // namespace
 
 Service::Service(ProactorPool* pp)
@@ -1035,6 +1042,8 @@ std::optional<ErrorReply> Service::VerifyCommandState(const CommandId* cid, CmdA
   // If there is no connection owner, it means the command it being called
   // from another command or used internally, therefore is always permitted.
   if (dfly_cntx.conn() != nullptr && !dfly_cntx.conn()->IsPrivileged() && cid->IsRestricted()) {
+    VLOG(1) << "Non-admin attempt to execute " << cid->name() << " "
+            << ConnectionLogContext(dfly_cntx.conn());
     return ErrorReply{"Cannot execute restricted command (admin only)"};
   }
 
@@ -1182,6 +1191,10 @@ void Service::DispatchCommand(CmdArgList args, facade::ConnectionContext* cntx)
     return;
   }
 
+  VLOG_IF(1, cid->opt_mask() & CO::CommandOpt::DANGEROUS)
+      << "Executing dangerous command " << cid->name() << " "
+      << ConnectionLogContext(dfly_cntx->conn());
+
   bool is_trans_cmd = CO::IsTransKind(cid->name());
   if (dfly_cntx->conn_state.exec_info.IsCollecting() && !is_trans_cmd) {
     // TODO: protect against aggregating huge transactions.
@@ -1993,7 +2006,7 @@ void Service::EvalInternal(CmdArgList args, const EvalArgs& eval_args, Interpret
     });
 
     if (*sid != ServerState::tlocal()->thread_index()) {
-      VLOG(1) << "Migrating connection " << cntx->conn() << " from "
+      VLOG(2) << "Migrating connection " << cntx->conn() << " from "
               << ProactorBase::me()->GetPoolIndex() << " to " << *sid;
       cntx->conn()->RequestAsyncMigration(shard_set->pool()->at(*sid));
     }
@@ -2192,7 +2205,7 @@ void Service::Exec(CmdArgList args, ConnectionContext* cntx) {
 
   exec_info.state = ConnectionState::ExecInfo::EXEC_RUNNING;
 
-  VLOG(1) << "StartExec " << exec_info.body.size();
+  VLOG(2) << "StartExec " << exec_info.body.size();
 
   // Make sure we flush whatever responses we aggregated in the reply builder.
   SinkReplyBuilder::ReplyAggregator agg(rb);

src/server/server_family.cc

@@ -2942,11 +2942,13 @@ void ServerFamily::Register(CommandRegistry* registry) {
       << CI{"AUTH", CO::NOSCRIPT | CO::FAST | CO::LOADING, -2, 0, 0, acl::kAuth}.HFUNC(Auth)
       << CI{"BGSAVE", CO::ADMIN | CO::GLOBAL_TRANS, -1, 0, 0, acl::kBGSave}.HFUNC(BgSave)
       << CI{"CLIENT", CO::NOSCRIPT | CO::LOADING, -2, 0, 0, acl::kClient}.HFUNC(Client)
-      << CI{"CONFIG", CO::ADMIN, -2, 0, 0, acl::kConfig}.HFUNC(Config)
+      << CI{"CONFIG", CO::ADMIN | CO::DANGEROUS, -2, 0, 0, acl::kConfig}.HFUNC(Config)
       << CI{"DBSIZE", CO::READONLY | CO::FAST | CO::LOADING, 1, 0, 0, acl::kDbSize}.HFUNC(DbSize)
       << CI{"DEBUG", CO::ADMIN | CO::LOADING, -2, 0, 0, acl::kDebug}.HFUNC(Debug)
-      << CI{"FLUSHDB", CO::WRITE | CO::GLOBAL_TRANS, 1, 0, 0, acl::kFlushDB}.HFUNC(FlushDb)
-      << CI{"FLUSHALL", CO::WRITE | CO::GLOBAL_TRANS, -1, 0, 0, acl::kFlushAll}.HFUNC(FlushAll)
+      << CI{"FLUSHDB", CO::WRITE | CO::GLOBAL_TRANS | CO::DANGEROUS, 1, 0, 0, acl::kFlushDB}.HFUNC(
+             FlushDb)
+      << CI{"FLUSHALL", CO::WRITE | CO::GLOBAL_TRANS | CO::DANGEROUS, -1, 0, 0, acl::kFlushAll}
+             .HFUNC(FlushAll)
       << CI{"INFO", CO::LOADING, -1, 0, 0, acl::kInfo}.HFUNC(Info)
       << CI{"HELLO", CO::LOADING, -1, 0, 0, acl::kHello}.HFUNC(Hello)
       << CI{"LASTSAVE", CO::LOADING | CO::FAST, 1, 0, 0, acl::kLastSave}.HFUNC(LastSave)
@@ -2954,8 +2956,9 @@ void ServerFamily::Register(CommandRegistry* registry) {
              Latency)
       << CI{"MEMORY", kMemOpts, -2, 0, 0, acl::kMemory}.HFUNC(Memory)
       << CI{"SAVE", CO::ADMIN | CO::GLOBAL_TRANS, -1, 0, 0, acl::kSave}.HFUNC(Save)
-      << CI{"SHUTDOWN", CO::ADMIN | CO::NOSCRIPT | CO::LOADING, -1, 0, 0, acl::kShutDown}.HFUNC(
-             ShutdownCmd)
+      << CI{"SHUTDOWN",    CO::ADMIN | CO::NOSCRIPT | CO::LOADING | CO::DANGEROUS, -1, 0, 0,
+            acl::kShutDown}
+             .HFUNC(ShutdownCmd)
       << CI{"SLAVEOF", kReplicaOpts, 3, 0, 0, acl::kSlaveOf}.HFUNC(ReplicaOf)
       << CI{"REPLICAOF", kReplicaOpts, -3, 0, 0, acl::kReplicaOf}.HFUNC(ReplicaOf)
       << CI{"ADDREPLICAOF", kReplicaOpts, 5, 0, 0, acl::kReplicaOf}.HFUNC(AddReplicaOf)