Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sftp.Client.Rename() giving permission denied, but works with other sftp clients #479

Open
iamneal opened this issue Nov 2, 2021 · 6 comments

Comments

@iamneal
Copy link

iamneal commented Nov 2, 2021

I have written some code that takes:

  • user credentials
  • a sftp server to connect to
  • a filename to process

My code takes all this data, then:

  • makes an sftp.Client to the server using the user credentials
  • reads the file from the server
  • moves the file on the server to an archive so it isn't processed twice, or deletes it if the move fails

my code works fine for reading the file from the server, but when it attempts to do a client.Rename(filename, archiveFilename) it is giving me:

sftp: "Failed to rename file /EXAMPLE_FILE 2021-11-02.csv" (SSH_FX_FAILURE)

similarly when I attempt to do client.Remove(filename) I am getting:

sftp: "User <redacted> does not have directory make, remove or rename permissions." (SSH_FX_PERMISSION_DENIED)

I thought maybe the sftp server didn't give our user enough permissions to move/delete files, but the rename worked with both command line sftp OpenSSH_8.1p1, and Filezilla when using the same user creds

The user permissions on the server allow us to read, put, and rename files in its userspace. It cannot mkdir, but the archive dir already exists. I can't give much more detail on the server architecture, because I don't have access, but if there is any other info I can provide, I'll try

my code look pretty much like this:

func processFile(file string, client *sftp.Client) error {
  f, err := client.Open(file)
  if err != nil {
    return err
  }
  readFile(f)

  f.Close()

  err = client.Rename(file, archiveFilename)
  if err != nil {
    logError(err)
    err = client.Remove(file)
    if err != nil {
      logError(err)
    }
  }
  return nil
}

I appreciate any help on this, thanks

@puellanivis
Copy link
Collaborator

I've been in and out of hospital all week, and have been without internet due to a recent move. Just wanted to give an FYI on this thread. I've been thinking about it, but haven't had much I can poke at.

@puellanivis
Copy link
Collaborator

puellanivis commented Dec 15, 2021

I’ve been percolating on this for awhile. The only thing I can really think of is that the filenames are not being given as absolute? (i.e. sftp.Client does not naïvely support relative paths.) It’s hard to say without for instance example text from say OpenSSH_8.1p1. Seeing the actual commands issued, and the output resulting from it (obviously redacting secret information).

Understanding the full scope of what you’re trying to do as an example of sftp CLI commands can point to where assumptions may have been mistaken.

@iamneal
Copy link
Author

iamneal commented Feb 22, 2022

Ok so update on this, I figured out the cause.

Our application copies a bunch of files from a sftp server, does some processing on them. This particular file was opened, copied, but only the WriteCloser we were writing to was closed. The remote file was left open. When we later attempted to do the sftpclient.Rename(file, archivefile), it failed because that reader was left open.

So basically the error was caused by doing this:

func example(client *sftp.Client, filename, archiveFilename string)
	readCloser, _ := client.Open(filename)	

	// defer readCloser.Close()  <-- didn't do this

	if err := client.Rename(filename, archiveFilename); err != nil {
		log.Warn(...) // <-- the error we logged
	}
}

The bug ended up being on our end, but its too bad we couldn't get a more descriptive error when we attempted to do the rename on the open file. Failed to rename file /EXAMPLE_FILE 2021-11-02.csv" (SSH_FX_FAILURE) is pretty generic.

Regardless, since the error was not in this package, we can close this issue. Thanks for the help

@puellanivis
Copy link
Collaborator

puellanivis commented Feb 22, 2022

AAaaaaaah… yeah. Windows will do that sort of weird thing a lot. Lock a file basically and if you try and remove or delete it, it’s refused.

FYI: the defer readCloser.Close() probably wouldn’t help if it is in the same function call as a later Rename since the Close will not happen until after the Rename.

PS: On the error returned, the "Failed to rename file…” text comes from Windows itself, so there’s little we could even possibly do about improving the message. :(

@iamneal
Copy link
Author

iamneal commented Feb 22, 2022

ah good catch on the example, that is a bit off.

So as far as improving the error message, it seems like the reason we can't improve it, is because that error is thrown from the remote end, so there isn't really a way to detect if the error was due to an open file, or if it was due to something else. is that right?

would it be possible to track which files are opened by an sftp.Client, and then throw an error if the user attempts a sftpclient.Rename() on a file that has been opened by that same client? At least for my application, there shouldn't be a reason to even attempt to rename a file if it is being read somewhere else.

This might be an optimization best left to the user of the sftp package though, and not in the sftp.Client

@puellanivis
Copy link
Collaborator

Yes, the SSH_FX_FAILURE error is a generic error code, with a free-form string returned as the message. So, there’s not really a way to unpack that error message programmatically and give a more meaningful information to the client.

I suppose in principle it would be possible to keep a map of open filenames, but the costs greatly outweigh the very narrow benefit that might be produced… and in particular, on *nix (Linux, Darwin, BSDs, etc) you can rename an open file. So, we would have to also OS-detect what the server is running on and… 🤪 it gets out of hand really quick.

So, yeah, unfortunately, there’s not much we can help here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants