Repository navigation
Improve file operation safety Part 2 - #238
Merged
Merged
Conversation
A local rename without replacing falls back where the filesystem refuses RENAME_NOREPLACE, RENAME_EXCL or hard links (FUSE, vfat, non-APFS volumes): a regular file moves by link and unlink, anything else by an existence check and a rename; Utility::moveIntoPlace chooses between replacing and not replacing
Remote renames without replacing fall back to a rename when the server refuses the hard link (vfat, CIFS, FUSE, protected_hardlinks), and a moved file whose source cannot be removed afterwards counts as moved; without posix-rename the server's own rename refuses an existing destination, so the destination is only checked after a failure. Replacing without posix-rename removes a leftover aside, puts back a destination that only exists aside, and names the aside when moving it back fails; moveIntoPlaceInStrand chooses between the two
Finished transfers and renames move into place through one dispatcher per side, Utility::moveIntoPlace and SftpSession::moveIntoPlaceInStrand, and the error mapping lives in Operation; a remote rename that replaces moves the old item aside under the configured upload temp suffix instead of a fixed .nui-sftp-old, and uploads pass the known file type so the no-replace move skips an lstat
A queued remote rename that timed out runs later with its own copies of the paths instead of the freed operation, and transfers wait for their queued strand tasks before they are destroyed (archives and bulks too, not only while a stream is open), without throwing on a finalized strand
A download with reserved space only replaces the destination when every byte arrived: the bytes written are counted for synchronous and asynchronous reads, a read ending early fails with SizeMismatch, and a resumed or already complete partial file reserves nothing; the temp path is built once and its cleanup no longer throws
Utility::restrictToOwner creates or limits a file to owner read and write without following a link, and Utility::newFilePermissions gives the mode the system would give a new file
A download gets its modification time and permissions while it is still the temp file, which only its owner can read until then, and an upload sets the remote modification time on its temp file, so a failure there never reports failure after the destination was replaced; an upload whose finished temp file cannot be moved into place removes it
A downloaded symlink is created under the temp name and moved into place, so it replaces an existing item when overwriting is allowed instead of failing on it; checking for an existing destination no longer throws
A bulk download applies inherited or configured directory permissions once every entry is done, children before their parents, so a read-only directory still receives its files instead of failing them as skippable
The tar writer gives a member with unknown permissions the mode of a new one (0644, 0755 for a directory, 0777 for a symlink) instead of 07777
An uploaded archive never carries setuid, setgid or sticky bits, and members whose permissions are unknown, as on Windows, get the tar writer's defaults instead of 0777
AsyncTransferContext answers hasEnded from its outcome, the separate ended flag is gone
Archive downloads check for an existing archive and remove their temp file without the throwing filesystem overloads
The process umask is read at the start of main, before any thread creates files, since reading it outside Linux sets it for a moment
PathError::toString writes control bytes and every byte outside a valid UTF-8 sequence as \xNN, so its text is always UTF-8; paths split segments with one splitter, build their text once, validate a segment only where it enters, and joining onto an unset RemotePath gives an unset path instead of undefined behaviour; a PathSegment reads from JSON
Windows device names include COM0, LPT0, COM and LPT with a superscript digit, CONIN$ and CONOUT$, also with spaces before the extension, and a Windows name may be at most 255 UTF-16 code units; device names are upper-cased as ASCII only
A Windows LocalPath has an upper-case drive letter and no \\?\ prefix (\\?\UNC\server\share becomes //server/share), device and namespace paths like \\.\PhysicalDrive0 or a server named '.', '?' or '..' are refused, NUL is refused in the root too, and isWithinIgnoringCase compares ASCII letters without case for the delete guards; joining onto an unset LocalPath fails with Empty
The local delete guard also compares case-insensitively for Windows paths and on macOS, and refuses an existing directory that is a home or one of its ancestors by file identity (other case, 8.3 and NFD names, symlinked ancestors); it protects every home it finds (Windows: FOLDERID_Profile through SHGetKnownFolderPath, USERPROFILE, HOME; elsewhere HOME and the passwd entry), logs when there is none, never throws, splits on backslashes only for Windows paths and names the reason with PathError::toString
SftpSession canonicalizes a path through the server (canonicalPathInStrand) and reports the start directory without any fallback (canonicalHomeDirectoryInStrand), which homeDirectoryInStrand builds on; the mock server's REALPATH resolves symlinks like realpath
Remote deletes are checked by a RemoteDeleteGuard: it canonicalizes each target's parent on the server (once per parent and request, once per sync root) so a symlinked ancestor cannot hide the home, also refuses the home without regard to ASCII case and, on servers with Windows paths, names with a backslash or a trailing dot or space; an unknown home is looked up without the / fallback and logged. The three delete RPCs refuse through one refuseAnyProtected and answer with a localized reason that says nothing was deleted; the queue checks once more with the same guard
A sync delete of a protected directory also refuses every planned entry below it, remotely and locally, instead of deleting its content, and the sync dialog paints only the entries that failed; the local delete operation leaves the guard check to the queue
SftpSession checks the strand and unset paths of an in-strand call with one requireSetInStrand for all its paths, and a listing takes symlink names from its entries instead of a second list
The frontend asks the backend for its local path platform before it loads the state, so local paths read at startup follow the backend's rules instead of POSIX ones
The docs of BulkDeleteRequest and DeleteOperation::appendScanResult describe one delete operation per request and how a scan result is checked
paths/utf8.hpp is the one UTF-8 module: besides validation it sanitizes (U+FFFD for every byte outside a valid sequence), truncates at a code point boundary and converts to and from UTF-16 (wide text on Windows) without throwing; Utility::Utf8 decodes and encodes through it, and the snippet error excerpt and the typed line backspace cut with truncatedUtf8
Native paths convert to UTF-8 without throwing: nativeToUtf8 replaces bytes that are not UTF-8 (POSIX) and unpaired surrogates (Windows) with U+FFFD, convertNativeToUtf8 also says whether that happened, checkedNativeToUtf8 and LocalPath::make refuse such text with InvalidUtf8, and nativeFromUtf8 converts through the wide text on Windows; Utility::errorMessage gives an error code or filesystem error as UTF-8, read with FormatMessageW for Windows system errors
A listed name that is not UTF-8 is kept for display with U+FFFD and nameIsValidUtf8 false, and DirectoryEntry::nameSegment refuses it, so walkers, sync scans and explorer actions reject it as InvalidName and never act on a different item called like its display text; server owner, group, ACL and link literals and local owner and group names are sanitized the same way, a downloaded link whose literal is not UTF-8 and an uploaded link whose target is not are refused, and the mock server can list raw name bytes
Calls into the frontend never throw on text that is not UTF-8: RpcHelper::callRemoteSafely sends such a call again with U+FFFD for every invalid byte and logs any other failure, RPC replies, operation queue and process events and the session, main and file tracking calls go through it, and the logger forwards sanitized messages; the log file is opened by its native name, wide on Windows
Error texts and local paths reach logs, replies, operation errors and the UI as UTF-8: error codes and exceptions go through Utility::errorMessage, paths through nativeToUtf8 and nativeFromUtf8 instead of the narrow string(), which uses the ANSI code page on Windows and throws outside it; language files are read from a stream opened by the native path, environment variables, the user name, known folders and drive labels convert from their wide text on Windows, and the test temporary directory is handed over with the wide environment API
Starting a process looks up and runs the executable by its native name, and on Windows passes the arguments and the environment as wide text converted from UTF-8, so no part of the command goes through the ANSI code page
5cript
enabled auto-merge (squash)
October 16, 2026 13:47
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 2 of 3: review fixes for transfers, paths, UTF-8, local deletes, sync and the queue.
paths/utf8.hppmodule, native conversions that never throw, a UTF-8 code page manifest and console on Windows, and the NarrowPathConversions ctestremoveSomeno longer removes treesUtility::LifetimeToken