From 2fb565941f45a2ab3b653f823cdd749fdd86df27 Mon Sep 17 00:00:00 2001 From: Silversupplier Date: Sun, 3 May 2026 03:22:38 +0900 Subject: [PATCH] [Application] Harden recent-project deletion flow Move project deletion to the OS recycle bin so the action is reversible, and tighten the pre-delete checks against drive/volume roots, symbolic links, and Windows junctions. The previous flow used QDir::removeRecursively unconditionally, with a drive-root guard that compared against the system root instead of the folder's own volume. - ProjectPersistence::deleteProject now calls QFile::moveToTrash and refuses to permanently delete on failure, surfacing a clear error instead of silently falling back to recursive removal. - canDeleteProjectFolder rejects symlinks/junctions on the project folder and on every entry, and uses QDir::isRoot plus QStorageInfo to detect both drive and volume roots. - removeRecentProject and upsertRecentProject share a canonical, case-insensitive folder-key helper so Windows/macOS path casing no longer leaves stale recent entries; write failures are now logged via qWarning instead of silently swallowed. - MainWindow's confirmation/warning dialogs use Qt::PlainText so user-supplied project names and paths cannot be interpreted as rich text, and the prompt now tells the user the folder will be moved to the recycle bin. --- src/application/MainWindow.cpp | 34 +++++--- src/application/ProjectPersistence.cpp | 109 +++++++++++++++++-------- 2 files changed, 99 insertions(+), 44 deletions(-) diff --git a/src/application/MainWindow.cpp b/src/application/MainWindow.cpp index a735721..b24c573 100644 --- a/src/application/MainWindow.cpp +++ b/src/application/MainWindow.cpp @@ -198,24 +198,38 @@ void MainWindow::showProjectNavigator() { }); navigator->setDeleteProjectHandler([this](const ProjectMetadata& metadata) { if (metadata.isBuiltInDemo()) { - QMessageBox::information(this, "Delete Project", "The built-in demo project cannot be deleted."); + QMessageBox info(QMessageBox::Information, + "Delete Project", + "The built-in demo project cannot be deleted.", + QMessageBox::Ok, + this); + info.setTextFormat(Qt::PlainText); + info.exec(); return; } - const auto choice = QMessageBox::question( - this, - "Delete Project", - QString("Delete \"%1\" and its project folder?\n\n%2") - .arg(metadata.name, metadata.folderPath), - QMessageBox::Yes | QMessageBox::No, - QMessageBox::No); - if (choice != QMessageBox::Yes) { + QMessageBox confirm(QMessageBox::Question, + "Delete Project", + QString("Move \"%1\" to the recycle bin?\n\n%2\n\n" + "You can restore it from the recycle bin if you change your mind.") + .arg(metadata.name, metadata.folderPath), + QMessageBox::Yes | QMessageBox::No, + this); + confirm.setTextFormat(Qt::PlainText); + confirm.setDefaultButton(QMessageBox::No); + if (confirm.exec() != QMessageBox::Yes) { return; } QString errorMessage; if (!ProjectPersistence::deleteProject(metadata, &errorMessage)) { - QMessageBox::warning(this, "Delete Project", errorMessage); + QMessageBox warning(QMessageBox::Warning, + "Delete Project", + errorMessage, + QMessageBox::Ok, + this); + warning.setTextFormat(Qt::PlainText); + warning.exec(); return; } diff --git a/src/application/ProjectPersistence.cpp b/src/application/ProjectPersistence.cpp index 4b14c89..152995a 100644 --- a/src/application/ProjectPersistence.cpp +++ b/src/application/ProjectPersistence.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -10,6 +11,7 @@ #include #include #include +#include #include "domain/ImportValidationService.h" @@ -28,6 +30,20 @@ bool isProjectManagedEntry(const QString& fileName) { || fileName.compare(kWorkspaceFileName, Qt::CaseInsensitive) == 0; } +QString normalizedFolderKey(const QString& folderPath) { + const auto canonical = QFileInfo(folderPath).canonicalFilePath(); + const auto base = canonical.isEmpty() ? QDir(folderPath).absolutePath() : canonical; +#if defined(Q_OS_WIN) || defined(Q_OS_MAC) + return base.toLower(); +#else + return base; +#endif +} + +bool sameFolderPath(const QString& lhs, const QString& rhs) { + return normalizedFolderKey(lhs) == normalizedFolderKey(rhs); +} + QString projectFilePath(const QString& folderPath) { return QDir(folderPath).filePath(kProjectFileName); } @@ -96,10 +112,9 @@ void upsertRecentProject(const ProjectMetadata& metadata) { QJsonArray updated; updated.append(toJson(metadata)); - const auto normalizedFolder = QDir(metadata.folderPath).absolutePath(); for (const auto& value : projects) { const auto existing = fromJson(value.toObject()); - if (QDir(existing.folderPath).absolutePath() == normalizedFolder) { + if (sameFolderPath(existing.folderPath, metadata.folderPath)) { continue; } updated.append(value); @@ -119,10 +134,9 @@ void removeRecentProject(const QString& folderPath) { } QJsonArray updated; - const auto normalizedFolder = QDir(folderPath).absolutePath(); for (const auto& value : document.object().value("projects").toArray()) { const auto existing = fromJson(value.toObject()); - if (QDir(existing.folderPath).absolutePath() == normalizedFolder) { + if (sameFolderPath(existing.folderPath, folderPath)) { continue; } updated.append(value); @@ -130,24 +144,40 @@ void removeRecentProject(const QString& folderPath) { QJsonObject root; root["projects"] = updated; - QString ignoredError; - writeJsonDocument(recentPath, QJsonDocument(root), &ignoredError); + QString writeError; + if (!writeJsonDocument(recentPath, QJsonDocument(root), &writeError)) { + qWarning() << "Failed to update recent projects list:" << writeError; + } } bool canDeleteProjectFolder(const QString& folderPath, QString* errorMessage) { - QDir folder(folderPath); - if (!folder.exists()) { + const auto setError = [errorMessage](const QString& message) { if (errorMessage != nullptr) { - *errorMessage = QString("Project folder does not exist: %1").arg(folderPath); + *errorMessage = message; } + }; + + QDir folder(folderPath); + if (!folder.exists()) { + setError(QString("Project folder does not exist: %1").arg(folderPath)); return false; } const QFileInfo folderInfo(folder.absolutePath()); - if (folderInfo.absoluteFilePath() == QDir(folderInfo.absolutePath()).rootPath()) { - if (errorMessage != nullptr) { - *errorMessage = QString("Refusing to delete a drive root: %1").arg(folderPath); - } + if (folderInfo.isSymLink() || folderInfo.isJunction()) { + setError(QString("Refusing to delete a symbolic link or junction: %1").arg(folderPath)); + return false; + } + + if (QDir(folderInfo.absoluteFilePath()).isRoot()) { + setError(QString("Refusing to delete a drive root: %1").arg(folderPath)); + return false; + } + + const QStorageInfo storage(folderInfo.absoluteFilePath()); + if (storage.isValid() && !storage.rootPath().isEmpty() + && QFileInfo(storage.rootPath()).absoluteFilePath() == folderInfo.absoluteFilePath()) { + setError(QString("Refusing to delete a volume root: %1").arg(folderPath)); return false; } @@ -155,12 +185,16 @@ bool canDeleteProjectFolder(const QString& folderPath, QString* errorMessage) { QDir::AllEntries | QDir::Hidden | QDir::System | QDir::NoDotAndDotDot, QDir::Name); for (const auto& entry : entries) { + if (entry.isSymLink() || entry.isJunction()) { + setError(QString( + "Refusing to delete project folder because it contains a link not created by SafeCrowd: %1") + .arg(entry.fileName())); + return false; + } if (!entry.isFile() || !isProjectManagedEntry(entry.fileName())) { - if (errorMessage != nullptr) { - *errorMessage = QString( - "Refusing to delete project folder because it contains a file or folder not created by SafeCrowd: %1") - .arg(entry.fileName()); - } + setError(QString( + "Refusing to delete project folder because it contains a file or folder not created by SafeCrowd: %1") + .arg(entry.fileName())); return false; } } @@ -1109,17 +1143,19 @@ ProjectMetadata ProjectPersistence::loadProject(const QString& folderPath) { } bool ProjectPersistence::deleteProject(const ProjectMetadata& metadata, QString* errorMessage) { - if (metadata.isBuiltInDemo()) { + const auto setError = [errorMessage](const QString& message) { if (errorMessage != nullptr) { - *errorMessage = "Built-in demo projects cannot be deleted."; + *errorMessage = message; } + }; + + if (metadata.isBuiltInDemo()) { + setError("Built-in demo projects cannot be deleted."); return false; } if (metadata.folderPath.isEmpty()) { - if (errorMessage != nullptr) { - *errorMessage = "Project folder is missing."; - } + setError("Project folder is missing."); return false; } @@ -1131,9 +1167,7 @@ bool ProjectPersistence::deleteProject(const ProjectMetadata& metadata, QString* const auto loaded = loadProject(metadata.folderPath); if (!loaded.isValid()) { - if (errorMessage != nullptr) { - *errorMessage = "The selected folder does not contain a valid SafeCrowd project."; - } + setError("The selected folder does not contain a valid SafeCrowd project."); return false; } @@ -1141,16 +1175,23 @@ bool ProjectPersistence::deleteProject(const ProjectMetadata& metadata, QString* return false; } - QDir folder(metadata.folderPath); - if (!folder.removeRecursively()) { - if (errorMessage != nullptr) { - *errorMessage = QString("Failed to delete project folder: %1").arg(metadata.folderPath); - } - return false; + const auto absoluteFolder = QFileInfo(metadata.folderPath).absoluteFilePath(); + QString trashPath; + if (QFile::moveToTrash(absoluteFolder, &trashPath)) { + qInfo().noquote() << "Moved project folder to trash:" << absoluteFolder + << "->" << (trashPath.isEmpty() ? QStringLiteral("(unknown)") : trashPath); + removeRecentProject(metadata.folderPath); + return true; } - removeRecentProject(metadata.folderPath); - return true; + qWarning().noquote() << "moveToTrash failed for" << absoluteFolder + << "- aborting deletion to keep the operation reversible"; + setError(QString( + "Failed to move the project folder to the recycle bin:\n%1\n\n" + "The project was not deleted. Please check folder permissions and try again, " + "or remove the folder manually.") + .arg(metadata.folderPath)); + return false; } bool ProjectPersistence::loadProjectReview(const ProjectMetadata& metadata, safecrowd::domain::ImportResult* importResult) {