[Client] Warn when data paths point inside the program directory (#7473)

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>

cockatrice/CMakeLists.txt

@@ -59,6 +59,7 @@ set(cockatrice_SOURCES
     src/interface/widgets/dialogs/dlg_update.cpp
     src/interface/widgets/dialogs/dlg_view_log.cpp
     src/interface/widgets/dialogs/override_printing_warning.cpp
+    src/interface/widgets/dialogs/path_safety_warning.cpp
     src/interface/widgets/dialogs/tip_of_the_day.cpp
     src/interface/widgets/deck_share/deck_share_utils.cpp
     src/interface/widgets/deck_share/shared_deck_preview_widget.cpp

cockatrice/src/interface/widgets/dialogs/path_safety_warning.cpp

@@ -0,0 +1,34 @@
+#include "path_safety_warning.h"
+
+#include "../../client/settings/cache_settings.h"
+
+#include <QCoreApplication>
+#include <QMessageBox>
+#include <QObject>
+#include <QString>
+#include <libcockatrice/settings/paths_settings.h>
+
+class QWidget;
+
+bool PathSafetyWarning::confirmOutsideProgramDir(QWidget *parent, const QString &path)
+{
+    if (SettingsCache::instance().getIsPortableBuild()) {
+        return true;
+    }
+
+    if (!PathsSettings::isInsideDir(path, QCoreApplication::applicationDirPath())) {
+        return true;
+    }
+
+    QMessageBox box(QMessageBox::Warning, QObject::tr("Path inside the program directory"),
+                    QObject::tr("'%1' is inside the Cockatrice program directory.\n\n"
+                                "Files stored there can be deleted by an update or by uninstalling "
+                                "Cockatrice, so this change was not applied. Choose a location outside "
+                                "the program directory.")
+                        .arg(path),
+                    QMessageBox::Ok, parent);
+    box.setTextFormat(Qt::PlainText);
+    box.exec();
+
+    return false;
+}

cockatrice/src/interface/widgets/dialogs/path_safety_warning.h

@@ -0,0 +1,25 @@
+#ifndef COCKATRICE_PATH_SAFETY_WARNING_H
+#define COCKATRICE_PATH_SAFETY_WARNING_H
+
+class QString;
+class QWidget;
+
+namespace PathSafetyWarning
+{
+
+/**
+ * @brief Warns when the user tries to configure a data path inside the program directory.
+ *
+ * Shows a message box explaining that files kept there can be deleted by an
+ * update or by uninstalling Cockatrice. Portable builds are always exempt:
+ * their data lives beside the executables by design.
+ *
+ * @param parent The parent widget
+ * @param path The path the user is trying to configure
+ * @return true when the path is outside the program directory and safe to apply
+ */
+bool confirmOutsideProgramDir(QWidget *parent, const QString &path);
+
+} // namespace PathSafetyWarning
+
+#endif // COCKATRICE_PATH_SAFETY_WARNING_H

cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp

@@ -2,6 +2,7 @@
 
 #include "../../../client/settings/cache_settings.h"
 #include "../../pixel_map_generator.h"
+#include "../dialogs/path_safety_warning.h"
 #include "update/card_spoiler/spoiler_background_updater.h"
 
 #include <QAbstractItemModel>
@@ -363,6 +364,9 @@ void DeckEditorSettingsPage::spoilerPathButtonClicked()
     if (lsPath.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, lsPath + "/spoiler.xml")) {
+        return;
+    }
 
     mpSpoilerSavePathLineEdit->setText(lsPath + "/spoiler.xml");
     SettingsCache::instance().paths().setSpoilerDatabasePath(lsPath + "/spoiler.xml");

cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp

@@ -1,6 +1,7 @@
 #include "general_settings_page.h"
 
 #include "../../../client/settings/cache_settings.h"
+#include "../dialogs/path_safety_warning.h"
 #include "../interface/card_picture_loader/card_picture_loader.h"
 #include "../main.h"
 #include "../server/user/user_info_connection.h"
@@ -378,6 +379,9 @@ void GeneralSettingsPage::deckPathButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     deckPathEdit->setText(path);
     SettingsCache::instance().paths().setDeckPath(path);
@@ -389,6 +393,9 @@ void GeneralSettingsPage::filtersPathButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     filtersPathEdit->setText(path);
     SettingsCache::instance().paths().setFiltersPath(path);
@@ -400,6 +407,9 @@ void GeneralSettingsPage::replaysPathButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     replaysPathEdit->setText(path);
     SettingsCache::instance().paths().setReplaysPath(path);
@@ -411,6 +421,9 @@ void GeneralSettingsPage::picsPathButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     picsPathEdit->setText(path);
     SettingsCache::instance().paths().setPicsPath(path);
@@ -422,6 +435,9 @@ void GeneralSettingsPage::cardDatabasePathButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     cardDatabasePathEdit->setText(path);
     SettingsCache::instance().paths().setCardDatabasePath(path);
@@ -433,6 +449,9 @@ void GeneralSettingsPage::customCardDatabaseButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     customCardDatabasePathEdit->setText(path);
     SettingsCache::instance().paths().setCustomCardDatabasePath(path);
@@ -444,6 +463,9 @@ void GeneralSettingsPage::tokenDatabasePathButtonClicked()
     if (path.isEmpty()) {
         return;
     }
+    if (!PathSafetyWarning::confirmOutsideProgramDir(this, path)) {
+        return;
+    }
 
     tokenDatabasePathEdit->setText(path);
     SettingsCache::instance().paths().setTokenDatabasePath(path);

cockatrice/src/interface/window_main.cpp

@@ -58,7 +58,9 @@
 #include <QAction>
 #include <QApplication>
 #include <QButtonGroup>
+#include <QCheckBox>
 #include <QCloseEvent>
+#include <QCoreApplication>
 #include <QDateTime>
 #include <QDebug>
 #include <QDesktopServices>
@@ -603,8 +605,39 @@ MainWindow::MainWindow(QWidget *parent)
     QTimer::singleShot(0, this, &MainWindow::applyStartupDestination);
 }
 
