fix wrong error raised when private key is missing or invalid

.github/workflows/test.yml

@@ -27,6 +27,19 @@ jobs:
         uses: actions/setup-python@v6
         with:
           python-version: ${{ matrix.python-version }}
+      - name: Runner Environment
+        run: |
+          PASS=$(grep -m1 '^PASS = ' tests/common.py | cut -d\' -f2)
+          if [ -z "$PASS" ]; then
+            LINE=$(awk '/^PASS/{print NR; exit}' tests/common.py)
+            echo "::error file=tests/common.py,line=${LINE:-1},title=Test passphrase unreadable::No single-quoted value found for the PASS assignment."
+            exit 1
+          fi
+          PASSWORD="$(openssl rand -base64 33)"
+          echo "::add-mask::$PASSWORD"
+          echo "SFTPRETTY_KEY_PASS=$PASS" >> $GITHUB_ENV
+          echo "PASSWORD=$PASSWORD" >> $GITHUB_ENV
+        shell: bash
       - name: ${{ matrix.os }} SSH
         if: startsWith(matrix.os, 'macos')
         run: |
@@ -35,12 +48,12 @@ jobs:
       - name: ${{ matrix.os }} SSH
         if: startsWith(matrix.os, 'ubuntu')
         run: |
-          (echo ${{ secrets.PASSWORD }}; echo ${{ secrets.PASSWORD }}) | sudo passwd $USER
+          (echo "$PASSWORD"; echo "$PASSWORD") | sudo passwd $USER
       - name: ${{ matrix.os }} SSH
         if: startsWith(matrix.os, 'windows')
         run: |
           $authorizedKey = Get-Content -Path id_sftpretty.pub
-          $pass = ConvertTo-SecureString -AsPlainText -Force -String ${{ secrets.PASSWORD }}
+          $pass = ConvertTo-SecureString -AsPlainText -Force -String $env:PASSWORD
           $privateKey = Get-Content -Path id_sftpretty
           $user = Get-LocalUser -Name (([System.Environment]::UserName))
           $user | Set-LocalUser -Password $pass
@@ -58,7 +71,7 @@ jobs:
           Get-WindowsCapability -Online | Where-Object Name -like 'OpenSSH*'
           Set-Service -Name sshd -StartupType Automatic -Status Running
           Set-Service -Name ssh-agent -StartupType Automatic -Status Running
-          ssh-keygen -f $Key --% -N "" -p -P ${{ secrets.PRIVATE_KEY }}
+          ssh-keygen -f $Key --% -N "" -p -P %SFTPRETTY_KEY_PASS%
           if (!(Get-NetFirewallRule -Name 'OpenSSH-Server-In-TCP' -ErrorAction SilentlyContinue | Select-Object Name, Enabled)) {
               Write-Output "Firewall Rule 'OpenSSH-Server-In-TCP' does not exist, creating it..."
               New-NetFirewallRule -Name 'OpenSSH-Server-In-TCP' -DisplayName 'OpenSSH Server (sshd)' -Enabled True -Direction Inbound -Protocol TCP -Action Allow -LocalPort 22

sftpretty/__init__.py

@@ -13,7 +13,7 @@
 from pathlib import Path
 from sftpretty.exceptions import (CredentialException, ConnectionException,
                                   HostKeysException, LoggingException)
-from sftpretty.helpers import _callback, drivedrop, hash, localtree, retry
+from sftpretty.helpers import _callback, drivepath, hash, localtree, retry
 from socket import gaierror, timeout
 from stat import S_ISDIR, S_ISREG
 from tempfile import mkstemp
@@ -187,8 +187,11 @@ class Connection(object):
     :raises ConnectionException:
     :raises CredentialException:
     :raises HostKeysException:
+    :raises KeyError:
     :raises LoggingException:
+    :raises OSError:
     :raises PasswordRequiredException:
+    :raises PermissionError:
     :raises SSHException:
     '''
     def __init__(self, host, cnopts=None, default_path=None, password=None,
@@ -214,33 +217,39 @@ def _set_authentication(self, password, private_key, private_key_pass):
         if private_key is not None:
             # Use key path or provided key object
             key_types = {'EC': ECDSAKey, 'OPENSSH': Ed25519Key, 'RSA': RSAKey}
-            if isinstance(private_key, str):
+            if isinstance(private_key, (str, Path)):
                 key_file = Path(private_key).expanduser().absolute().as_posix()
                 try:
-                    with open(key_file, 'r', encoding='utf-8') as head:
-                        key_id = head.readline()[11:][:-18]
+                    with open(key_file, 'rb') as head:
+                        header = head.readline(64).decode('ascii', 'replace')
+                    key_id = header.rpartition(' PRIVATE KEY-----')[0][11:]
                     log.debug(f'Key ID: [{key_id}]')
                     key = key_types[key_id.strip()]
-                except KeyError as err:
-                    log.error(('Unable to identify key type from file provided'
-                              f': \n[{key_file}]'))
-                    raise err
-                except PasswordRequiredException as err:
-                    log.error(('No password provided for encrypted private '
-                               'key encrypted private key.'))
-                    raise err
-                except PermissionError as err:
-                    log.error(('File permission preventing user access to:\n'
-                              f'[{key_file}]'))
-                    raise err
-                except SSHException as err:
-                    log.error(('Path provided is an invalid key file, a '
-                               'directory or does not exist, please revise '
-                               'and provide a path to a valid private key.'))
-                    raise err
-                finally:
                     private_key = key.from_private_key_file(
                         key_file, password=private_key_pass)
+                except KeyError:
+                    log.error(('Unsupported key format, paramiko only reads '
+                               'EC, OPENSSH and RSA PEM keys. Re-encode with '
+                              f'ssh-keygen -p -f <keyfile>:\n[{key_file}]'))
+                    raise
+                except PermissionError:
+                    log.error(('File permission preventing user access to:\n'
+                              f'[{key_file}]'))
+                    raise
+                except OSError:
+                    log.error(('Path provided is a directory or does not '
+                               'exist, please revise and provide a path to a '
+                              f'readable private key:\n[{key_file}]'))
+                    raise
+                except PasswordRequiredException:
+                    log.error(('No password provided for encrypted private '
+                               f'key:\n[{key_file}]'))
+                    raise
+                except SSHException:
+                    log.error(('Path provided is an invalid or corrupt key '
+                               'file, please revise and provide a path to a '
+                               'valid private key.'))
+                    raise
             self._transport.auth_publickey(self._username, private_key)
         elif password is not None:
             self._transport.auth_password(self._username, password)
@@ -331,9 +340,9 @@ def _sftp_channel(self):
             meta.settimeout(self._timeout)
 
             if self._cache.cwd is None:
-                self._cache.cwd = drivedrop(channel.normalize('.'))
+                self._cache.cwd = drivepath(channel.normalize('.'))
 
-            channel.chdir(drivedrop(self._cache.cwd))
+            channel.chdir(drivepath(self._cache.cwd))
             log.info(f'Current Working Directory: [{self._cache.cwd}]')
 
             yield channel
@@ -556,6 +565,8 @@ def _get(self, remotefile, localpath=None, callback=None,
                  max_concurrent_prefetch_requests=None, prefetch=True,
                  preserve_mtime=False, resume=False):
 
+            remotefile = drivepath(remotefile)
+
             if localpath is None:
                 localpath = Path(remotefile).name
 
@@ -807,6 +818,8 @@ def getfo(self, remotefile, flo, callback=None,
         def _getfo(self, remotefile, flo, callback=None,
                    max_concurrent_prefetch_requests=None, prefetch=True):
 
+            remotefile = drivepath(remotefile)
+
             if callback is None:
                 callback = partial(_callback, remotefile, logger=logger)
 
@@ -876,7 +889,7 @@ def _put(self, localfile, remotepath=None, callback=None,
                                local_attributes.st_mtime)
 
             with self._sftp_channel() as channel:
-                remotepath = drivedrop(remotepath)
+                remotepath = drivepath(remotepath)
                 if resume:
                     remote = channel.stat(remotepath)
                     if S_ISREG(remote.st_mode):
@@ -1108,6 +1121,8 @@ def _putfo(self, flo, remotepath=None, file_size=None, callback=None,
 
             if remotepath is None:
                 remotepath = uuid4().hex
+            else:
+                remotepath = drivepath(remotepath)
 
             with self._sftp_channel() as channel:
                 attributes = channel.putfo(flo, remotepath=remotepath,
@@ -1192,8 +1207,8 @@ def chdir(self, remotepath):
         :raises: IOError, if path does not exist
         '''
         with self._sftp_channel() as channel:
-            channel.chdir(drivedrop(remotepath))
-            self._cache.cwd = drivedrop(channel.normalize('.'))
+            channel.chdir(drivepath(remotepath))
+            self._cache.cwd = drivepath(channel.normalize('.'))
 
     def chmod(self, remotepath, mode=700):
         '''Set the permission mode of a remotepath, where mode is an octal.
@@ -1206,7 +1221,7 @@ def chmod(self, remotepath, mode=700):
         :raises: IOError, if the file doesn't exist
         '''
         with self._sftp_channel() as channel:
-            channel.chmod(drivedrop(remotepath), mode=int(str(mode), 8))
+            channel.chmod(drivepath(remotepath), mode=int(str(mode), 8))
 
     def chown(self, remotepath, uid=None, gid=None):
         '''Set uid/gid on remotepath, you may specify either or both.
@@ -1220,7 +1235,7 @@ def chown(self, remotepath, uid=None, gid=None):
         :raises: IOError, if user lacks permission or if the file doesn't exist
         '''
         with self._sftp_channel() as channel:
-            remotepath = drivedrop(remotepath)
+            remotepath = drivepath(remotepath)
             if uid is None or gid is None:
                 if uid is None and gid is None:
                     return
@@ -1266,7 +1281,7 @@ def exists(self, remotepath):
         '''
         with self._sftp_channel() as channel:
             try:
-                channel.stat(remotepath)
+                channel.stat(drivepath(remotepath))
             except IOError as err:
                 if err.errno == 2:
                     return False
@@ -1281,7 +1296,7 @@ def getcwd(self):
         :returns: (str) Remote current working directory. None, if not set.
         '''
         with self._sftp_channel() as channel:
-            cwd = drivedrop(channel.getcwd())
+            cwd = drivepath(channel.getcwd())
 
         return cwd
 
@@ -1294,7 +1309,7 @@ def isdir(self, remotepath):
         '''
         with self._sftp_channel() as channel:
             try:
-                result = S_ISDIR(channel.stat(remotepath).st_mode)
+                result = S_ISDIR(channel.stat(drivepath(remotepath)).st_mode)
             except IOError:
                 # No such directory
                 result = False
@@ -1310,7 +1325,7 @@ def isfile(self, remotepath):
         '''
         with self._sftp_channel() as channel:
             try:
-                result = S_ISREG(channel.stat(remotepath).st_mode)
+                result = S_ISREG(channel.stat(drivepath(remotepath)).st_mode)
             except IOError:
                 # No such file
                 result = False
@@ -1326,7 +1341,7 @@ def lexists(self, remotepath):
         '''
         with self._sftp_channel() as channel:
             try:
-                channel.lstat(drivedrop(remotepath))
+                channel.lstat(drivepath(remotepath))
             except IOError:
                 return False
 
@@ -1341,7 +1356,7 @@ def listdir(self, remotepath='.'):
 
         '''
         with self._sftp_channel() as channel:
-            directory = sorted(channel.listdir(drivedrop(remotepath)))
+            directory = sorted(channel.listdir(drivepath(remotepath)))
 
         return directory
 
@@ -1359,7 +1374,7 @@ def listdir_attr(self, remotepath='.'):
         :returns: (list of SFTPAttributes) Sorted directory content as objects.
         '''
         with self._sftp_channel() as channel:
-            directory = sorted(channel.listdir_attr(drivedrop(remotepath)),
+            directory = sorted(channel.listdir_attr(drivepath(remotepath)),
                                key=lambda attribute: attribute.filename)
 
         return directory
@@ -1373,7 +1388,7 @@ def lstat(self, remotepath):
         :returns: (obj) SFTPAttributes object
         '''
         with self._sftp_channel() as channel:
-            lstat = channel.lstat(drivedrop(remotepath))
+            lstat = channel.lstat(drivepath(remotepath))
 
         return lstat
 
@@ -1387,7 +1402,7 @@ def mkdir(self, remotedir, mode=700):
         :returns: None
         '''
         with self._sftp_channel() as channel:
-            channel.mkdir(drivedrop(remotedir), mode=int(str(mode), 8))
+            channel.mkdir(drivepath(remotedir), mode=int(str(mode), 8))
 
     def mkdir_p(self, remotedir, mode=700):
         '''Create a directory and any missing parent locations as needed. Set
@@ -1402,7 +1417,7 @@ def mkdir_p(self, remotedir, mode=700):
         :raises: OSError
         '''
         try:
-            remotedir = drivedrop(remotedir)
+            remotedir = drivepath(remotedir)
             if self.isdir(remotedir):
                 return
             elif self.isfile(remotedir):
@@ -1431,9 +1446,9 @@ def normalize(self, remotepath):
         :raises: IOError, if remotepath can't be resolved
         '''
         with self._sftp_channel() as channel:
-            absolute = channel.normalize(drivedrop(remotepath))
+            absolute = channel.normalize(drivepath(remotepath))
 
-        return drivedrop(absolute)
+        return drivepath(absolute)
 
     def open(self, remotefile, bufsize=-1, mode='r'):
         '''Open a file on the remote server.
@@ -1447,7 +1462,7 @@ def open(self, remotefile, bufsize=-1, mode='r'):
         :raises: IOError, if the file could not be opened.
         '''
         with self._sftp_channel() as channel:
-            remotefile = drivedrop(remotefile)
+            remotefile = drivepath(remotefile)
             flo = channel.open(remotefile, bufsize=bufsize, mode=mode)
 
         return flo
@@ -1460,10 +1475,10 @@ def readlink(self, remotelink):
         :return: (str) Absolute path to target.
         '''
         with self._sftp_channel() as channel:
-            remotelink = drivedrop(remotelink)
+            remotelink = drivepath(remotelink)
             link_destination = channel.normalize(channel.readlink(remotelink))
 
-        return drivedrop(link_destination)
+        return drivepath(link_destination)
 
     def remotetree(self, container, remotedir, localdir, recurse=True):
         '''Recursively map remote directory tree to a dictionary container.
@@ -1510,7 +1525,7 @@ def remove(self, remotefile):
         :raises: IOError
         '''
         with self._sftp_channel() as channel:
-            channel.remove(drivedrop(remotefile))
+            channel.remove(drivepath(remotefile))
 
     def rename(self, remotepath, newpath, posix=True):
         '''Rename a path on the remote host.
@@ -1527,7 +1542,7 @@ def rename(self, remotepath, newpath, posix=True):
         '''
         with self._sftp_channel() as channel:
             renamer = channel.posix_rename if posix else channel.rename
-            renamer(drivedrop(remotepath), drivedrop(newpath))
+            renamer(drivepath(remotepath), drivepath(newpath))
 
     def rmdir(self, remotedir):
         '''Delete remote directory.
@@ -1537,7 +1552,7 @@ def rmdir(self, remotedir):
         :returns: None
         '''
         with self._sftp_channel() as channel:
-            channel.rmdir(drivedrop(remotedir))
+            channel.rmdir(drivepath(remotedir))
 
     def stat(self, remotepath):
         '''Return information about remote location.
@@ -1547,7 +1562,7 @@ def stat(self, remotepath):
         :returns: (obj) SFTPAttributes
         '''
         with self._sftp_channel() as channel:
-            stat = channel.stat(drivedrop(remotepath))
+            stat = channel.stat(drivepath(remotepath))
 
         return stat
 
@@ -1562,7 +1577,7 @@ def symlink(self, remote_src, remote_dest):
         :raises: any underlying error, IOError if remote_dest already exists
         '''
         with self._sftp_channel() as channel:
-            channel.symlink(remote_src, drivedrop(remote_dest))
+            channel.symlink(drivepath(remote_src), drivepath(remote_dest))
 
     def truncate(self, remotepath, size):
         '''Change the size of the file specified by path. Used to modify the
@@ -1577,7 +1592,7 @@ def truncate(self, remotepath, size):
         :raises: IOError, if file does not exist
         '''
         with self._sftp_channel() as channel:
-            remotepath = drivedrop(remotepath)
+            remotepath = drivepath(remotepath)
             channel.truncate(remotepath, size)
             size = channel.stat(remotepath).st_size
 
@@ -1620,7 +1635,7 @@ def pwd(self):
         :returns: (str) Current working directory.
         '''
         with self._sftp_channel() as channel:
-            self._cache.cwd = drivedrop(channel.normalize('.'))
+            self._cache.cwd = drivepath(channel.normalize('.'))
 
         return self._cache.cwd