Improve file operation safety Part 1

CMakeLists.txt

@@ -83,6 +83,9 @@ add_executable(${PROJECT_NAME})
 target_link_libraries(${PROJECT_NAME} PUBLIC core-target)
 
 if (WIN32)
+    # A GUI program: a console program lives in a console window and exits with it, closing that window or a crash
+    # of Windows Terminal ended the app silently. --console attaches to the console it was started from.
+    set_target_properties(${PROJECT_NAME} PROPERTIES WIN32_EXECUTABLE TRUE)
     include("${CMAKE_CURRENT_LIST_DIR}/_cmake/windows/icon.cmake")
     file(TO_CMAKE_PATH "${NUI_SFTP_ICO}" NUI_SFTP_ICO_FORWARD)
     configure_file(
@@ -139,6 +142,7 @@ add_subdirectory("${CMAKE_SOURCE_DIR}/dependencies/gimo" EXCLUDE_FROM_ALL)
 add_subdirectory("${CMAKE_SOURCE_DIR}/dependencies/traits" EXCLUDE_FROM_ALL)
 add_subdirectory("${CMAKE_SOURCE_DIR}/constants/source/constants")
 add_subdirectory("${CMAKE_SOURCE_DIR}/ids/source/ids")
+add_subdirectory("${CMAKE_SOURCE_DIR}/paths/source/paths")
 add_subdirectory("${CMAKE_SOURCE_DIR}/shared_data/source/shared_data")
 add_subdirectory("${CMAKE_SOURCE_DIR}/utility/source/utility")
 add_subdirectory("${CMAKE_SOURCE_DIR}/log/source/log")
@@ -190,6 +194,7 @@ else()
         add_subdirectory(ssh/test/ssh)
         add_subdirectory(backend/test/backend)
         add_subdirectory(utility/test/utility)
+        add_subdirectory(paths/test/paths)
         add_subdirectory(shared_data/test/shared_data)
         add_subdirectory(shared_data/benchmark/shared_data)
         add_subdirectory(tar-archive/test/tar_archive)

CONTRIBUTING.md

@@ -226,8 +226,9 @@ cmake --build build/clang_debug --target \
   nui-sftp-shared-data-test nui-sftp-ssh-test \
   nui-sftp-tar-archive-test nui-sftp-command-store-test
 
-# Run the whole suite.
-ctest --test-dir build/clang_debug --output-on-failure
+# Run the whole suite. Tests run in parallel; the timeout makes a hang fail
+# after two minutes instead of blocking the run.
+ctest --test-dir build/clang_debug --output-on-failure --timeout 120 -j$(nproc)
 
 # Or run an individual binary.
 ./build/clang_debug/test/nui-sftp-utility-test
@@ -328,7 +329,7 @@ update both workflows if it changes the build surface.
 1. Fork the repository and create a feature branch from `main`.
 2. Keep commits focused; prefer small, reviewable changes over large rewrites.
 3. Run a debug build with tests enabled locally and make sure `ctest
-   --output-on-failure` passes from `build/clang_debug`.
+   --output-on-failure --timeout 120 -j$(nproc)` passes from `build/clang_debug`.
 4. Format any C++ changes with clang-format.
 5. Open the PR against `main`. Describe the user-visible behaviour, the
    motivation, and anything reviewers should pay particular attention to.
@@ -377,7 +378,7 @@ needed.
             "type": "shell",
             "dependsOn": "build_debug",
             "command": "ctest",
-            "args": ["--test-dir", "build/clang_debug", "--output-on-failure"],
+            "args": ["--test-dir", "build/clang_debug", "--output-on-failure", "--timeout", "120", "-j", "8"],
             "group": "test"
         }
     ]

backend/include/backend/dropped_entry.hpp

@@ -1,11 +1,12 @@
 #pragma once
 
-#include <shared_data/directory_entry.hpp>
+#include <shared_data/dropped_item.hpp>
 
 #include <filesystem>
+#include <optional>
 
 /**
  * @brief Describes a file dropped onto the window from the desktop, with just enough detail for the transfer.
- * @param path Local path of the dropped file or directory.
+ * @param path Local path of the dropped file or directory, nothing when it is not an absolute path.
  */
-SharedData::DirectoryEntry leanDirectoryEntryFromPath(std::filesystem::path const& path);
+std::optional<SharedData::DroppedItem> droppedItemFromPath(std::filesystem::path const& path);

backend/include/backend/existence_check.hpp

@@ -0,0 +1,24 @@
+#pragma once
+
+#include <ssh/sftp_session.hpp>
+
+#include <chrono>
+#include <expected>
+#include <string>
+#include <vector>
+
+/**
+ * @brief Whether each of @p paths exists on the local disk. Only "not found" is absent, a dangling symlink exists. A
+ *        path that cannot be checked fails the whole check: a transfer overwrites what it takes for absent.
+ */
+std::expected<std::vector<bool>, std::string> localPathsExist(std::vector<std::string> const& paths);
+
+/**
+ * @brief Whether each of @p paths exists on the server, with the same rules as localPathsExist. Few paths are
+ *        checked with one lstat each, more with one listing per parent directory.
+ */
+std::expected<std::vector<bool>, std::string> remotePathsExist(
+    SecureShell::SftpSession& session,
+    std::vector<std::string> const& paths,
+    std::chrono::milliseconds timeout
+);

backend/include/backend/local_delete_guard.hpp

@@ -0,0 +1,56 @@
+#pragma once
+
+#include <paths/local_path.hpp>
+
+#include <fmt/format.h>
+
+#include <expected>
+#include <optional>
+#include <ranges>
+#include <string>
+#include <string_view>
+
+/**
+ * @brief Refuses a root directory and @p home or a directory containing it.
+ */
+inline std::expected<void, std::string>
+refuseProtectedLocalDelete(Paths::LocalPath const& target, std::optional<Paths::LocalPath> const& home)
+{
+    if (target.empty())
+        return std::unexpected(std::string{"no path"});
+    if (target.isRoot())
+        return std::unexpected(fmt::format("'{}' is a root directory", target.toUtf8()));
+    if (home && home->isWithin(target))
+        return std::unexpected(fmt::format("'{}' contains the home directory", target.toUtf8()));
+    return {};
+}
+
+/**
+ * @brief A local path that may be deleted, or the reason to refuse it: not absolute, containing '..', a root
+ *        directory, or @p home or a directory containing it.
+ */
+inline std::expected<Paths::LocalPath, std::string>
+localDeleteTarget(std::string const& utf8, std::optional<Paths::LocalPath> const& home)
+{
+    // LocalPath resolves ".." lexically, which goes elsewhere when a directory on the way is a symlink.
+    for (auto const segment : std::views::split(std::string_view{utf8}, '/'))
+    {
+        for (auto const part : std::views::split(segment, '\\'))
+        {
+            if (std::string_view{part.begin(), part.end()} == "..")
+                return std::unexpected(fmt::format("'{}' contains '..'", utf8));
+        }
+    }
+
+    auto target = Paths::LocalPath::fromUtf8(utf8);
+    if (!target)
+        return std::unexpected(fmt::format("'{}' {}", utf8, Paths::toString(target.error().reason)));
+    if (auto allowed = refuseProtectedLocalDelete(*target, home); !allowed)
+        return std::unexpected(std::move(allowed).error());
+    return *std::move(target);
+}
+
+/**
+ * @brief The home directory of the user running the backend, empty when it cannot be determined.
+ */
+std::optional<Paths::LocalPath> localHomeDirectory();

backend/include/backend/opener.hpp

@@ -5,6 +5,7 @@
 #include <expected>
 #include <filesystem>
 #include <memory>
+#include <optional>
 #include <string>
 
 class Opener
@@ -19,6 +20,13 @@ class Opener
 
     std::expected<void, std::string> openFile(std::filesystem::path const& path, bool openWith);
 
+    /**
+     * @brief Why opening @p path could run code: a program, a script, a shortcut or an application bundle. Nothing
+     *        for an ordinary document or a folder.
+     * @return A localized message key describing the risk.
+     */
+    std::optional<std::string> riskOf(std::filesystem::path const& path) const;
+
     /**
      *  @brief Open the native file manager showing @p path.
      *  @details For a directory, opens the file manager at that directory. For a file, opens the

backend/include/backend/rpc_filesystem.hpp

@@ -4,6 +4,8 @@
 #include <backend/opener.hpp>
 #include <persistence/state/local_filesystem_options.hpp>
 
+#include <functional>
+
 class RpcFilesystem : public RpcHelper::StrandRpc
 {
   public:
@@ -12,7 +14,8 @@ class RpcFilesystem : public RpcHelper::StrandRpc
         Nui::Window& wnd,
         Nui::RpcHub& hub,
         Persistence::LocalFilesystemOptions options,
-        Opener& opener
+        Opener& opener,
+        std::function<Persistence::RiskyFilePolicy()> riskyFilePolicy = []() { return Persistence::RiskyFilePolicy::Warn; }
     );
 
   private:
@@ -34,4 +37,6 @@ class RpcFilesystem : public RpcHelper::StrandRpc
   private:
     Persistence::LocalFilesystemOptions options_;
     Opener* opener_;
+    /** @brief Read on every open, so a changed setting applies at once. */
+    std::function<Persistence::RiskyFilePolicy()> riskyFilePolicy_;
 };

backend/include/backend/session.hpp

@@ -209,7 +209,7 @@ class Session
     void registerRpcSftpOpenSyncSession();
     void registerRpcSftpRecomputeSyncDiff();
     void registerRpcSftpLoadSyncDiffChildren();
-    void registerRpcSftpBuildSyncEnqueuePlan();
+    void registerRpcSftpExecuteSyncSelection();
     void registerRpcSftpCancelSyncDiff();
     void registerRpcSftpCloseSyncSession();
     void registerOperationQueuePauseUnpause();

backend/include/backend/sftp/all_operations.hpp

@@ -2,6 +2,7 @@
 #include <backend/sftp/download_operation.hpp>
 #include <backend/sftp/upload_operation.hpp>
 #include <backend/sftp/delete_operation.hpp>
+#include <backend/sftp/local_delete_operation.hpp>
 #include <backend/sftp/rename_operation.hpp>
 #include <backend/sftp/scan_operation.hpp>
 #include <backend/sftp/bulk_download_operation.hpp>
@@ -48,6 +49,10 @@ auto Operation::visit(FunctionT&& func) const
         {
             return func(static_cast<RenameOperation const&>(*this));
         }
+        case LocalDelete:
+        {
+            return func(static_cast<LocalDeleteOperation const&>(*this));
+        }
         case ArchiveDownload:
         {
             return func(static_cast<ArchiveDownloadOperation const&>(*this));

backend/include/backend/sftp/archive_download_operation.hpp

@@ -1,6 +1,9 @@
 #pragma once
 
 #include <backend/sftp/operation.hpp>
+#include <backend/sftp/scan_operation.hpp>
+#include <backend/sftp/scan_phase.hpp>
+#include <paths/local_path.hpp>
 #include <ssh/async/buffer_provider.hpp>
 #include <ssh/async/transfer_context.hpp>
 #include <ssh/file_stream.hpp>
@@ -10,6 +13,8 @@
 #include <tar_archive/entry_writer.hpp>
 #include <tar_archive/compression.hpp>
 #include <shared_data/directory_entry.hpp>
+#include <shared_data/file_operations/archive_download_root.hpp>
+#include <tar_archive/member_header.hpp>
 
 #include <cstdint>
 #include <filesystem>
@@ -62,13 +67,12 @@ class ArchiveDownloadOperation : public Operation
          *    `foo/sub/`, `foo/sub/baz.txt`.
          *  - Other types (symlinks, devices, ...) are silently skipped.
          *
-         * `fullPath` must carry the full remote path; `path` is used only if
-         * `fullPath` is empty.
+         * A root that is the remote root directory has no name and is refused.
          */
-        std::vector<SharedData::DirectoryEntry> entries{};
+        std::vector<SharedData::ArchiveDownloadRoot> roots{};
 
         /** @brief Destination archive path on the local filesystem. */
-        std::filesystem::path localArchivePath{};
+        Paths::LocalPath localArchivePath{};
 
         /** @brief Compression codec. Use TarArchive::Compression::Auto to pick from the extension. */
         TarArchive::Compression compression{TarArchive::Compression::Auto};
@@ -81,6 +85,8 @@ class ArchiveDownloadOperation : public Operation
 
         /** @brief Refuse to overwrite a pre-existing final archive. */
         bool mayOverwrite{false};
+        /** @brief Mark the archive as downloaded from the internet, see Utility::markAsDownloaded. */
+        bool markAsDownloaded{true};
 
         /** @brief Create any missing parents of @ref localArchivePath. */
         bool createMissingDirectories{false};
@@ -98,9 +104,25 @@ class ArchiveDownloadOperation : public Operation
     std::expected<WorkStatus, Error> work() override;
     std::expected<WorkStatus, Error> workInStrand() override;
 
+    // The scan phase blocks on SFTP futures, so it runs off the strand like a standalone scan.
+    bool usesStrand() const noexcept override
+    {
+        return scanPhase_ == nullptr;
+    }
+
     bool isBarrier() const noexcept override
     {
-        return false;
+        return scanPhase_ != nullptr;
+    }
+
+    /**
+     * @brief Scans the directory root of @p scan before packing; roots are scanned in the order added.
+     */
+    void addScanRoot(std::unique_ptr<ScanOperation> scan);
+
+    bool scanning() const noexcept
+    {
+        return scanPhase_ != nullptr;
     }
 
     int parallelWorkDoable(int) const noexcept override
@@ -129,16 +151,15 @@ class ArchiveDownloadOperation : public Operation
      *        rather than silently buried inside prepare.
      *
      *        Safe to call multiple times — one call per directory root in
-     *        @ref Options::entries.  Matching is by @p remoteRootPath which
-     *        must equal the root entry's full remote path
-     *        (fullPath-or-fallback-to-path in the root @ref DirectoryEntry).
+     *        @ref Options::roots.  Matching is by @p remoteRootPath, the
+     *        path of the root.
      *
      *        Threading: invoked on the queue's asio strand between SFTP-strand
      *        work batches, so no extra synchronisation is required against
      *        prepareInStrand (which runs on the SFTP strand).
      */
     void setScanResultForRoot(
-        std::filesystem::path const& remoteRootPath,
+        Paths::RemotePath const& remoteRootPath,
         std::vector<SharedData::DirectoryEntry> entries,
         std::uint64_t totalBytes
     );
@@ -152,8 +173,8 @@ class ArchiveDownloadOperation : public Operation
      */
     struct ResolvedEntry
     {
-        SharedData::DirectoryEntry tarMeta{};
-        std::filesystem::path remoteFullPath{};
+        TarArchive::MemberHeader header{};
+        Paths::RemotePath source{};
     };
 
     std::expected<void, Error> prepareInStrand();
@@ -162,14 +183,14 @@ class ArchiveDownloadOperation : public Operation
      * @brief Expand a preceding ScanOperation's flat entry vector into the
      *        op's ResolvedEntry list, prefixing each entry's tar path with
      *        @p archivePrefix and composing remote paths under
-     *        @p remoteRootFullPath. Index 0 (the synthetic scan root) is
+     *        @p remoteRoot. Index 0 (the synthetic scan root) is
      *        skipped because the root directory entry is emitted separately
      *        by flattenRootsInStrand.
      */
     void appendPrescannedInStrand(
         std::vector<SharedData::DirectoryEntry> const& entries,
-        std::filesystem::path const& remoteRootFullPath,
-        std::filesystem::path const& archivePrefix
+        Paths::RemotePath const& remoteRoot,
+        Paths::RelativePath const& archivePrefix
     );
     std::expected<void, Error> openNextEntryInStrand();
     std::expected<bool, Error> readChunkInStrand();
@@ -191,14 +212,16 @@ class ArchiveDownloadOperation : public Operation
     std::uint64_t totalBytesTransferred_{0u};
     std::uint64_t totalPayloadBytes_{0u};
     SecureShell::BufferLease buffer_;
+    /** @brief options_.localArchivePath for the filesystem calls, empty when none was given. */
+    std::filesystem::path localArchiveNativePath_{};
     std::filesystem::path tempPath_{};
     /**
      * @brief Pre-scanned entry lists delivered by preceding ScanOperation(s),
      *        keyed by the directory root's remote full path. flattenRoots
      *        looks each directory root up here before falling back to a live
      *        recurseIntoDirectoryInStrand listing.
      */
-    std::map<std::filesystem::path, std::pair<std::vector<SharedData::DirectoryEntry>, std::uint64_t>>
+    std::map<Paths::RemotePath, std::pair<std::vector<SharedData::DirectoryEntry>, std::uint64_t>>
         prescannedRoots_{};
     /**
      * @brief Reused purely for its bytes-transferred + B/s meter machinery;
@@ -208,4 +231,5 @@ class ArchiveDownloadOperation : public Operation
      */
     std::shared_ptr<SecureShell::AsyncTransferContext> throughputMeter_{
         std::make_shared<SecureShell::AsyncTransferContext>()};
+    std::unique_ptr<ScanPhase<ScanOperation>> scanPhase_{};
 };