+void MainWindow::warnAboutDataPathsInProgramDir()
+{
+    auto &paths = SettingsCache::instance().paths();
+    if (SettingsCache::instance().getIsPortableBuild() || paths.getAppDirWarningAcknowledged()) {
+        return;
+    }
+
+    const QStringList offenders = paths.pathsInsideDir(QCoreApplication::applicationDirPath());
+    if (offenders.isEmpty()) {
+        return;
+    }
+
+    QMessageBox box(QMessageBox::Warning, tr("Data paths inside the program directory"),
+                    tr("These paths are configured inside the Cockatrice program directory:\n\n%1\n\n"
+                       "Cockatrice keeps its own data outside the program directory by default. Files "
+                       "stored inside it can be deleted by an update or by uninstalling Cockatrice. "
+                       "Change them in the Settings dialog - most are on the General page - to keep "
+                       "them safe.")
+                        .arg(offenders.join(QLatin1Char('\n'))),
+                    QMessageBox::Ok, this);
+    box.setTextFormat(Qt::PlainText);
+    auto *dontShowAgain = new QCheckBox(tr("Don't show this again"));
+    box.setCheckBox(dontShowAgain);
+    box.exec();
+    if (dontShowAgain->isChecked()) {
+        paths.setAppDirWarningAcknowledged(true);
+    }
+}
+
 void MainWindow::startupConfigCheck()
 {
+    warnAboutDataPathsInProgramDir();
+
     const bool isCleanInstall = SettingsCache::instance().network().getClientVersion() == CLIENT_INFO_NOT_SET;
 
     // checkUnknownSets() is intentionally deferred from the card database load

cockatrice/src/interface/window_main.h

@@ -149,6 +149,9 @@ private:
 
     void runFirstRunWizard(bool firstRun = false);
 
+    /** @brief Warns once when a non-portable install keeps its data paths inside the program directory. */
+    void warnAboutDataPathsInProgramDir();
+
     inline QString getCardUpdaterBinaryName()
     {
         return "oracle";

libcockatrice_settings/libcockatrice/settings/paths_settings.cpp

@@ -2,6 +2,8 @@
 
 #include "libcockatrice/settings/settings_manager.h"
 
+#include <QDir>
+#include <QStringList>
 #include <QVariant>
 
 class QObject;
@@ -121,3 +123,52 @@ void PathsSettings::setSpoilerDatabasePath(const QString &_spoilerDatabasePath)
     setValue(_spoilerDatabasePath, "spoilerDatabase");
     emit cardDatabasePathChanged();
 }
+
+QStringList PathsSettings::pathsInsideDir(const QString &baseDir) const
+{
+    QStringList offenders;
+    const auto check = [&offenders, &baseDir](const QString &path) {
+        if (!path.isEmpty() && isInsideDir(path, baseDir)) {
+            offenders.append(QDir::cleanPath(path));
+        }
+    };
+
+    check(getDeckPath());
+    check(getFiltersPath());
+    check(getReplaysPath());
+    check(getPicsPath());
+    check(getCustomPicsPath());
+    check(getThemesPath());
+    check(getCardDatabasePath());
+    check(getCustomCardDatabasePath());
+    check(getTokenDatabasePath());
+    check(getSpoilerCardDatabasePath());
+    check(getRedirectCachePath());
+
+    offenders.removeDuplicates();
+    return offenders;
+}
+
+bool PathsSettings::isInsideDir(const QString &path, const QString &baseDir)
+{
+#if defined(Q_OS_WIN) || defined(Q_OS_MACOS)
+    // Windows and default macOS volumes are case-insensitive filesystems.
+    constexpr Qt::CaseSensitivity caseSensitivity = Qt::CaseInsensitive;
+#else
+    constexpr Qt::CaseSensitivity caseSensitivity = Qt::CaseSensitive;
+#endif
+    const QString base = QDir::cleanPath(baseDir);
+    const QString cleaned = QDir::cleanPath(path);
+    const QString prefix = base.endsWith(QLatin1Char('/')) ? base : base + QLatin1Char('/');
+    return cleaned == base || cleaned.startsWith(prefix, caseSensitivity);
+}
+
+bool PathsSettings::getAppDirWarningAcknowledged() const
+{
+    return getValue("appDirWarningAcknowledged").toBool();
+}
+
+void PathsSettings::setAppDirWarningAcknowledged(bool _appDirWarningAcknowledged)
+{
+    setValue(_appDirWarningAcknowledged, "appDirWarningAcknowledged");
+}

libcockatrice_settings/libcockatrice/settings/paths_settings.h

@@ -4,6 +4,7 @@
 #include "settings_manager.h"
 
 #include <QString>
+#include <QStringList>
 #include <libcockatrice/interfaces/interface_paths_settings_provider.h>
 #include <qtmetamacros.h>
 
@@ -27,6 +28,24 @@ public:
     [[nodiscard]] QString getSpoilerCardDatabasePath() const override;
     [[nodiscard]] QString getRedirectCachePath() const override;
 
+    /**
+     * @brief Returns the configured paths that resolve inside baseDir.
+     *
+     * Comparison is lexical (QDir::cleanPath); symlinks are not resolved, so a
+     * path that only reaches baseDir through a symlink is not reported.
+     */
+    [[nodiscard]] QStringList pathsInsideDir(const QString &baseDir) const;
+
+    /**
+     * @brief Whether path resolves inside baseDir, as a single-path check.
+     *
+     * A path counts as inside when it equals baseDir or is nested below it;
+     * a sibling sharing the name prefix (baseDir + "2") does not.
+     */
+    [[nodiscard]] static bool isInsideDir(const QString &path, const QString &baseDir);
+
+    [[nodiscard]] bool getAppDirWarningAcknowledged() const;
+
     void setDeckPath(const QString &_deckPath);
     void setFiltersPath(const QString &_filtersPath);
     void setReplaysPath(const QString &_replaysPath);
@@ -37,14 +56,17 @@ public:
     void setCustomCardDatabasePath(const QString &_customCardDatabasePath);
     void setTokenDatabasePath(const QString &_tokenDatabasePath);
     void setSpoilerDatabasePath(const QString &_spoilerDatabasePath);
+    void setAppDirWarningAcknowledged(bool _appDirWarningAcknowledged);
 
 signals:
     void cardDatabasePathChanged();
     void picsPathChanged();
     void themeChanged();
 
-private:
+public:
     explicit PathsSettings(const QString &settingPath, QObject *parent = nullptr);
+
+private:
     PathsSettings(const PathsSettings & /*other*/);
 };
 

