Repository navigation
server: do not let SSH_FXP_RENAME replace an existing file - #704
Merged
Merged
Conversation
The os-backed Server handled SSH_FXP_RENAME with a bare os.Rename, which replaces newpath when it already exists. The SFTP v3 draft makes that an error, OpenSSH's sftp-server refuses it with SSH_FX_FAILURE, and the request server's InMemHandler already enforces it, so a client relying on RENAME to never clobber (upload to a temporary name and rename into place, or rename as a lock between two writers) silently lost the existing file against this server. Refuse the request when newpath exists. posix-rename@openssh.com keeps its overwriting behaviour.
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.
Repro: against the Go
Server,client.Rename(old, new)wherenewalready exists returns OK andnewnow holds the contents ofold. OpenSSH's sftp-server answers the same request with SSH_FX_FAILURE and leavesnewuntouched.Cause:
handlePacketpassed SSH_FXP_RENAME straight toos.Rename, which replaces its target. The v3 draft makes RENAME onto an existing name an error,InMemHandlerandTestRequestRenamealready enforce that for the request server, andClient.PosixRenameis documented as the form that replaces, so a client using RENAME as a no-clobber step (upload to a temporary name and rename into place, or rename as a lock between two writers) lost the existing file against this server.Fix: handle the packet in an
sshFxpRenamePacket.respondthat refuses with EEXIST when newpath exists, as OpenSSH does, and leaves posix-rename@openssh.com overwriting. AddsTestRenameDoesNotOverwrite, which fails on the current server and passes against OpenSSH's sftp-server.make integrationandmake integration_w_racepass locally on macOS with Go 1.26.10.