Improve file operation safety Part 2

CMakeLists.txt

@@ -6,6 +6,9 @@ project(nui-sftp VERSION "${PROJECT_VERSION}" LANGUAGES CXX)
 
 set(SPDLOG_FMT_EXTERNAL ON CACHE BOOL "Use an external fmt library (provide it manually)" FORCE)
 set(SPDLOG_BUILD_SHARED OFF CACHE BOOL "Dont build shared spdlog" FORCE)
+if (WIN32)
+    set(SPDLOG_WCHAR_FILENAMES ON CACHE BOOL "Open log files by their wide name" FORCE)
+endif()
 set(NUI_FILE_EXPLORER_MEMORY64 ON CACHE BOOL "Enable MEMORY64 for nui-file-explorer" FORCE)
 set(JSON_Diagnostics ON CACHE BOOL "Enable json diagnostics" FORCE)
 set(NUI_JSON_DIAGNOSTICS ON CACHE BOOL "Enable json diagnostics for nlohmann_json through nui" FORCE)
@@ -88,6 +91,9 @@ if (WIN32)
     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)
+    # activeCodePage UTF-8: narrow Windows APIs in third-party code (libssh, yaml-cpp, spdlog) read UTF-8.
+    set(NUI_SFTP_MANIFEST "${CMAKE_CURRENT_LIST_DIR}/_cmake/windows/nui-sftp.manifest")
+    file(TO_CMAKE_PATH "${NUI_SFTP_MANIFEST}" NUI_SFTP_MANIFEST_FORWARD)
     configure_file(
         "${CMAKE_CURRENT_LIST_DIR}/_cmake/windows/nui-sftp.rc.in"
         "${CMAKE_BINARY_DIR}/generated/nui-sftp.rc"
@@ -96,7 +102,7 @@ if (WIN32)
     target_sources(${PROJECT_NAME} PRIVATE "${CMAKE_BINARY_DIR}/generated/nui-sftp.rc")
     set_source_files_properties(
         "${CMAKE_BINARY_DIR}/generated/nui-sftp.rc"
-        PROPERTIES OBJECT_DEPENDS "${NUI_SFTP_ICO}"
+        PROPERTIES OBJECT_DEPENDS "${NUI_SFTP_ICO};${NUI_SFTP_MANIFEST}"
     )
     add_dependencies(${PROJECT_NAME} nui-sftp-icon)
 elseif (APPLE AND NOT EMSCRIPTEN)
@@ -199,6 +205,10 @@ else()
         add_subdirectory(shared_data/benchmark/shared_data)
         add_subdirectory(tar-archive/test/tar_archive)
         add_subdirectory(command-store/test/command-store)
+        add_test(
+            NAME NarrowPathConversions
+            COMMAND "${CMAKE_COMMAND}" "-DSOURCE_DIR=${CMAKE_SOURCE_DIR}" -P "${CMAKE_SOURCE_DIR}/_cmake/check_narrow_path_text.cmake"
+        )
     endif()
 
     if (NUI_SFT_ENABLE_LANGUAGE_TOOLING)

_cmake/check_narrow_path_text.cmake

@@ -0,0 +1,45 @@
+# Fails when native first-party sources call string() or generic_string(), which convert a path through the ANSI code
+# page on Windows and throw outside it. Paths::nativeToUtf8 and Paths::nativeFromUtf8 convert explicitly. Frontend
+# sources are not checked, a path there is UTF-8 already. Run with -DSOURCE_DIR=<repository root>.
+
+set(roots
+    backend
+    command-store/include
+    command-store/source
+    log
+    nui-file-explorer/source/nui-file-explorer/support
+    paths/include
+    persistence/include
+    persistence/source
+    shared_data/include
+    shared_data/source
+    ssh/include
+    ssh/source
+    tar-archive/include
+    tar-archive/source
+    utility/include
+    utility/source
+)
+
+set(found "")
+foreach(root IN LISTS roots)
+    file(GLOB_RECURSE sources "${SOURCE_DIR}/${root}/*.cpp" "${SOURCE_DIR}/${root}/*.hpp" "${SOURCE_DIR}/${root}/*.mm")
+    foreach(source IN LISTS sources)
+        file(RELATIVE_PATH relative "${SOURCE_DIR}" "${source}")
+        if (relative MATCHES "(^|/)test/")
+            continue()
+        endif()
+        file(STRINGS "${source}" lines REGEX "\\.(generic_)?string\\(\\)")
+        foreach(line IN LISTS lines)
+            if (NOT line MATCHES "^[ \t]*//")
+                string(STRIP "${line}" stripped)
+                string(APPEND found "\n  ${relative}: ${stripped}")
+            endif()
+        endforeach()
+    endforeach()
+endforeach()
+
+if (found)
+    message(FATAL_ERROR "Narrow path conversions, use Paths::nativeToUtf8 or Paths::nativeFromUtf8:${found}")
+endif()
+message(STATUS "No narrow path conversions in native sources.")

_cmake/windows/nui-sftp.manifest

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
+  <application xmlns="urn:schemas-microsoft-com:asm.v3">
+    <windowsSettings>
+      <activeCodePage xmlns="http://schemas.microsoft.com/SMI/2019/WindowsSettings">UTF-8</activeCodePage>
+    </windowsSettings>
+  </application>
+</assembly>

_cmake/windows/nui-sftp.rc.in

@@ -4,6 +4,9 @@
 // (Windows picks the lowest-numbered ICON resource).
 1 ICON "@NUI_SFTP_ICO_FORWARD@"
 
+// The application manifest: CREATEPROCESS_MANIFEST_RESOURCE_ID (1) of type RT_MANIFEST (24).
+1 24 "@NUI_SFTP_MANIFEST_FORWARD@"
+
 VS_VERSION_INFO VERSIONINFO
  FILEVERSION    @SEMVER_MAJOR@,@SEMVER_MINOR@,@SEMVER_PATCH@,0
  PRODUCTVERSION @SEMVER_MAJOR@,@SEMVER_MINOR@,@SEMVER_PATCH@,0

backend/include/backend/dropped_entry.hpp

@@ -6,7 +6,9 @@
 #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, nothing when it is not an absolute path.
+ * @brief Describes a file dropped onto the window from the desktop, with just enough detail for the transfer. A
+ *        dropped link or junction is a symlink, never what it points to.
+ * @param path Local path of the dropped item. Nothing, and a log line, when it is not an absolute path or cannot be
+ *        inspected.
  */
 std::optional<SharedData::DroppedItem> droppedItemFromPath(std::filesystem::path const& path);

backend/include/backend/existence_check.hpp

@@ -3,6 +3,7 @@
 #include <ssh/sftp_session.hpp>
 
 #include <chrono>
+#include <cstddef>
 #include <expected>
 #include <string>
 #include <vector>
@@ -14,8 +15,15 @@
 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.
+ * @brief How many queried children of one remote directory are checked by listing it instead of an lstat each.
+ */
+constexpr std::size_t remoteListingThreshold = 10;
+
+/**
+ * @brief Whether each of @p paths exists on the server, with the same rules as localPathsExist. The children of a
+ *        directory with at least @ref remoteListingThreshold of them queried are checked with one listing of it
+ *        (names only), the others with one lstat each. A directory that cannot be listed for its permissions has its
+ *        children looked up one by one, and so has a name only a case or normalization variant of is listed.
  */
 std::expected<std::vector<bool>, std::string> remotePathsExist(
     SecureShell::SftpSession& session,

backend/include/backend/local_delete_guard.hpp

@@ -2,55 +2,33 @@
 
 #include <paths/local_path.hpp>
 
-#include <fmt/format.h>
-
 #include <expected>
-#include <optional>
-#include <ranges>
 #include <string>
-#include <string_view>
+#include <vector>
 
 /**
- * @brief Refuses a root directory and @p home or a directory containing it.
+ * @brief Refuses an unset path, a root directory, and each of @p homes or a directory containing one.
+ *
+ * Paths are compared as text, and without ASCII case for Windows paths and on macOS, whose filesystems fold case.
+ * A target that is an existing directory (not a link to one) is also compared by file identity with every home and
+ * each of its ancestors, which catches other spellings of the same directory: case beyond ASCII, 8.3 names, NFD
+ * names and ancestors reached through a symlink.
  */
-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 {};
-}
+std::expected<void, std::string>
+refuseProtectedLocalDelete(Paths::LocalPath const& target, std::vector<Paths::LocalPath> const& homes);
 
 /**
- * @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.
+ * @brief A local path that may be deleted, or the reason to refuse it: not a valid absolute path, containing '..'
+ *        (also as '\\..' on Windows) or refused by @ref refuseProtectedLocalDelete.
  */
-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);
-}
+std::expected<Paths::LocalPath, std::string>
+localDeleteTarget(std::string const& utf8, std::vector<Paths::LocalPath> const& homes);
 
 /**
- * @brief The home directory of the user running the backend, empty when it cannot be determined.
+ * @brief The home directories of the user running the backend, which the delete guards protect, each once.
+ *
+ * On Windows the profile folder (FOLDERID_Profile) and the USERPROFILE and HOME variables, elsewhere the HOME
+ * variable and the home of the passwd entry. When none can be determined a warning is logged and only roots stay
+ * protected. Never throws.
  */
-std::optional<Paths::LocalPath> localHomeDirectory();
+std::vector<Paths::LocalPath> localHomeDirectories();

backend/include/backend/local_operation_queue.hpp

@@ -0,0 +1,152 @@
+#pragma once
+
+#include <backend/rpc_helper.hpp>
+#include <backend/sftp/local_delete_operation.hpp>
+#include <ids/ids.hpp>
+#include <paths/local_path.hpp>
+#include <shared_data/file_operations/compared_state.hpp>
+#include <shared_data/file_operations/operation_completed.hpp>
+#include <shared_data/file_operations/operation_mode.hpp>
+
+#include <nui/rpc.hpp>
+#include <boost/asio/any_io_executor.hpp>
+#include <boost/asio/strand.hpp>
+
+#include <atomic>
+#include <deque>
+#include <expected>
+#include <functional>
+#include <memory>
+#include <optional>
+#include <string>
+#include <string_view>
+#include <vector>
+
+/**
+ * @brief Runs local deletes apart from the SSH sessions, so closing a tab or losing a connection never stops one.
+ *
+ * The deletes run one after another on the strand the queue shares with the sessions, a work step at a time, and
+ * report to the frontend under "LocalOperationQueue::...". A delete a session started waits while the queue of that
+ * session is paused, for as long as the session exists.
+ */
+class LocalOperationQueue
+    : public RpcHelper::StrandRpc
+    , public std::enable_shared_from_this<LocalOperationQueue>
+{
+  public:
+    using OperationCompleted = SharedData::OperationCompleted;
+
+    struct DeleteRequest
+    {
+        Ids::OperationId operationId{};
+        std::vector<Paths::LocalPath> targets{};
+        /** @brief See LocalDeleteOperation::LocalDeleteOperationOptions::recursive. */
+        bool recursive{false};
+        /** @brief The card refreshes the local listing when the delete ends. */
+        bool insertRefresh{false};
+        /** @brief A priority delete runs before the others and is never held. */
+        SharedData::OperationMode mode{SharedData::OperationMode::Queued};
+        /** @brief The session whose paused queue holds the delete, none for a delete without a session. */
+        std::optional<Ids::SessionId> owner{};
+        /** @brief The queue of @ref owner is paused, so the delete starts held. */
+        bool held{false};
+        /** @brief See LocalDeleteOperation::LocalDeleteOperationOptions::comparedStates. */
+        std::vector<SharedData::ComparedState> comparedStates{};
+    };
+
+    LocalOperationQueue(
+        boost::asio::any_io_executor executor,
+        std::shared_ptr<boost::asio::strand<boost::asio::any_io_executor>> strand,
+        Nui::Window& wnd,
+        Nui::RpcHub& hub,
+        std::shared_ptr<std::atomic_bool const> localDeletionPrevented
+    );
+    ~LocalOperationQueue() override = default;
+    LocalOperationQueue(LocalOperationQueue const&) = delete;
+    LocalOperationQueue& operator=(LocalOperationQueue const&) = delete;
+    LocalOperationQueue(LocalOperationQueue&&) = delete;
+    LocalOperationQueue& operator=(LocalOperationQueue&&) = delete;
+
+    /**
+     * @brief Handles "LocalOperationQueue::addDelete" (operation id, local paths), the delete of explorer items
+     *        without a session, and "LocalOperationQueue::cancel" (operation id).
+     */
+    void registerRpc();
+
+    /**
+     * @brief Queues a delete with one card, labelled with the directory its targets lie in. Refused as a whole when
+     *        one target is a root directory or contains a home directory. Assumed in strand.
+     */
+    std::expected<void, Operation::Error> addDelete(DeleteRequest request);
+
+    /**
+     * @brief Queues the delete of the explorer items @p pathsUtf8 with their content, as @ref addDelete with a card
+     *        that refreshes the local listing. Refused when deletion is prevented in the settings or a path is not
+     *        an item that may be deleted. Assumed in strand.
+     *
+     * @param owner The session whose explorer asks, none without one.
+     * @param held The queue of @p owner is paused.
+     * @return A message for Utility::localizedMessage or the language file saying why nothing was queued.
+     */
+    std::expected<void, std::string> addExplorerDelete(
+        Ids::OperationId operationId,
+        std::vector<std::string> const& pathsUtf8,
+        std::optional<Ids::SessionId> owner,
+        bool held
+    );
+
+    /**
+     * @brief Holds the deletes of @p owner, or lets them run again. Assumed in strand.
+     */
+    void hold(Ids::SessionId const& owner, bool held);
+
+    /**
+     * @brief The deletes of @p owner run on their own from now on, never held again. Assumed in strand.
+     */
+    void release(Ids::SessionId const& owner);
+
+    /**
+     * @brief Ends the delete @p operationId as canceled and tells the frontend. Assumed in strand.
+     */
+    void cancel(Ids::OperationId const& operationId);
+
+    /**
+     * @brief Works the first delete that is not held for one step and tells the frontend when it ends. Assumed in
+     *        strand; the queue drives itself, this is public for tests.
+     *
+     * @return True while a delete that is not held is left.
+     */
+    bool work();
+
+    /**
+     * @brief Test hook: @p observer sees every completion right before it is sent to the frontend.
+     */
+    void setCompletionObserver(std::function<void(OperationCompleted const&)> observer);
+
+  private:
+    struct QueuedDelete
+    {
+        Ids::OperationId operationId;
+        std::unique_ptr<LocalDeleteOperation> operation;
+        std::optional<Ids::SessionId> owner;
+        bool held;
+        bool priority;
+    };
+
+    /**
+     * @brief Posts the next work step unless one is posted already.
+     */
+    void schedule();
+
+    void complete(
+        QueuedDelete const& queued,
+        SharedData::OperationCompletionReason reason,
+        std::optional<Operation::Error> error
+    );
+    std::string rpcName(std::string_view event) const;
+
+    std::shared_ptr<std::atomic_bool const> localDeletionPrevented_;
+    std::deque<QueuedDelete> deletes_{};
+    bool scheduled_{false};
+    std::function<void(OperationCompleted const&)> completionObserver_{};
+};