tests/settings/settings_defaults_test.cpp

@@ -1,6 +1,7 @@
 #include "libcockatrice/card/card_localization.h"
 
 #include "gtest/gtest.h"
+#include <QDir>
 #include <QHash>
 #include <QList>
 #include <QString>
@@ -15,6 +16,7 @@
 #include <libcockatrice/settings/game_settings.h>
 #include <libcockatrice/settings/interface_settings.h>
 #include <libcockatrice/settings/network_settings.h>
+#include <libcockatrice/settings/paths_settings.h>
 #include <libcockatrice/settings/personal_settings.h>
 #include <libcockatrice/settings/sound_settings.h>
 #include <libcockatrice/settings/tabs_settings.h>
@@ -695,6 +697,55 @@ TEST_F(SettingsDefaultsTest, VisualDeckStorage_ShowUploadTime_SetAndGet)
     ASSERT_EQ(s.getVisualDeckStorageShowUploadTime(), false);
 }
 
+TEST_F(SettingsDefaultsTest, Paths_AppDirWarningAcknowledged_Default)
+{
+    PathsSettings s(settingsPath, nullptr);
+    ASSERT_EQ(s.getAppDirWarningAcknowledged(), false);
+}
+
+TEST_F(SettingsDefaultsTest, Paths_AppDirWarningAcknowledged_SetAndGet)
+{
+    PathsSettings s(settingsPath, nullptr);
+    s.setAppDirWarningAcknowledged(true);
+    ASSERT_EQ(s.getAppDirWarningAcknowledged(), true);
+}
+
+TEST_F(SettingsDefaultsTest, Paths_PathsInsideDir_DetectsNestedAndIgnoresSiblings)
+{
+    PathsSettings s(settingsPath, nullptr);
+    const QString base = QDir::cleanPath(settingsPath);
+
+    s.setDeckPath(base + "/decks/");
+    s.setPicsPath(base + "/pics");
+    s.setReplaysPath(base + "2"); // sibling sharing the name prefix, not nested
+
+    const QStringList offenders = s.pathsInsideDir(base);
+    ASSERT_EQ(offenders.size(), 2);
+    ASSERT_TRUE(offenders.contains(base + "/decks"));
+    ASSERT_TRUE(offenders.contains(base + "/pics"));
+}
+
+TEST_F(SettingsDefaultsTest, Paths_PathsInsideDir_EmptyWhenNothingMatches)
+{
+    PathsSettings s(settingsPath, nullptr);
+    s.setDeckPath("/somewhere/else/decks");
+    ASSERT_TRUE(s.pathsInsideDir(QDir::cleanPath(settingsPath)).isEmpty());
+}
+
+TEST_F(SettingsDefaultsTest, Paths_PathsInsideDir_MatchesExactBaseAndNormalizesDots)
+{
+    PathsSettings s(settingsPath, nullptr);
+    const QString base = QDir::cleanPath(settingsPath);
+
+    s.setCustomPicsPath(base);                // exactly the base directory
+    s.setThemesPath(base + "/pics/../decks"); // needs cleanPath normalization
+
+    const QStringList offenders = s.pathsInsideDir(base);
+    ASSERT_EQ(offenders.size(), 2);
+    ASSERT_TRUE(offenders.contains(base));
+    ASSERT_TRUE(offenders.contains(base + "/decks"));
+}
+
 } // namespace
 
 int main(int argc, char **argv)