[Server] Add per-run configuration overrides (#7820)

Add --set key=value to override a section-qualified scalar configuration key for a single run and --print-config to print the effective configuration, with secrets redacted. SettingsCache owns the override map, so reads see the override without rewriting the ini file, and rebuilds the disallowed-name regexps when users/disallowedregexp is overridden. Bare keys and array-scoped keys are rejected because array loops read them relative to the active group, where a full-key override would either be ignored or rewrite every entry.

Cover the ini, default, override, override-only-key, regexp-rebuild, secret-redaction, and parse paths with a server-labelled gtest.

servatrice/src/main.cpp

@@ -40,6 +40,7 @@
 #include <QObject>
 #include <QSharedPointer>
 #include <QString>
+#include <QStringList>
 #include <QThread>
 #include <QVariant>
 #include <QVector>
@@ -124,6 +125,37 @@ void myMessageOutput2(QtMsgType /*type*/, const QMessageLogContext &, const QStr
     std::cerr << msg.toStdString() << std::endl;
 }
 
+namespace
+{
+void printEffectiveConfig(const SettingsCache &settings)
+{
+    for (const QString &key : settings.effectiveKeys()) {
+        const QString rendered =
+            SettingsCache::isSecretKey(key) ? QStringLiteral("<redacted>") : settings.value(key).toString();
+        std::cout << key.toStdString() << '=' << rendered.toStdString() << '\n';
+    }
+}
+
+bool applyConfigOverrides(SettingsCache &settings, const QStringList &values)
+{
+    for (const QString &entry : values) {
+        QString key;
+        QVariant value;
+        if (!SettingsCache::parseOverride(entry, key, value)) {
+            qCritical() << "Invalid --set value; expected a scalar key=value:" << entry;
+            return false;
+        }
+
+        settings.setOverride(key, value);
+    }
+
+    if (!values.isEmpty()) {
+        settings.rebuildDisallowedRegExps();
+    }
+    return true;
+}
+} // namespace
+
 int main(int argc, char *argv[])
 {
     QCoreApplication app(argc, argv);
@@ -147,6 +179,17 @@ int main(int argc, char *argv[])
     QCommandLineOption configPathOpt("config", "Read server configuration from <file>", "file", "");
     parser.addOption(configPathOpt);
 
+    QCommandLineOption setConfigOpt(
+        "set",
+        "Override a section-qualified scalar configuration value for this run (key=value), such as "
+        "server/port. May be repeated; bare keys and array keys such as rooms/roomlist/1/name are "
+        "rejected; never writes the config file.",
+        "key=value");
+    parser.addOption(setConfigOpt);
+
+    QCommandLineOption printConfigOpt("print-config", "Print the effective configuration and exit");
+    parser.addOption(printConfigOpt);
+
     parser.process(app);
 
     bool testRandom = parser.isSet(testRandomOpt);
@@ -165,6 +208,15 @@ int main(int argc, char *argv[])
     qWarning() << "Using configuration file: " << configPath;
     settingsCache = new SettingsCache(configPath);
 
+    if (!applyConfigOverrides(*settingsCache, parser.values(setConfigOpt))) {
+        return EXIT_FAILURE;
+    }
+
+    if (parser.isSet(printConfigOpt)) {
+        printEffectiveConfig(*settingsCache);
+        return EXIT_SUCCESS;
+    }
+
     loggerThread = new QThread;
     loggerThread->setObjectName("logger");
     logger = new ServerLogger(logToConsole, settingsCache->value("server/writelog", 1).toBool(),

servatrice/src/settingscache.cpp

@@ -18,6 +18,78 @@ SettingsCache::SettingsCache(const QString &fileName, QSettings::Format format,
     // first, figure out if we are running in portable mode
     isPortableBuild = QFile::exists(qApp->applicationDirPath() + "/portable.dat");
 
+    rebuildDisallowedRegExps();
+}
+
+QVariant SettingsCache::value(const QString &key, const QVariant &defaultValue) const
+{
+    const auto it = overrides.constFind(key);
+    if (it != overrides.constEnd()) {
+        return it.value();
+    }
+    return QSettings::value(key, defaultValue);
+}
+
+void SettingsCache::setOverride(const QString &key, const QVariant &value)
+{
+    overrides.insert(key, value);
+}
+
+bool SettingsCache::parseOverride(const QString &entry, QString &key, QVariant &value)
+{
+    const int separator = entry.indexOf('=');
+    if (separator <= 0) {
+        return false;
+    }
+
+    const QString parsedKey = entry.left(separator).trimmed();
+    if (parsedKey.isEmpty() || !parsedKey.contains('/')) {
+        // A bare key is read relative to the active QSettings group by array
+        // loops, so one override could rewrite every room, channel, or game type.
+        return false;
+    }
+
+    const QStringList segments = parsedKey.split('/');
+    if (segments.last() == "size") {
+        return false;
+    }
+    for (const QString &segment : segments) {
+        bool isNumber = false;
+        segment.toInt(&isNumber);
+        if (isNumber) {
+            // Numeric segments identify array entries, which are read relative
+            // to the active array group and never match a full-key override.
+            return false;
+        }
+    }
+
+    key = parsedKey;
+    value = entry.mid(separator + 1);
+    return true;
+}
+
+bool SettingsCache::isSecretKey(const QString &key)
+{
+    const QString lower = key.toLower();
+    return lower.contains("password") || lower.contains("secret") || lower.contains("legacy_backup_key");
+}
+
+QStringList SettingsCache::effectiveKeys() const
+{
+    QStringList keys = allKeys();
+    for (auto it = overrides.constBegin(); it != overrides.constEnd(); ++it) {
+        if (!keys.contains(it.key())) {
+            keys.append(it.key());
+        }
+    }
+    keys.sort();
+    return keys;
+}
+
+void SettingsCache::rebuildDisallowedRegExps()
+{
+    disallowedRegExp.clear();
+
     QStringList disallowedRegExpStr = value("users/disallowedregexp", "").toString().split(",", Qt::SkipEmptyParts);
     disallowedRegExpStr.removeDuplicates();
     for (const QString &regExpStr : disallowedRegExpStr) {

servatrice/src/settingscache.h

@@ -1,10 +1,13 @@
 #ifndef SERVATRICE_SETTINGSCACHE_H
 #define SERVATRICE_SETTINGSCACHE_H
 
+#include <QHash>
 #include <QList>
 #include <QRegularExpression>
 #include <QSettings>
 #include <QString>
+#include <QStringList>
+#include <QVariant>
 #include <qtmetamacros.h>
 
 class QObject;
@@ -14,12 +17,51 @@ class SettingsCache : public QSettings
     Q_OBJECT
 private:
     bool isPortableBuild;
+    QHash<QString, QVariant> overrides;
 
 public:
     SettingsCache(const QString &fileName = "servatrice.ini",
                   QSettings::Format format = QSettings::IniFormat,
                   QObject *parent = 0);
     static QString guessConfigurationPath();
+
+    /**
+     * @brief Read a value, preferring a per-run override set with setOverride().
+     *
+     * QSettings::value() is not virtual, so this hides the base member. Every
+     * server call site reads through a SettingsCache pointer, which is why the
+     * override is visible without changing those call sites.
+     */
+    QVariant value(const QString &key, const QVariant &defaultValue = QVariant()) const;
+
+    /** @brief Override a key for this process only; never written back to the ini file. */
+    void setOverride(const QString &key, const QVariant &value);
+
+    /**
+     * @brief Parse one --set entry into a key and value.
+     *
+     * Rejects a missing '=', an empty key, and array-scoped keys (a numeric
+     * path segment or a trailing "/size"). QSettings reads array entries
+     * through beginReadArray(), which bypasses this class's value(), so an
+     * override for those keys would be silently ignored.
+     */
+    static bool parseOverride(const QString &entry, QString &key, QVariant &value);
+
+    /** @brief True when --print-config must redact this key's value. */
+    static bool isSecretKey(const QString &key);
+
+    /** @brief allKeys() plus override-only keys, sorted; the --print-config listing. */
+    QStringList effectiveKeys() const;
+
+    /** @brief Overrides in effect, for --print-config and tests. */
+    const QHash<QString, QVariant> &getOverrides() const
+    {
+        return overrides;
+    }
+
+    /** @brief Rebuild disallowedRegExp from users/disallowedregexp after overrides are applied. */
+    void rebuildDisallowedRegExps();
+
     QList<QRegularExpression> disallowedRegExp;
     bool getIsPortableBuild() const
     {

tests/settings/CMakeLists.txt

@@ -14,3 +14,15 @@ cockatrice_add_test(
 )
 
 cockatrice_add_test(NAME settings_migration_test SOURCES settings_migration_test.cpp LIBS libcockatrice_settings)
+
+cockatrice_add_test(
+  NAME
+  servatrice_settings_test
+  SOURCES
+  ${CMAKE_SOURCE_DIR}/servatrice/src/settingscache.cpp
+  servatrice_settings_test.cpp
+  INCLUDE_DIRS
+  ${CMAKE_SOURCE_DIR}/servatrice/src
+  LABELS
+  server
+)

tests/settings/servatrice_settings_test.cpp

@@ -0,0 +1,162 @@
+#include "settingscache.h"
+
+#include <QByteArray>
+#include <QCoreApplication>
+#include <QFile>
+#include <QSettings>
+#include <QString>
+#include <QTemporaryDir>
+#include <gtest/gtest.h>
+
+namespace
+{
+/**
+ * QSettings' path helpers and the portable-mode probe need a QCoreApplication.
+ * The test binary keeps one alive for the whole process.
+ */
+QCoreApplication &testApplication()
+{
+    static int argc = 1;
+    static char name[] = "servatrice_settings_test";
+    static char *argv[] = {name, nullptr};
+    static QCoreApplication application(argc, argv);
+    return application;
+}
+
+class ServatriceSettingsTest : public ::testing::Test
+{
+protected:
+    QTemporaryDir dir;
+
+    void SetUp() override
+    {
+        testApplication();
+        ASSERT_TRUE(dir.isValid());
+    }
+
+    QString configPath() const
+    {
+        return dir.filePath(QStringLiteral("servatrice.ini"));
+    }
+
+    void writeConfig(const QByteArray &contents) const
+    {
+        QFile file(configPath());
+        ASSERT_TRUE(file.open(QIODevice::WriteOnly | QIODevice::Text));
+        ASSERT_EQ(file.write(contents), contents.size());
+    }
+};
+} // namespace
+
+TEST_F(ServatriceSettingsTest, ReadsValueFromIni)
+{
+    writeConfig("[server]\nport=4747\n");
+    SettingsCache settings(configPath());
+
+    EXPECT_EQ(settings.value("server/port").toInt(), 4747);
+}
+
+TEST_F(ServatriceSettingsTest, MissingKeyFallsBackToDefault)
+{
+    writeConfig("[server]\nport=4747\n");
+    SettingsCache settings(configPath());
+
+    EXPECT_EQ(settings.value("server/missing", 1234).toInt(), 1234);
+}
+
+TEST_F(ServatriceSettingsTest, OverrideWinsWithoutTouchingTheFile)
+{
+    writeConfig("[server]\nport=4747\n");
+    SettingsCache settings(configPath());
+    settings.setOverride("server/port", 1234);
+
+    EXPECT_EQ(settings.value("server/port").toInt(), 1234);
+    // The base class still sees the file value: an override is per-run only.
+    EXPECT_EQ(static_cast<QSettings &>(settings).value("server/port").toInt(), 4747);
+}
+
+TEST_F(ServatriceSettingsTest, OverrideAppearsWhenKeyIsAbsentFromTheFile)
+{
+    writeConfig("[server]\nport=4747\n");
+    SettingsCache settings(configPath());
+    settings.setOverride("server/name", "override-only");
+
+    EXPECT_EQ(settings.value("server/name").toString(), QStringLiteral("override-only"));
+    EXPECT_FALSE(settings.allKeys().contains("server/name"));
+    EXPECT_TRUE(settings.getOverrides().contains("server/name"));
+}
+
+TEST_F(ServatriceSettingsTest, RebuildUsesTheOverriddenRegexpList)
+{
+    writeConfig("[users]\ndisallowedregexp=^bad$\n");
+    SettingsCache settings(configPath());
+    ASSERT_EQ(settings.disallowedRegExp.size(), 1);
+    EXPECT_TRUE(settings.disallowedRegExp.first().match("bad").hasMatch());
+
+    settings.setOverride("users/disallowedregexp", "^worse$");
+    settings.rebuildDisallowedRegExps();
+
+    ASSERT_EQ(settings.disallowedRegExp.size(), 1);
+    EXPECT_TRUE(settings.disallowedRegExp.first().match("worse").hasMatch());
+    EXPECT_FALSE(settings.disallowedRegExp.first().match("bad").hasMatch());
+}
+
+TEST_F(ServatriceSettingsTest, EffectiveKeysIncludeOverridesAndStaySorted)
+{
+    writeConfig("[server]\nport=4747\n");
+    SettingsCache settings(configPath());
+    settings.setOverride("server/name", "override-only");
+
+    const QStringList keys = settings.effectiveKeys();
+    EXPECT_TRUE(keys.contains("server/name"));
+    EXPECT_TRUE(keys.contains("server/port"));
+
+    QStringList sorted = keys;
+    sorted.sort();
+    EXPECT_EQ(keys, sorted);
+}
+
+TEST(ServatriceSettingsParsingTest, SecretKeysAreRecognised)
+{
+    EXPECT_TRUE(SettingsCache::isSecretKey("authentication/password"));
+    EXPECT_TRUE(SettingsCache::isSecretKey("database/password"));
+    EXPECT_TRUE(SettingsCache::isSecretKey("smtp/password"));
+    EXPECT_TRUE(SettingsCache::isSecretKey("security/legacy_backup_key"));
+    EXPECT_TRUE(SettingsCache::isSecretKey("server/apiSecret"));
+    EXPECT_FALSE(SettingsCache::isSecretKey("server/port"));
+}
+
+TEST(ServatriceSettingsParsingTest, ParseOverrideAcceptsScalarKeys)
+{
+    QString key;
+    QVariant value;
+
+    ASSERT_TRUE(SettingsCache::parseOverride("server/port=14747", key, value));
+    EXPECT_EQ(key, QStringLiteral("server/port"));
+    EXPECT_EQ(value.toString(), QStringLiteral("14747"));
+
+    // Only the first '=' separates the key; the rest belongs to the value.
+    ASSERT_TRUE(SettingsCache::parseOverride("server/name=a=b", key, value));
+    EXPECT_EQ(value.toString(), QStringLiteral("a=b"));
+}
+
+TEST(ServatriceSettingsParsingTest, ParseOverrideRejectsMalformedEntries)
+{
+    QString key;
+    QVariant value;
+
+    EXPECT_FALSE(SettingsCache::parseOverride("server/port", key, value));
+    EXPECT_FALSE(SettingsCache::parseOverride("=14747", key, value));
+    EXPECT_FALSE(SettingsCache::parseOverride("   =14747", key, value));
+}
+
+TEST(ServatriceSettingsParsingTest, ParseOverrideRejectsBareAndArrayScopedKeys)
+{
+    QString key;
+    QVariant value;
+
+    EXPECT_FALSE(SettingsCache::parseOverride("name=Kitchen", key, value));
+    EXPECT_FALSE(SettingsCache::parseOverride("rooms/roomlist/1/name=Kitchen", key, value));
+    EXPECT_FALSE(SettingsCache::parseOverride("rooms/roomlist/01/name=Kitchen", key, value));
+    EXPECT_FALSE(SettingsCache::parseOverride("rooms/roomlist/size=2", key, value));
+}