Merge pull request #1893 from Expensify/flo_whitelistParams

Only log whitelisted params in addLogParams

libstuff/SLog.cpp

@@ -3,6 +3,7 @@
 
 // Global logging state shared between all threads
 atomic<int> _g_SLogMask(LOG_INFO);
+atomic<bool> GLOBAL_IS_LIVE{true};
 
 void SLogStackTrace(int level) {
     // If the level isn't set in the log mask, nothing more to do
@@ -39,18 +40,30 @@ void SLogStackTrace(int level) {
     }
 }
 
+// If the param name is not in this whitelist, we will log <REDACTED> in addLogParams.
+static const set<string> PARAMS_WHITELIST = {
+    "accountID",
+    "command",
+    "indexName",
+    "isUnique",
+};
+
 string addLogParams(string&& message, const map<string, string>& params) {
     if (params.empty()) {
         return message;
     }
 
-    message += " ~~ ";
-    for (size_t i = 0; i < params.size(); ++i) {
-        if (i > 0) {
-            message += " ";
+    message += " ~~";
+    for (const auto& [key, value] : params) {
+        message += " ";
+        string valueToLog = value;
+        if (!SContains(PARAMS_WHITELIST, key)) {
+            if (!GLOBAL_IS_LIVE) {
+                STHROW("500 Log param not in the whitelist, either do not log that or add it to PARAMS_WHITELIST if it's not sensitive");
+            }
+            valueToLog = "<REDACTED>";
         }
-        const auto& param = *next(params.begin(), i);
-        message += param.first + ": '" + param.second + "'";
+        message += key + ": '" + valueToLog + "'";
     }
 
     return message;

libstuff/libstuff.h

@@ -28,6 +28,9 @@ struct SData;
 
 using namespace std;
 
+// Global indicating whether we're running the server on dev or production.
+extern atomic<bool> GLOBAL_IS_LIVE;
+
 // Initialize libstuff on every thread before calling any of its functions
 void SInitialize(string threadName = "", const char* processName = 0);
 

main.cpp

@@ -149,6 +149,8 @@ int main(int argc, char* argv[]) {
         cout << "Protip: check syslog for details, or run 'bedrock -?' for help" << endl;
     }
 
+    GLOBAL_IS_LIVE = args.isSet("-live");
+
     // Initialize the sqlite library before any other code has a chance to do anything with it.
     // Set the logging callback for sqlite errors.
     SASSERT(sqlite3_config(SQLITE_CONFIG_LOG, SQLite::_sqliteLogCallback, 0) == SQLITE_OK);

sqlitecluster/SQLite.cpp

@@ -460,7 +460,7 @@ bool SQLite::verifyTable(const string& tableName, const string& sql, bool& creat
 }
 
 bool SQLite::verifyIndex(const string& indexName, const string& tableName, const string& indexSQLDefinition, bool isUnique, bool createIfNotExists) {
-    SINFO("Verifying index", {{"indexName", indexName}, {"isUnique?", to_string(isUnique)}});
+    SINFO("Verifying index", {{"indexName", indexName}, {"isUnique", to_string(isUnique)}});
     SQResult result;
     SASSERT(read("SELECT sql FROM sqlite_master WHERE type='index' AND tbl_name=" + SQ(tableName) + " AND name=" + SQ(indexName) + ";", result));