From 214b284814ebff680f66a199e290e2cdf33129a4 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Tue, 10 Nov 2026 12:29:45 +0100 Subject: [PATCH] [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 | 52 +++++++ servatrice/src/settingscache.cpp | 72 +++++++++ servatrice/src/settingscache.h | 42 +++++ tests/settings/CMakeLists.txt | 12 ++ tests/settings/servatrice_settings_test.cpp | 162 ++++++++++++++++++++ 5 files changed, 340 insertions(+) create mode 100644 tests/settings/servatrice_settings_test.cpp diff --git a/servatrice/src/main.cpp b/servatrice/src/main.cpp index 93d800741..92acef769 100644 --- a/servatrice/src/main.cpp +++ b/servatrice/src/main.cpp @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -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("") : 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", ""); 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(), diff --git a/servatrice/src/settingscache.cpp b/servatrice/src/settingscache.cpp index 4ecac7319..b8486a336 100644 --- a/servatrice/src/settingscache.cpp +++ b/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 ®ExpStr : disallowedRegExpStr) { diff --git a/servatrice/src/settingscache.h b/servatrice/src/settingscache.h index 817a62bfa..2a4aac331 100644 --- a/servatrice/src/settingscache.h +++ b/servatrice/src/settingscache.h @@ -1,10 +1,13 @@ #ifndef SERVATRICE_SETTINGSCACHE_H #define SERVATRICE_SETTINGSCACHE_H +#include #include #include #include #include +#include +#include #include class QObject; @@ -14,12 +17,51 @@ class SettingsCache : public QSettings Q_OBJECT private: bool isPortableBuild; + QHash 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 &getOverrides() const + { + return overrides; + } + + /** @brief Rebuild disallowedRegExp from users/disallowedregexp after overrides are applied. */ + void rebuildDisallowedRegExps(); + QList disallowedRegExp; bool getIsPortableBuild() const { diff --git a/tests/settings/CMakeLists.txt b/tests/settings/CMakeLists.txt index 9dbda9d6b..f38a5aa52 100644 --- a/tests/settings/CMakeLists.txt +++ b/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 +) diff --git a/tests/settings/servatrice_settings_test.cpp b/tests/settings/servatrice_settings_test.cpp new file mode 100644 index 000000000..47a00d7d6 --- /dev/null +++ b/tests/settings/servatrice_settings_test.cpp @@ -0,0 +1,162 @@ +#include "settingscache.h" + +#include +#include +#include +#include +#include +#include +#include + +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(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)); +}