Repository navigation
Improve file operation safety Part 1 - #233
Merged
Merged
Conversation
5cript
enabled auto-merge (rebase)
October 15, 2026 23:43
5cript
disabled auto-merge
October 15, 2026 23:44
5cript
force-pushed
the
fix/file-operation-safety-part1
branch
from
October 16, 2026 12:36
f095ebb to
c849526
Compare
5cript
enabled auto-merge (rebase)
October 16, 2026 12:36
Added fault injection, extension advertising and fixture commands to the mock SFTP server, with acknowledged test commands
Failed transfers no longer replace the destination, an SFTP read or write error or a short remote file ended async transfers like a success and finalized the partial file over the original Async transfers now end as completed, failed or canceled and carry the error. Downloads check the received size before the rename, uploads check that every byte was sent. Resumed async transfers request only the missing bytes, a complete partial download is finalized without reading, a followed symlink downloads with the size of its target, and short archive entries are errors instead of zero padding.
Folded scans into the operations that consume them, a scan handed its tree to whatever delete or download followed it in the queue, so cancelling or moving a card could delete a directory the user cancelled Directory downloads and uploads, recursive deletes and archives now own their scan and run it before their own work. The unit is one card with a scanning phase, cancel, reorder and failure act on both halves, and a missing or failed scan fails the unit instead of completing it with nothing done. Standalone scans only serve sync sessions and are matched by id. A directory upload completes under the id the frontend gave it. Remote recursive deletes classify the target with lstat, deleting a symlink to a directory removes the link and leaves the target alone.
Sync no longer plans anything for entries it cannot see into, an unreadable local directory listed as empty and its remote content was planned for deletion Scans keep a directory below the root whose listing is denied or which vanished as unreadable instead of empty or failing, and keep sockets, devices, FIFOs and Windows symlinks as excluded placeholders instead of dropping them. The diff leaves such names alone on both sides, as well as a directory facing a symlink, which would transfer through the link. Plans skip them inside transferred directories too. The dialog names the entries it leaves alone, the confirmation states how many items are deleted on which side and defaults to No, and the delete arrow of a row asks first. Downloads refuse a symlink at the temp file path, remote ignore rules resolve their directory lexically.
Transfers no longer apply unknown or special permissions, a directory download with inherited permissions made its target directory 07777 Unknown permissions, which the scan root always has, are not applied any more. Inherited permissions keep only the read, write and execute bits and honor the umask, so setuid, setgid and sticky never travel. Local scans record permissions, uploads inherit those of a followed symlink's target instead of the link's 0777.
Overwrites and renames never remove the existing item first, an upload deleted the remote file before renaming the new one into place and renames replaced existing items silently Uploads, symlink uploads and archive uploads finish with a rename that replaces atomically with posix-rename, or moves the destination aside and back when the server lacks it. Without permission to overwrite the rename itself refuses an existing destination, through hardlink@openssh for files where available, and locally through renameat2, renamex_np or MoveFileExW without replacing. Archive uploads write to a temp file that is removed when they fail. Renaming in the file explorer validates the new name, works on the item's own path and asks before replacing an existing item. The backend refuses renames that would move an item to another directory. SFTP rename, readlink, lstat and symlink calls pass UTF-8 paths.
Resumed downloads take the partial file's size from disk, tellp after opening in append mode reported 0 on Windows and the whole file was appended again
Added the paths target with LocalPath, RemotePath, RelativePath and validated path segments, made converting a plain Id into a typed id explicit RemotePath is UTF-8 with '/' as the only separator and never passes through std::filesystem, so backslashes and drive letters in server names stay characters on Windows. PathSegment refuses empty names, '.', '..', separators, NUL and invalid UTF-8, LocalPathSegment adds the Windows naming rules. RelativePath is the only way to map one side onto the other and cannot leave its root.
Validated names from directory listings, a server name like 'x/..' or '..' became a path outside the transferred or deleted directory Flat scans drop every '.' and '..' and set entries with names that are not a single path segment aside, transfers report them as failed. Sync trees keep them as invalid names and never act on them, remote trees also mark names that cannot exist locally, like 'a\b' or 'CON' on Windows. Directory downloads refuse such names before creating anything. SFTP listings decode names as UTF-8 instead of the ANSI code page.
Closed the mock server's stdin after the exit command, node on Windows cannot finish exiting while a read on its stdin pipe is pending
Drained the mock server's stderr in tests, on Windows the unread pipe filled up and blocked node until the kill timer, so every mock test took ten seconds
Set the result of a full read only after its last callback, the waiting caller could leave and the end-of-file callback wrote into its destroyed locals The transfer failure tests crashed or hung in about half of the runs under load. The ignore-file reader of the remote scan captures its buffer the same way.
Moved the SftpSession API to RemotePath, an unset or invalid remote path is refused before anything reaches the server Remote paths from the frontend are validated in the RPC handlers, and transfers validate theirs when they are created, so '..', relative paths and the root fail the operation instead of acting somewhere else. Remote paths no longer pass through std::filesystem::path, which read narrow strings in the ANSI code page and treated backslashes as separators on Windows. Relative symlink targets now resolve from the link's directory instead of the server's working directory, and a downloaded symlink keeps the link's own content, so relative links still point into the downloaded tree. The operations convert their std::filesystem::path options through a bridge until the options carry RemotePath themselves.
Added a bulk delete request that carries RemotePath targets, a relative or '..' target fails to parse instead of reaching the queue
Typed deletes on the backend with RemotePath, the root and the home directory or a directory containing it are refused DeleteOperation turns its scanned entries into RemotePath targets up front and refuses the whole delete when one of them is not below its root. The queuedRemove, deleteFiles and addBulkDelete RPCs validate every path before queuing anything. addBulkDelete now takes a BulkDeleteRequest, the frontend follows in the next commit.
Typed remote deletes in the frontend with RemotePath, from the explorer selection and the sync plan to the bulk delete request A selected name that does not stay in the current directory, like 'x/..', is refused instead of becoming a delete target. A file tracking deletion maps to a path below the tracked remote directory or is not mirrored, a local path outside the instance directory produced a '..' path before. The pre-delete check only sorts the asked directories and can no longer add targets.
Typed local deletes with LocalPath and refused a root directory, the home directory or a directory containing it, and paths with '..' The backend reports its platform with the version, so the frontend parses local paths by the backend's rules instead of WebAssembly's POSIX ones. removeSome checks every path before removing any.
Typed the sync roots as LocalPath and RemotePath, a row resolves to both sides below them or not at all openSyncSession refuses roots that are not absolute paths, and the remote and local root can no longer be swapped when the scans are queued. A relative key that the local platform cannot store, like 'CON' or 'a\\b' on Windows, does not resolve.
Made a bulk delete one operation with one card, its directories are scanned as roots of that operation Before, every directory of a bulk delete was its own operation with an id the frontend never saw, so a sync that only deleted directories never learned that they were gone. A symlink to a directory is still removed as a link.
Added a queued local delete operation, local deletes honour pause and show a card like remote ones It refuses the whole delete when one target is a root directory or contains the home directory.
Executed sync selections in the backend, it plans the rows and resolves them below its own roots instead of sending absolute paths through the frontend executeSyncSelection replaces buildSyncEnqueuePlan. The frontend names the operation ids, the backend queues files of each direction as one bulk transfer and remote and local deletes as one operation each, and reports which rows each covers. Directories without content are created with their missing parents, an empty nested directory failed before because its parent did not exist yet. Plan entries carry no paths any more. The frontend moves to the new call in the next commit and does not build in between.
Moved the sync dialog to executeSyncSelection, it names the operation ids, listens to them and maps the answer to rows A completion that arrives before the answer is kept and applied once the rows are known. The dialog no longer handles absolute paths, and the unqueued local remove call is gone: local deletes run as a queued operation with a card.
Removed the frontend's direct directory creation calls, the backend creates sync directories now
Added typed bulk upload and download requests, a local and a remote path can no longer be swapped in a bulk entry
Typed the roots and entries of bulk transfers, every entry resolves below both roots or is recorded as failed
Typed the queue's transfer entry points and bulk RPCs, the backend refuses invalid bulk paths with an error reply and resumes know which kind of bulk they hold
Closed the metadata stream in DeadTimestampWrittenOnDestruction before removing the instance directory, it failed on Windows
The GNU tar interop tests pass --force-local to GNU tar and give it forward slashes, a Windows drive path read as a remote host and a backslashed -C made them fail with MSYS tar
…modes Tests for an archive entry shorter than scanned, an archive download whose scan fails, the default mode of uploaded directories and in-place rename validation; the mock server keeps the mode a MKDIR asks for
5cript
force-pushed
the
fix/file-operation-safety-part1
branch
from
October 16, 2026 13:12
c849526 to
715288b
Compare
5cript
disabled auto-merge
October 16, 2026 13:38
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Safer file operations, part 1 of 3: typed paths and transfer, delete and sync fixes.
pathstarget (LocalPath,RemotePath,RelativePath,PathSegment) used by SftpSession, RPCs, the queue, archives and the explorer..orx/..can never point outside the directory being worked on