Skip to content

[Application] Harden recent-project deletion flow#167

Merged
SilverSupplier merged 1 commit into
mainfrom
claude/sleepy-mcnulty-9e6c33
May 3, 2026
Merged

[Application] Harden recent-project deletion flow#167
SilverSupplier merged 1 commit into
mainfrom
claude/sleepy-mcnulty-9e6c33

Conversation

@SilverSupplier
Copy link
Copy Markdown
Collaborator

@SilverSupplier SilverSupplier commented May 3, 2026

Summary

  • Move project deletion to the OS recycle bin (QFile::moveToTrash) so the action is reversible; refuse to fall back to permanent recursive removal on failure.
  • Tighten canDeleteProjectFolder to reject symbolic links and Windows junctions (folder itself and any entry), and to detect drive/volume roots via QDir::isRoot + QStorageInfo rather than comparing against the system root.
  • Share a canonical, case-insensitive folder-key helper between removeRecentProject and upsertRecentProject so Windows/macOS path casing no longer leaves stale recent entries; write failures are now logged via qWarning instead of silently swallowed.
  • Use Qt::PlainText on the confirmation/warning dialogs in MainWindow so user-supplied project names and paths cannot be interpreted as rich text, and update the prompt to reflect the recycle-bin behaviour.

Related Issue

Area

  • Engine
  • Domain
  • Application
  • Docs
  • Build
  • Analysis
  • Chore

Architecture Check

  • I kept the dependency direction application -> domain -> engine.
  • I did not add Qt UI code to src/domain.
  • I did not add domain or application dependencies to src/engine.
  • I used src/ as the include root.

Verification

  • cmake --preset windows-debug
  • cmake --build --preset build-debug
  • ctest --preset test-debug
  • Not run (reason below)

safecrowd_app.exe and safecrowd_tests.exe build cleanly. ctest reports 1/1 passed locally.

Manual scenarios still recommended on a clean profile:

  1. Create a SafeCrowd project, then delete it from the recent list — confirm it moves to the recycle bin and can be restored.
  2. Verify deletion is refused for: a folder containing an unrelated file/subfolder, a folder that is a junction/symlink, and any drive/volume root mistakenly registered.
  3. Verify the recent list updates correctly when the same folder is registered with different casing on Windows.

Risks / Follow-up

  • QFile::moveToTrash may fail on filesystems without a trash (e.g. some network shares). The new behaviour fails closed (no permanent delete); a future change could offer an explicit "permanent delete" path with extra confirmation if needed.
  • TOCTOU between canDeleteProjectFolder and moveToTrash is unchanged but lower-risk now that deletion is reversible.

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.
@SilverSupplier SilverSupplier requested a review from learncold as a code owner May 3, 2026 05:55
@SilverSupplier SilverSupplier merged commit acc735b into main May 3, 2026
4 checks passed
@SilverSupplier SilverSupplier deleted the claude/sleepy-mcnulty-9e6c33 branch May 3, 2026 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Task-프로젝트 삭제 흐름 안전성 강화

1 participant