fix(vfs): data safety, SSH policy and liveness fixes from the pre-merge review

Data safety (VFS):
- Copies write a temporary sibling and replace the destination only
  once it is complete (as writeAll() already did): a failed overwrite
  kept nothing of the old destination, and copying onto a hard link to
  the source truncated the source itself. A destination symlink is
  written through, and one that resolves to the source fails as the
  same file.
- Moving a symlink to another file system is refused: the fallback copy
  followed it and then removed the copied entries through the link,
  emptying its target.
- Saves keep the exact mode of the file they replace: the temporary was
  created with it, but filtered through the umask.

Deadlocks and blocking:
- writeAll( createParents ) waited on makeDirs() queued on its own
  orchestration pool: one thread deadlocked at once, two with two
  concurrent saves. Orchestration jobs now create parents directly.
- Copies and moves run on their own pool (Config::transferThreads), so
  long transfers no longer delay saves, reads and recursive removals.
- Local watches are registered and removed on the I/O pool: efsw and the
  directory checks touch the filesystem, which blocked the UI thread on
  slow mounts.

SSH security:
- "@revoked" known_hosts keys are rejected; they were skipped as
  unknown, so StrictHostKeyChecking accept-new trusted them.
- Sessions are shared only between configurations with the same
  security policy (known hosts, host key checking, algorithms,
  authentication settings), not just the same destination: a strict
  alias reused a session a permissive one had accepted.
- A configured algorithm restriction that permits nothing this libssh2
  supports fails the connection instead of using libssh2's defaults.

SSH liveness:
- Operations submitted to a failed or closed connection fail with
  ConnectionLost instead of staying pending forever.
- ssh-agent requests run on a worker thread (SSHAgent, Unix-domain
  sockets), with libssh2_userauth_publickey() and a sign callback that
  waits without blocking: an agent waiting for a hardware key touch no
  longer stalls every other session. Windows keeps libssh2's agent.
- Connecting no longer holds a helper thread while the user answers a
  host key or password prompt: the setup continues from the session's
  ready task, one ProxyJump hop at a time. Jump hosts only need to
  authenticate, not to offer SFTP.
- Idle connections close after SFTPProviderConfig::idleTimeout. The old
  check compared use_count() with 1 and never passed: connections now
  track their users (open handles, pumps, jump children).
- A remote watch agent that never answers falls back to polling after
  5 seconds; the directory listing waited for it indefinitely.

Core:
- CancellationSource::cancel() keeps the shared state alive while its
  callbacks run: a callback destroying the source freed it mid-cancel
  (heap use after free under AddressSanitizer).
- MainThreadExecutor::shutdown() detaches the executor from its
  dispatcher node and drops queued and later functions.
- String::icompare(): case-insensitive comparison without allocations.

VFSDirectoryModel (UI thread):
- Names compare with String::icompare() instead of lowercased copies,
  which made every sort, merge and insertion allocate (about 2x slower
  when sorting).
- Sorting by type asks the type provider once per entry instead of
  twice per comparison.
- The case-insensitive plain text filter keeps a search table instead
  of lowercasing every name on each row rebuild.
- Watch events update the visible rows in place instead of rebuilding
  them from every entry per event.

Drag and drop:
- The X11 drag source showed the pending badge over the source window
  for every application, which broke the X11 drag and drop integration
  tests in CI. DragOffer::sourceHandlesDrops now asks for it; eefm sets
  it, since it completes those drops as its own drag.

eefm:
- Shutdown destroyed the main thread executor before the VFS and the
  thread pool while continuations still held it: closing during a
  transfer could dispatch through freed memory. Transfers are cancelled,
  the VFS is destroyed, the executor is detached, and only then are the
  scene, its pool and the executor destroyed.
- Remote files opened locally were downloaded to
  /tmp/eefm/<scheme>/<host>/<name>: files with the same name from other
  folders, users or ports overwrote each other, in a directory shared by
  every user. They now go to a per-file folder (a hash of the complete
  URI) in the user's cache.

Every fix comes with a regression test.
This commit is contained in:
Martín Lucas Golini
2026-10-15 20:33:13 -03:00
parent 8827f8de86
commit 0e7e12ba76
38 changed files with 2448 additions and 500 deletions
+4
View File
@@ -539,6 +539,10 @@ class EE_API String {
static bool iequals( String::View str1, String::View str2 );
/** Compares ignoring ASCII case, without allocating (as comparing String::toLower() copies
* would). @return Negative, zero or positive, like std::string::compare(). */
static int icompare( std::string_view str1, std::string_view str2 );
/** Creates a random string using the dictionary characters. */
static std::string randString(
size_t len,
+8 -2
View File
@@ -15,8 +15,9 @@ class Node;
* deferred to the next scene update, even when execute() is called from the main thread, which is
* what Task::thenOn() requires.
*
* The dispatcher node must outlive the executor. Destroying the executor drops queued functions
* that have not run yet (releasing what they own, for example abandoning task sources).
* The dispatcher node must outlive the executor, or shutdown() must be called before the node is
* destroyed. Destroying the executor drops queued functions that have not run yet (releasing what
* they own, for example abandoning task sources).
*/
class EE_API MainThreadExecutor : public Executor {
public:
@@ -30,6 +31,11 @@ class EE_API MainThreadExecutor : public Executor {
void execute( Function function ) override;
/** Detaches the executor from its dispatcher node and drops the queued functions. Functions
* executed afterwards are dropped too. Call it before the dispatcher node is destroyed when
* work may still be executed on this executor (it must then outlive that work). */
void shutdown();
/** @return Number of functions waiting for the main thread. */
size_t pending() const;
+7
View File
@@ -42,6 +42,13 @@ class EE_API Sys {
/** @return A storage path for config files for every platform */
static std::string getConfigPath( const std::string& appname );
/** @return A storage path for cache files (data the application can recreate) for every
* platform, private to the user: $XDG_CACHE_HOME/appname (~/.cache/appname) on Linux and BSD,
* ~/Library/Caches/appname on macOS and iOS, %LOCALAPPDATA%\appname on Windows,
* ~/config/cache/appname on Haiku. Elsewhere, or without those variables, a directory in the
* temporary path. The directory is not created. */
static std::string getCachePath( const std::string& appname );
/** @return The path of the directory designated for temporary files. */
static std::string getTempPath();
+3
View File
@@ -47,6 +47,9 @@ struct DragOffer {
SmallVector<DragFile, 1> files;
DragActions actions{ static_cast<DragActions>( DragAction::Copy ) };
DragPreview preview;
/** The application completes drops on its own window (DragResult::releasedOverSource on
* X11): the preview shows a pending drop there instead of a rejected one. */
bool sourceHandlesDrops{ false };
};
struct DragResult {