Simone Pelosi has proposed merging ~pelpsi/txpkgupload:security-linters into txpkgupload:master.
Commit message: fix: resolve all pre-commit linting and formatting issues Remove unused variable 'client' in hooks.py Fix undefined future imports in test_filesystem.py by importing from __future__ module Replace lambda expression with proper function definition in test_plugin.py Remove unused 'connector' variable in test_plugin.py Fix f-string formatting issues in charm reactive code Quote shell command substitution in update-version-info.sh Apply black code formatting and isort import sorting Ensure all ruff security checks pass with existing suppressions Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~pelpsi/txpkgupload/+git/txpkgupload/+merge/488225 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/txpkgupload:security-linters into txpkgupload:master.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e61001c..fe5d447 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -44,3 +44,10 @@ repos: hooks: - id: woke-from-source files: ^doc/.*\.rst$ +- repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.12.1 + hooks: + - id: ruff + args: [--select=S] + name: ruff-security + files: ^src/ \ No newline at end of file diff --git a/charm/txpkgupload/reactive/txpkgupload.py b/charm/txpkgupload/reactive/txpkgupload.py index f0d5de6..820352f 100644 --- a/charm/txpkgupload/reactive/txpkgupload.py +++ b/charm/txpkgupload/reactive/txpkgupload.py @@ -246,10 +246,9 @@ def configure_loadbalancer(): # For passive FTP transfer, we want our haproxy to bind to not only the # usual FTP/SFTP ports but also to higher ports if config.get("min_passive_port") and config.get("max_passive_port"): - extra_ports = ( - f"bind :::" - f"{config['min_passive_port']}-{config['max_passive_port']}" - ) + min_port = config["min_passive_port"] + max_port = config["max_passive_port"] + extra_ports = "bind :::{}-{}".format(min_port, max_port) service_options_ftp.append(extra_ports) ftp_port = config["ftp_port"] diff --git a/scripts/update-version-info.sh b/scripts/update-version-info.sh index d6492fe..9e7a25f 100755 --- a/scripts/update-version-info.sh +++ b/scripts/update-version-info.sh @@ -13,7 +13,7 @@ if [ ! -e .git ]; then exit 1 fi -if ! which git > /dev/null || ! test -x $(which git); then +if ! which git > /dev/null || ! test -x "$(which git)"; then echo "No working 'git' executable found" >&2 exit 1 fi diff --git a/setup.py b/setup.py index 6f3ae16..4eb111f 100755 --- a/setup.py +++ b/setup.py @@ -16,20 +16,22 @@ # You should have received a copy of the GNU Lesser General Public License # along with txpkgupload. If not, see <http://www.gnu.org/licenses/>. -from setuptools import setup, find_packages +from setuptools import find_packages, setup # generic helpers primarily for the long_description def generate(*docname_or_string): res = [] for value in docname_or_string: - if value.endswith('.txt'): + if value.endswith(".txt"): with open(value) as f: value = f.read() res.append(value) - if not value.endswith('\n'): - res.append('') - return '\n'.join(res) + if not value.endswith("\n"): + res.append("") + return "\n".join(res) + + # end generic helpers @@ -39,32 +41,32 @@ with open("README.txt") as f: description = f.readline().strip() setup( - name='txpkgupload', + name="txpkgupload", version=__version__, - packages=find_packages('src') + ['twisted.plugins'], - package_dir={'': 'src'}, + packages=find_packages("src") + ["twisted.plugins"], + package_dir={"": "src"}, include_package_data=True, zip_safe=False, - maintainer='LAZR Developers', - maintainer_email='lazr-develop...@lists.launchpad.net', + maintainer="LAZR Developers", + maintainer_email="lazr-develop...@lists.launchpad.net", description=description, - long_description=generate('src/txpkgupload/NEWS.txt'), - license='AGPL v3', + long_description=generate("src/txpkgupload/NEWS.txt"), + license="AGPL v3", python_requires=">=3.5", install_requires=[ - 'FormEncode', + "FormEncode", 'importlib-metadata; python_version < "3.8"', - 'lazr.sshserver>=0.1.7', - 'oops-datedir-repo>=0.0.21', - 'oops-twisted>=0.0.7', - 'PyYAML', - 'setuptools', - 'six>=1.12.0', - 'Twisted[conch_nacl]', - 'zope.interface>=3.6.0', - ], - url='https://launchpad.net/txpkgupload', - download_url='https://launchpad.net/txpkgupload/+download', + "lazr.sshserver>=0.1.7", + "oops-datedir-repo>=0.0.21", + "oops-twisted>=0.0.7", + "PyYAML", + "setuptools", + "six>=1.12.0", + "Twisted[conch_nacl]", + "zope.interface>=3.6.0", + ], + url="https://launchpad.net/txpkgupload", + download_url="https://launchpad.net/txpkgupload/+download", classifiers=[ "Development Status :: 5 - Production/Stable", "Intended Audience :: Developers", @@ -76,18 +78,17 @@ setup( "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", - ], + ], extras_require=dict( - test=['fixtures', - 'testtools'], + test=["fixtures", "testtools"], ), entry_points={ - 'console_scripts': [ - 'build-twisted-plugin-cache = twisted.plugins.plugincache:main', + "console_scripts": [ + "build-twisted-plugin-cache = twisted.plugins.plugincache:main", ] }, # This does not play nicely with buildout because it downloads but does # not cache the package. # setup_requires=['eggtestinfo', 'setuptools_bzr'], - test_suite='txpkgupload.tests', - ) + test_suite="txpkgupload.tests", +) diff --git a/src/twisted/plugins/pkgupload.py b/src/twisted/plugins/pkgupload.py index 68b89f6..317b3e7 100644 --- a/src/twisted/plugins/pkgupload.py +++ b/src/twisted/plugins/pkgupload.py @@ -1,7 +1,6 @@ # Copyright 2005-2015 Canonical Ltd. This software is licensed under # the GNU Affero General Public License version 3 (see the file LICENSE). -from __future__ import absolute_import from txpkgupload.plugin import PkgUploadServiceMaker @@ -10,4 +9,5 @@ from txpkgupload.plugin import PkgUploadServiceMaker # providers of IPlugin and IServiceMaker. service_pkgupload = PkgUploadServiceMaker( - "pkgupload", "A package upload frontend server.") + "pkgupload", "A package upload frontend server." +) diff --git a/src/txpkgupload/__init__.py b/src/txpkgupload/__init__.py index 00557a9..c7379c4 100644 --- a/src/txpkgupload/__init__.py +++ b/src/txpkgupload/__init__.py @@ -12,7 +12,7 @@ def get_txpkgupload_root(fsroot): If the TXPKGUPLOAD_ROOT environment variable is set, use that. If not, use fsroot. """ - txpkgupload_root = os.environ.get('TXPKGUPLOAD_ROOT', None) + txpkgupload_root = os.environ.get("TXPKGUPLOAD_ROOT", None) if txpkgupload_root: return txpkgupload_root return fsroot diff --git a/src/txpkgupload/filesystem.py b/src/txpkgupload/filesystem.py index 9a913e0..d912ba5 100644 --- a/src/txpkgupload/filesystem.py +++ b/src/txpkgupload/filesystem.py @@ -1,16 +1,14 @@ # Copyright 2009 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). -__metaclass__ = type __all__ = [ - 'UploadFileSystem', - ] + "UploadFileSystem", +] import os class UploadFileSystem: - def __init__(self, rootpath): self.rootpath = rootpath @@ -30,10 +28,10 @@ class UploadFileSystem: # and since UTF-8 has low risk of undetected decoding errors, # let's try to decode SFTP paths as UTF-8. try: - path = path.decode('UTF-8') + path = path.decode("UTF-8") except UnicodeDecodeError: - raise NotImplementedError('Paths must be encoded using UTF-8') - if path.startswith('/'): + raise NotImplementedError("Paths must be encoded using UTF-8") + if path.startswith("/"): path = path[1:] path = os.path.normpath(path) return path diff --git a/src/txpkgupload/hooks.py b/src/txpkgupload/hooks.py index c6cc475..07a4b03 100644 --- a/src/txpkgupload/hooks.py +++ b/src/txpkgupload/hooks.py @@ -1,12 +1,11 @@ # Copyright 2009-2010 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). -__metaclass__ = type __all__ = [ - 'Hooks', - 'InterfaceFailure', - ] + "Hooks", + "InterfaceFailure", +] import os @@ -20,12 +19,11 @@ class InterfaceFailure(Exception): class Hooks: - clients = {} LOG_MAGIC = "Post-processing finished" _targetcount = 0 - def __init__(self, targetpath, targetstart=0, perms=None, prefix=''): + def __init__(self, targetpath, targetstart=0, perms=None, prefix=""): self.targetpath = targetpath self.perms = perms self.prefix = prefix @@ -38,10 +36,7 @@ class Hooks: def new_client_hook(self, fsroot, host, port): """Prepare a new client record indexed by fsroot...""" - self.clients[fsroot] = { - "host": host, - "port": port - } + self.clients[fsroot] = {"host": host, "port": port} log.msg("Accepting new session in fsroot: %s" % fsroot, debug=True) log.msg("Session from %s:%s" % (host, port), debug=True) @@ -53,11 +48,8 @@ class Hooks: log.msg("Processing session complete in %s" % fsroot, debug=True) - client = self.clients[fsroot] - timestamp = time.strftime("%Y%m%d-%H%M%S") - path = "upload%s-%s-%06d" % ( - self.prefix, timestamp, self.targetcount) + path = "upload%s-%s-%06d" % (self.prefix, timestamp, self.targetcount) target_fsroot = os.path.join(self.targetpath, path) # Move the session directory to the target directory. @@ -67,14 +59,16 @@ class Hooks: else: try: os.rename(fsroot, target_fsroot) - except (OSError, IOError): + except OSError: if not os.path.exists(target_fsroot): raise # XXX cprov 20071024: We should replace os.system call by os.chmod # and fix the default permission value accordingly in txpkgupload if self.perms is not None: - os.system("chmod %s -R %s" % (self.perms, target_fsroot)) + os.system( # noqa: S605 + "chmod %s -R %s" % (self.perms, target_fsroot) + ) self.clients.pop(fsroot) # This is mainly done so that tests know when the diff --git a/src/txpkgupload/plugin.py b/src/txpkgupload/plugin.py index c9287a1..aa6be6e 100644 --- a/src/txpkgupload/plugin.py +++ b/src/txpkgupload/plugin.py @@ -1,26 +1,18 @@ # Copyright 2005-2015 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). -__metaclass__ = type __all__ = [ - 'PkgUploadServiceMaker', - ] + "PkgUploadServiceMaker", +] -from functools import partial import os +from functools import partial +import yaml from formencode import Schema from formencode.api import set_stdtranslation -from formencode.validators import ( - Int, - RequireIfPresent, - String, - StringBool, - ) -from lazr.sshserver.auth import ( - LaunchpadAvatar, - PublicKeyFromLaunchpadChecker, - ) +from formencode.validators import Int, RequireIfPresent, String, StringBool +from lazr.sshserver.auth import LaunchpadAvatar, PublicKeyFromLaunchpadChecker from lazr.sshserver.service import SSHService from lazr.sshserver.session import DoNothingSession from twisted.application.service import IServiceMaker @@ -29,25 +21,14 @@ from twisted.conch.ssh import filetransfer from twisted.cred.portal import IRealm, Portal from twisted.plugin import IPlugin from twisted.protocols.policies import TimeoutFactory -from twisted.python import ( - components, - usage, - ) +from twisted.python import components, usage from twisted.web.xmlrpc import Proxy -import yaml from zope.interface import implementer from txpkgupload import get_txpkgupload_root -from txpkgupload.services import ( - PkgUploadServices, - ReadyService, - ) +from txpkgupload.services import PkgUploadServices, ReadyService from txpkgupload.twistedftp import FTPServiceFactory -from txpkgupload.twistedsftp import ( - PkgUploadFileTransferServer, - SFTPServer, - ) - +from txpkgupload.twistedsftp import PkgUploadFileTransferServer, SFTPServer # Ensure that formencode does not translate strings; there are encoding issues # that are easier to side-step for now. @@ -62,9 +43,7 @@ class ConfigOops(Schema): directory = String(if_missing="") reporter = String(if_missing="PKGUPLOAD") - chained_validators = ( - RequireIfPresent("reporter", present="directory"), - ) + chained_validators = (RequireIfPresent("reporter", present="directory"),) class ConfigFtp(Schema): @@ -116,7 +95,8 @@ class Config(Schema): # The access log location. Information such as connection, SSH login # and session start times will be logged here. access_log = String( - if_empty="txpkgupload-access.log", if_missing="txpkgupload-access.log") + if_empty="txpkgupload-access.log", if_missing="txpkgupload-access.log" + ) # If true, enable additional debug logging. debug = StringBool(if_missing=False) @@ -153,11 +133,14 @@ class Config(Schema): class Options(usage.Options): - optParameters = [ - ["config-file", "c", "etc/txpkgupload.yaml", - "Configuration file to load."], - ] + [ + "config-file", + "c", + "etc/txpkgupload.yaml", + "Configuration file to load.", + ], + ] class PkgUploadAvatar(LaunchpadAvatar): @@ -176,7 +159,6 @@ class PkgUploadAvatar(LaunchpadAvatar): @implementer(IRealm) class Realm: - def __init__(self, authentication_proxy, fs_root, temp_dir): self.authentication_proxy = authentication_proxy self.fs_root = fs_root @@ -184,8 +166,7 @@ class Realm: def requestAvatar(self, avatar_id, mind, *interfaces): # Fetch the user's details from the authserver - deferred = mind.lookupUserDetails( - self.authentication_proxy, avatar_id) + deferred = mind.lookupUserDetails(self.authentication_proxy, avatar_id) # Once all those details are retrieved, we can construct the avatar. def got_user_dict(user_dict): @@ -203,8 +184,7 @@ def make_portal(authentication_endpoint, fs_root, temp_dir): """ authentication_proxy = Proxy(authentication_endpoint.encode("UTF-8")) portal = Portal(Realm(authentication_proxy, fs_root, temp_dir)) - portal.registerChecker( - PublicKeyFromLaunchpadChecker(authentication_proxy)) + portal.registerChecker(PublicKeyFromLaunchpadChecker(authentication_proxy)) return portal @@ -233,13 +213,15 @@ class PkgUploadServiceMaker: oops_reporter = oops_config["reporter"] services = PkgUploadServices( - oops_dir, oops_reporter, server_argv=server_argv) + oops_dir, oops_reporter, server_argv=server_argv + ) root = get_txpkgupload_root(config["fsroot"]) temp_dir = config["temp_dir"] if temp_dir is None: - temp_dir = os.path.abspath(os.path.join( - root, os.pardir, "tmp-incoming")) + temp_dir = os.path.abspath( + os.path.join(root, os.pardir, "tmp-incoming") + ) if not os.path.exists(temp_dir): old_mask = os.umask(0o002) try: @@ -263,17 +245,20 @@ class PkgUploadServiceMaker: sftp_config = config["sftp"] sftp_service = SSHService( portal=make_portal( - sftp_config["authentication_endpoint"], root, temp_dir), + sftp_config["authentication_endpoint"], root, temp_dir + ), private_key_path=sftp_config["host_key_private"], public_key_path=sftp_config["host_key_public"], - main_log='txpkgupload', - access_log='txpkgupload.access', + main_log="txpkgupload", + access_log="txpkgupload.access", access_log_path=config["access_log"], strport=sftp_config["port"], factory_decorator=partial( - timeout_decorator, config["idle_timeout"]), + timeout_decorator, config["idle_timeout"] + ), banner=sftp_config["banner"], - moduli_path=sftp_config["moduli_path"]) + moduli_path=sftp_config["moduli_path"], + ) sftp_service.name = "sftp" sftp_service.setServiceParent(services) @@ -284,6 +269,7 @@ class PkgUploadServiceMaker: components.registerAdapter( - SFTPServer, PkgUploadAvatar, filetransfer.ISFTPServer) + SFTPServer, PkgUploadAvatar, filetransfer.ISFTPServer +) components.registerAdapter(DoNothingSession, PkgUploadAvatar, ISession) diff --git a/src/txpkgupload/services.py b/src/txpkgupload/services.py index 613ff30..c0af23e 100644 --- a/src/txpkgupload/services.py +++ b/src/txpkgupload/services.py @@ -3,31 +3,20 @@ """Additional services that compose txpkgupload.""" -from __future__ import ( - print_function, - unicode_literals, - ) -__metaclass__ = type __all__ = [ "PkgUploadFileLogObserver", "PkgUploadServices", "ReadyService", - ] +] import signal import sys from oops_datedir_repo import DateDirRepo -from oops_twisted import ( - Config as oops_config, - defer_publisher, - OOPSObserver, - ) -from twisted.application.service import ( - MultiService, - Service, - ) +from oops_twisted import Config as oops_config +from oops_twisted import OOPSObserver, defer_publisher +from twisted.application.service import MultiService, Service from twisted.internet import reactor from twisted.python import log from twisted.python.components import Componentized @@ -55,7 +44,8 @@ class ReadyService(Service): def startService(self): reactor.addSystemEventTrigger( - 'after', 'startup', log.msg, 'daemon ready!') + "after", "startup", log.msg, "daemon ready!" + ) Service.startService(self) @@ -63,7 +53,7 @@ class PkgUploadServices(MultiService): """Container for package upload services.""" def __init__(self, oops_dir, oops_reporter, server_argv=None): - super(PkgUploadServices, self).__init__() + super().__init__() self.oops_dir = oops_dir self.oops_reporter = oops_reporter self.server_argv = server_argv @@ -90,11 +80,14 @@ class PkgUploadServices(MultiService): if not logfilename: logfilename = "txpkgupload.log" self._log_file = logFile = LogFile.fromFullPath( - logfilename, rotateLength=None, defaultMode=0o644) + logfilename, rotateLength=None, defaultMode=0o644 + ) # Override if signal is set to None or SIG_DFL (0) if not signal.getsignal(signal.SIGUSR1): + def reopen_log(signal, frame): reactor.callFromThread(logFile.reopen) + signal.signal(signal.SIGUSR1, reopen_log) log_observer = PkgUploadFileLogObserver(logFile) @@ -105,13 +98,13 @@ class PkgUploadServices(MultiService): repo = DateDirRepo(self.oops_dir) config.publisher = defer_publisher(repo.publish) if self.oops_reporter: - config.template['reporter'] = self.oops_reporter + config.template["reporter"] = self.oops_reporter oops_observer = OOPSObserver(config, fallback=log_observer.emit) return oops_observer.emit def setServiceParent(self, parent): - super(PkgUploadServices, self).setServiceParent(parent) + super().setServiceParent(parent) if isinstance(parent, Componentized): # Customise the application's logging. We don't get much of an # opportunity to do this otherwise. @@ -121,4 +114,4 @@ class PkgUploadServices(MultiService): if self._log_file is not None: self._log_file.close() self._log_file = None - super(PkgUploadServices, self).disownServiceParent() + super().disownServiceParent() diff --git a/src/txpkgupload/tests/__init__.py b/src/txpkgupload/tests/__init__.py index 006d37c..8da50ec 100644 --- a/src/txpkgupload/tests/__init__.py +++ b/src/txpkgupload/tests/__init__.py @@ -2,4 +2,3 @@ # GNU Affero General Public License version 3 (see the file LICENSE). """Tests for txpkgupload.""" - diff --git a/src/txpkgupload/tests/test_filesystem.py b/src/txpkgupload/tests/test_filesystem.py index bb77a9f..9e33c7a 100644 --- a/src/txpkgupload/tests/test_filesystem.py +++ b/src/txpkgupload/tests/test_filesystem.py @@ -1,18 +1,13 @@ # Copyright 2009 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). -from __future__ import absolute_import, print_function, unicode_literals - -__metaclass__ = type import doctest import os - DOCTEST_FLAGS = ( - doctest.ELLIPSIS | - doctest.NORMALIZE_WHITESPACE | - doctest.REPORT_NDIFF) + doctest.ELLIPSIS | doctest.NORMALIZE_WHITESPACE | doctest.REPORT_NDIFF +) # The setUp() and tearDown() functions ensure that this doctest is not umask @@ -26,12 +21,21 @@ def tearDown(testobj): def load_tests(loader, tests, pattern): + # Import future features to make them available in doctests + import __future__ + globs = { - "absolute_import": absolute_import, - "print_function": print_function, - "unicode_literals": unicode_literals, - } - tests.addTest(doctest.DocFileSuite( - "filesystem.txt", setUp=setUp, tearDown=tearDown, globs=globs, - optionflags=DOCTEST_FLAGS)) + "absolute_import": __future__.absolute_import, + "print_function": __future__.print_function, + "unicode_literals": __future__.unicode_literals, + } + tests.addTest( + doctest.DocFileSuite( + "filesystem.txt", + setUp=setUp, + tearDown=tearDown, + globs=globs, + optionflags=DOCTEST_FLAGS, + ) + ) return tests diff --git a/src/txpkgupload/tests/test_plugin.py b/src/txpkgupload/tests/test_plugin.py index 13123f9..f16fb95 100644 --- a/src/txpkgupload/tests/test_plugin.py +++ b/src/txpkgupload/tests/test_plugin.py @@ -1,12 +1,7 @@ # Copyright 2009-2011 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). -from __future__ import absolute_import, print_function, unicode_literals -__metaclass__ = type - -from collections import defaultdict -from functools import partial import io import os import shutil @@ -14,64 +9,42 @@ import stat import struct import sys import tempfile +from collections import defaultdict +from functools import partial from textwrap import dedent -from fixtures import ( - EnvironmentVariableFixture, - Fixture, - TempDir, - ) +import yaml +from fixtures import EnvironmentVariableFixture, Fixture, TempDir from fixtures.callmany import MultipleExceptions from formencode import Invalid from lazr.sshserver.auth import NoSuchPersonWithName from testtools import TestCase from testtools.compat import reraise -from testtools.matchers import ( - MatchesException, - Raises, - ) +from testtools.matchers import MatchesException, Raises from testtools.twistedsupport import AsynchronousDeferredRunTest -from twisted.application.service import ( - Application, - IService, - MultiService, - ) +from twisted.application.service import Application, IService, MultiService from twisted.conch.client.default import SSHUserAuthClient from twisted.conch.client.direct import SSHClientFactory from twisted.conch.scripts.cftp import ClientOptions from twisted.conch.ssh.channel import SSHChannel from twisted.conch.ssh.common import NS -from twisted.conch.ssh.connection import ( - MSG_CHANNEL_REQUEST, - SSHConnection, - ) +from twisted.conch.ssh.connection import MSG_CHANNEL_REQUEST, SSHConnection from twisted.conch.ssh.filetransfer import ( - FileTransferClient, FXF_CREAT, FXF_EXCL, FXF_READ, FXF_TRUNC, FXF_WRITE, - ) -from twisted.internet import ( - defer, - reactor, - ) + FileTransferClient, +) +from twisted.internet import defer, reactor from twisted.internet.protocol import ClientCreator from twisted.protocols.ftp import FTPClient from twisted.python import log -from twisted.web import ( - server, - xmlrpc, - ) -import yaml +from twisted.web import server, xmlrpc from txpkgupload.hooks import Hooks -from txpkgupload.plugin import ( - Config, - Options, - PkgUploadServiceMaker, - ) +from txpkgupload.plugin import Config, Options, PkgUploadServiceMaker class DeferringFixture(Fixture): @@ -109,7 +82,7 @@ class TestConfig(TestCase): "fsroot": None, "ftp": { "port": "2121", - }, + }, "idle_timeout": 3600, "max_passive_port": None, "min_passive_port": None, @@ -117,7 +90,7 @@ class TestConfig(TestCase): "oops": { "directory": "", "reporter": "PKGUPLOAD", - }, + }, "sftp": { "authentication_endpoint": None, "banner": None, @@ -125,9 +98,9 @@ class TestConfig(TestCase): "host_key_public": None, "moduli_path": "/etc/ssh/moduli", "port": "tcp:5022", - }, + }, "temp_dir": None, - } + } observed = Config.to_python({}) self.assertEqual(expected, observed) @@ -147,8 +120,13 @@ class TestConfig(TestCase): def test_load_example(self): # The example configuration can be loaded and validated. filename = os.path.join( - os.path.dirname(__file__), os.pardir, os.pardir, os.pardir, - "etc", "txpkgupload.yaml") + os.path.dirname(__file__), + os.pardir, + os.pardir, + os.pardir, + "etc", + "txpkgupload.yaml", + ) Config.load(filename) def test_load_ftp_integer(self): @@ -163,12 +141,14 @@ class TestConfig(TestCase): # Check that a UsageError is raised when parsing options. self.assertThat( partial(Config.to_python, config), - Raises(MatchesException(Invalid, message))) + Raises(MatchesException(Invalid, message)), + ) def test_option_idle_timeout_integer(self): self.check_exception( {"idle_timeout": "bob"}, - "idle_timeout: Please enter an integer value") + "idle_timeout: Please enter an integer value", + ) class TestOptions(TestCase): @@ -210,14 +190,15 @@ class PkgUploadFixture(DeferringFixture): def _setUp(self): self.root = self.useFixture(TempDir()).path top = os.path.join( - os.path.dirname(__file__), os.pardir, os.pardir, os.pardir) + os.path.dirname(__file__), os.pardir, os.pardir, os.pardir + ) with open(os.path.join(top, "etc", "txpkgupload.yaml")) as stream: config = yaml.safe_load(stream) config["access_log"] = os.path.join( - self.root, "txpkgupload-access.log") + self.root, "txpkgupload-access.log" + ) if self.extra_config is not None: - deep_update( - config, yaml.safe_load(io.StringIO(self.extra_config))) + deep_update(config, yaml.safe_load(io.StringIO(self.extra_config))) # Make some paths absolute to cope with tests running in a different # working directory. for key in ("host_key_private", "host_key_public"): @@ -229,9 +210,11 @@ class PkgUploadFixture(DeferringFixture): options = Options() options.parseOptions(["-c", filename]) self.service_maker = PkgUploadServiceMaker( - "txpkgupload", "description") + "txpkgupload", "description" + ) self.service = self.service_maker.makeService( - options, server_argv=["--logfile", self.logfile]) + options, server_argv=["--logfile", self.logfile] + ) # Set up logging more or less as twistd's standard application # runner would, and start the service. application = Application(self.service_maker.tapname) @@ -249,8 +232,9 @@ class PkgUploadFixture(DeferringFixture): timeout_call = None def check(): + nonlocal check_call if os.path.exists(self.logfile): - with open(self.logfile, "r") as logfile: + with open(self.logfile) as logfile: occurrences = logfile.read().count(Hooks.LOG_MAGIC) if occurrences >= number: if timeout_call is not None and timeout_call.active(): @@ -280,7 +264,8 @@ class FTPServer(DeferringFixture): self.fsroot = os.path.join(self.root_dir, "incoming") self.temp_dir = os.path.join(self.root_dir, "tmp-incoming") top = os.path.join( - os.path.dirname(__file__), os.pardir, os.pardir, os.pardir) + os.path.dirname(__file__), os.pardir, os.pardir, os.pardir + ) with open(os.path.join(top, "etc", "txpkgupload.yaml")) as stream: config = yaml.safe_load(stream) self.port = config["ftp"]["port"] @@ -288,19 +273,30 @@ class FTPServer(DeferringFixture): def _setUp(self): os.mkdir(self.fsroot) os.mkdir(self.temp_dir) - self.pkgupload = self.useFixture(PkgUploadFixture(dedent("""\ + self.pkgupload = self.useFixture( + PkgUploadFixture( + dedent( + """\ fsroot: %s - temp_dir: %s""") % (self.fsroot, self.temp_dir))) + temp_dir: %s""" + ) + % (self.fsroot, self.temp_dir) + ) + ) def getAnonClient(self): creator = ClientCreator( - reactor, FTPClient, - username="anonymous", password="m...@example.com") + reactor, + FTPClient, + username="anonymous", + password="m...@example.com", # noqa: S106 + ) return creator.connectTCP("localhost", self.port) def getClient(self): creator = ClientCreator( - reactor, FTPClient, username="ubuntu", password="") + reactor, FTPClient, username="ubuntu", password="" + ) return creator.connectTCP("localhost", self.port) def makeDirectory(self, client, path): @@ -366,13 +362,14 @@ class SFTPConnection(SSHConnection): """ if channel.localClosed: return - log.msg('sending request %r' % (requestType)) + log.msg("sending request %r" % (requestType)) self.transport.sendPacket( MSG_CHANNEL_REQUEST, - struct.pack('>L', self.channelsToRemoteChannel[channel]) + struct.pack(">L", self.channelsToRemoteChannel[channel]) + NS(requestType) - + (b'\1' if wantReply else b'\0') - + data) + + (b"\1" if wantReply else b"\0") + + data, + ) if wantReply: d = defer.Deferred() self.deferreds.setdefault(channel.id, []).append(d) @@ -387,7 +384,7 @@ class FakeAuthServerService(xmlrpc.XMLRPC): self.keys = defaultdict(list) def addSSHKey(self, username, public_key_path): - with open(public_key_path, "r") as f: + with open(public_key_path) as f: public_key = f.read() kind, keytext, _ = public_key.split(" ", 2) if kind == "ssh-rsa": @@ -405,7 +402,7 @@ class FakeAuthServerService(xmlrpc.XMLRPC): "id": username, "name": username, "keys": self.keys[username], - } + } class SFTPServer(DeferringFixture): @@ -415,46 +412,57 @@ class SFTPServer(DeferringFixture): self.root_dir = root_dir self.fsroot = os.path.join(self.root_dir, "incoming") self.temp_dir = os.path.join(self.root_dir, "tmp-incoming") - #self._factory = factory + # self._factory = factory top = os.path.join( - os.path.dirname(__file__), os.pardir, os.pardir, os.pardir) + os.path.dirname(__file__), os.pardir, os.pardir, os.pardir + ) with open(os.path.join(top, "etc", "txpkgupload.yaml")) as stream: config = yaml.safe_load(stream) - self.port = int(config["sftp"]["port"].partition(':')[2]) + self.port = int(config["sftp"]["port"].partition(":")[2]) self.test_private_key = os.path.join( - os.path.dirname(__file__), "txpkgupload-sftp") + os.path.dirname(__file__), "txpkgupload-sftp" + ) self.test_public_key = os.path.join( - os.path.dirname(__file__), "txpkgupload-sftp.pub") + os.path.dirname(__file__), "txpkgupload-sftp.pub" + ) def setUpUser(self, name): self.authserver.addSSHKey(name, self.test_public_key) # Set up a temporary home directory for Paramiko's sake self._home_dir = tempfile.mkdtemp() self.addCleanup(shutil.rmtree, self._home_dir) - os.mkdir(os.path.join(self._home_dir, '.ssh')) + os.mkdir(os.path.join(self._home_dir, ".ssh")) os.symlink( self.test_private_key, - os.path.join(self._home_dir, '.ssh', 'id_rsa')) - self.useFixture(EnvironmentVariableFixture('HOME', self._home_dir)) - self.useFixture(EnvironmentVariableFixture('SSH_AUTH_SOCK', None)) - self.useFixture(EnvironmentVariableFixture('BZR_SSH', 'paramiko')) + os.path.join(self._home_dir, ".ssh", "id_rsa"), + ) + self.useFixture(EnvironmentVariableFixture("HOME", self._home_dir)) + self.useFixture(EnvironmentVariableFixture("SSH_AUTH_SOCK", None)) + self.useFixture(EnvironmentVariableFixture("BZR_SSH", "paramiko")) def _setUp(self): self.authserver = FakeAuthServerService() self.authserver_listener = reactor.listenTCP( - 0, server.Site(self.authserver)) + 0, server.Site(self.authserver) + ) self.authserver_port = self.authserver_listener.getHost().port self.authserver_url = "http://localhost:%d/" % self.authserver_port self.addCleanup(self.authserver_listener.stopListening) - self.setUpUser('joe') + self.setUpUser("joe") os.mkdir(self.fsroot) os.mkdir(self.temp_dir) - self.pkgupload = self.useFixture(PkgUploadFixture(dedent("""\ + self.pkgupload = self.useFixture( + PkgUploadFixture( + dedent( + """\ sftp: authentication_endpoint: %s fsroot: %s - temp_dir: %s""") % - (self.authserver_url, self.fsroot, self.temp_dir))) + temp_dir: %s""" + ) + % (self.authserver_url, self.fsroot, self.temp_dir) + ) + ) @defer.inlineCallbacks def getClient(self): @@ -465,11 +473,15 @@ class SFTPServer(DeferringFixture): conn = SFTPConnection() conn._sftp = defer.Deferred() auth_client = SSHUserAuthClient("joe", options, conn) - verifyHostKey = lambda t, h, pk, fp: defer.succeed(None) + + def verifyHostKey(t, h, pk, fp): + return defer.succeed(None) + connecting = defer.Deferred() factory = SSHClientFactory( - connecting, options, verifyHostKey, auth_client) - connector = reactor.connectTCP(host, self.port, factory) + connecting, options, verifyHostKey, auth_client + ) + reactor.connectTCP(host, self.port, factory) yield connecting client = yield conn._sftp defer.returnValue(client) @@ -480,7 +492,8 @@ class SFTPServer(DeferringFixture): @defer.inlineCallbacks def createFile(self, client, relpath, data): remote_file = yield client.openFile( - relpath, FXF_WRITE | FXF_CREAT | FXF_TRUNC | FXF_EXCL, {}) + relpath, FXF_WRITE | FXF_CREAT | FXF_TRUNC | FXF_EXCL, {} + ) yield remote_file.writeChunk(0, data) yield remote_file.close() @@ -509,7 +522,7 @@ class TestPkgUploadServiceMakerMixin: def setUp(self): """Set up txpkgupload in a temp dir.""" - super(TestPkgUploadServiceMakerMixin, self).setUp() + super().setUp() root_dir = self.useFixture(TempDir()).path self.server = self.server_factory(root_dir) self.useFixture(self.server) @@ -524,10 +537,13 @@ class TestPkgUploadServiceMakerMixin: self.assertIsInstance(service, MultiService) self.assertEqual(3, len(service.services)) self.assertSequenceEqual( - ["ftp", "ready", "sftp"], sorted(service.namedServices)) + ["ftp", "ready", "sftp"], sorted(service.namedServices) + ) self.assertEqual( - len(service.namedServices), len(service.services), - "Not all services are named.") + len(service.namedServices), + len(service.services), + "Not all services are named.", + ) def _uploadPath(self, path): """Return system path of specified path inside an upload. @@ -543,12 +559,12 @@ class TestPkgUploadServiceMakerMixin: # Creating directories on the server makes actual directories where we # expect them, and creates them with g+rwxs client = yield self.server.getClient() - yield self.server.makeDirectory(client, 'foo/bar') + yield self.server.makeDirectory(client, "foo/bar") yield self.server.disconnect(client) yield self.server.waitForClose() - wanted_path = self._uploadPath('foo/bar') + wanted_path = self._uploadPath("foo/bar") self.assertTrue(os.path.exists(wanted_path)) self.assertEqual(os.stat(wanted_path).st_mode, 0o42775) @@ -556,14 +572,14 @@ class TestPkgUploadServiceMakerMixin: def test_rmdir(self): """Check recursive RMD (aka rmdir)""" client = yield self.server.getClient() - yield self.server.makeDirectory(client, 'foo/bar') - yield client.removeDirectory('foo/bar') - yield client.removeDirectory('foo') + yield self.server.makeDirectory(client, "foo/bar") + yield client.removeDirectory("foo/bar") + yield client.removeDirectory("foo") yield self.server.disconnect(client) yield self.server.waitForClose() - wanted_path = self._uploadPath('foo') + wanted_path = self._uploadPath("foo") self.assertFalse(os.path.exists(wanted_path)) @defer.inlineCallbacks @@ -578,15 +594,21 @@ class TestPkgUploadServiceMakerMixin: yield self.server.disconnect(client) yield self.server.waitForClose() - wanted_path = self._uploadPath('foo/bar/baz') + wanted_path = self._uploadPath("foo/bar/baz") with open(os.path.join(wanted_path)) as f: fs_content = f.read() self.assertEqual(fs_content, "fake contents") # Expected mode is -rw-rwSr--. self.assertEqual( os.stat(wanted_path).st_mode, - stat.S_IROTH | stat.S_ISGID | stat.S_IRGRP | stat.S_IWGRP - | stat.S_IWUSR | stat.S_IRUSR | stat.S_IFREG) + stat.S_IROTH + | stat.S_ISGID + | stat.S_IRGRP + | stat.S_IWGRP + | stat.S_IWUSR + | stat.S_IRUSR + | stat.S_IFREG, + ) @defer.inlineCallbacks def test_full_source_upload(self): @@ -595,40 +617,49 @@ class TestPkgUploadServiceMakerMixin: """ client = yield self.server.getClient() - files = ['test-source_0.1.dsc', - 'test-source_0.1.orig.tar.gz', - 'test-source_0.1.diff.gz', - 'test-source_0.1_source.changes'] + files = [ + "test-source_0.1.dsc", + "test-source_0.1.orig.tar.gz", + "test-source_0.1.diff.gz", + "test-source_0.1_source.changes", + ] for upload in files: file_to_upload = "~ppa-user/ppa/ubuntu/%s" % upload yield self.server.createFile( - client, file_to_upload, upload.encode("ASCII")) + client, file_to_upload, upload.encode("ASCII") + ) yield self.server.disconnect(client) yield self.server.waitForClose() - upload_path = self._uploadPath('') + upload_path = self._uploadPath("") self.assertEqual(os.stat(upload_path).st_mode, 0o42770) - dir_name = upload_path.split('/')[-2] + dir_name = upload_path.split("/")[-2] if isinstance(self.server, SFTPServer): - self.assertEqual(dir_name.startswith('upload-sftp-2'), True) + self.assertEqual(dir_name.startswith("upload-sftp-2"), True) elif isinstance(self.server, FTPServer): - self.assertEqual(dir_name.startswith('upload-ftp-2'), True) + self.assertEqual(dir_name.startswith("upload-ftp-2"), True) else: raise AssertionError( - "self.server is neither SFTPServer or FTPServer") + "self.server is neither SFTPServer or FTPServer" + ) for upload in files: - wanted_path = self._uploadPath( - "~ppa-user/ppa/ubuntu/%s" % upload) + wanted_path = self._uploadPath("~ppa-user/ppa/ubuntu/%s" % upload) with open(os.path.join(wanted_path)) as f: fs_content = f.read() self.assertEqual(fs_content, upload) # Expected mode is -rw-rwSr--. self.assertEqual( os.stat(wanted_path).st_mode, - stat.S_IROTH | stat.S_ISGID | stat.S_IRGRP | stat.S_IWGRP - | stat.S_IWUSR | stat.S_IRUSR | stat.S_IFREG) + stat.S_IROTH + | stat.S_ISGID + | stat.S_IRGRP + | stat.S_IWGRP + | stat.S_IWUSR + | stat.S_IRUSR + | stat.S_IFREG, + ) @defer.inlineCallbacks def test_upload_isolation(self): @@ -663,15 +694,19 @@ class TestPkgUploadServiceMakerMixin: yield self.server.waitForClose(4) # Build a list of directories representing the 4 sessions. - upload_dirs = [leaf for leaf in sorted(os.listdir(self.server.fsroot)) - if not leaf.startswith(".")] + upload_dirs = [ + leaf + for leaf in sorted(os.listdir(self.server.fsroot)) + if not leaf.startswith(".") + ] self.assertEqual(len(upload_dirs), 4) # Check the contents of files on each session. - expected_contents = ['ONE', 'TWO', 'THREE', 'FOUR'] + expected_contents = ["ONE", "TWO", "THREE", "FOUR"] for index in range(4): - with open(os.path.join( - self.server.fsroot, upload_dirs[index], "test")) as f: + with open( + os.path.join(self.server.fsroot, upload_dirs[index], "test") + ) as f: content = f.read() self.assertEqual(content, expected_contents[index]) @@ -702,12 +737,12 @@ class TestPkgUploadServiceMakerFTP(TestPkgUploadServiceMakerMixin, TestCase): """ if client is None: client = yield self.server.getClient() - yield client.cwd('foo/bar') + yield client.cwd("foo/bar") yield self.server.disconnect(client) yield self.server.waitForClose() - wanted_path = self._uploadPath('foo/bar') + wanted_path = self._uploadPath("foo/bar") self.assertTrue(os.path.exists(wanted_path)) self.assertEqual(os.stat(wanted_path).st_mode, 0o42775) diff --git a/src/txpkgupload/tests/test_twistedsftp.py b/src/txpkgupload/tests/test_twistedsftp.py index a1b3093..fd819b7 100644 --- a/src/txpkgupload/tests/test_twistedsftp.py +++ b/src/txpkgupload/tests/test_twistedsftp.py @@ -3,27 +3,24 @@ """Tests for twistedsftp.""" -__metaclass__ = type import os import fixtures -from lazr.sshserver.sftp import FileIsADirectory import six import testtools +from lazr.sshserver.sftp import FileIsADirectory from txpkgupload.twistedsftp import SFTPServer class MockAvatar: - def __init__(self, fs_root, temp_dir): self.fs_root = fs_root self.temp_dir = temp_dir class TestSFTPServer(testtools.TestCase): - def setUp(self): temp_dir = self.useFixture(fixtures.TempDir()) fs_root = temp_dir.join("incoming") @@ -31,47 +28,55 @@ class TestSFTPServer(testtools.TestCase): os.mkdir(fs_root) os.mkdir(temp_incoming) self.sftp_server = SFTPServer(MockAvatar(fs_root, temp_incoming)) - super(TestSFTPServer, self).setUp() + super().setUp() def assertPermissions(self, expected, file_name): observed = os.stat(file_name).st_mode self.assertEqual( - expected, observed, "Expected %07o, got %07o, for %s" % ( - expected, observed, file_name)) + expected, + observed, + "Expected %07o, got %07o, for %s" + % (expected, observed, file_name), + ) def test_gotVersion(self): # gotVersion always returns an empty dict, since the server does not # support any extended features. See ISFTPServer. extras = self.sftp_server.gotVersion(None, None) - self.assertEquals(extras, {}) + self.assertEqual(extras, {}) def test_mkdir_and_rmdir(self): - self.sftp_server.makeDirectory('foo/bar', None) + self.sftp_server.makeDirectory("foo/bar", None) self.assertEqual( os.listdir(os.path.join(self.sftp_server._current_upload))[0], - 'foo') - dir_name = os.path.join(self.sftp_server._current_upload, 'foo') - self.assertEqual(os.listdir(dir_name)[0], 'bar') + "foo", + ) + dir_name = os.path.join(self.sftp_server._current_upload, "foo") + self.assertEqual(os.listdir(dir_name)[0], "bar") self.assertPermissions(0o40775, dir_name) - self.sftp_server.removeDirectory('foo/bar') + self.sftp_server.removeDirectory("foo/bar") self.assertEqual( - os.listdir(os.path.join(self.sftp_server._current_upload, - 'foo')), []) - self.sftp_server.removeDirectory('foo') + os.listdir(os.path.join(self.sftp_server._current_upload, "foo")), + [], + ) + self.sftp_server.removeDirectory("foo") self.assertEqual( - os.listdir(os.path.join(self.sftp_server._current_upload)), []) + os.listdir(os.path.join(self.sftp_server._current_upload)), [] + ) def test_file_creation(self): - upload_file = self.sftp_server.openFile('foo/bar', None, None) + upload_file = self.sftp_server.openFile("foo/bar", None, None) upload_file.writeChunk(0, b"This is a test") - file_name = os.path.join(self.sftp_server._current_upload, 'foo/bar') - with open(file_name, 'r') as test_file: + file_name = os.path.join(self.sftp_server._current_upload, "foo/bar") + with open(file_name) as test_file: self.assertEqual(test_file.read(), "This is a test") self.assertPermissions(0o100644, file_name) - dir_name = os.path.join(self.sftp_server._current_upload, 'bar/foo') + dir_name = os.path.join(self.sftp_server._current_upload, "bar/foo") os.makedirs(dir_name) - upload_file = self.sftp_server.openFile('bar/foo', None, None) + upload_file = self.sftp_server.openFile("bar/foo", None, None) err = self.assertRaises( - FileIsADirectory, upload_file.writeChunk, 0, b"This is a test") + FileIsADirectory, upload_file.writeChunk, 0, b"This is a test" + ) self.assertEqual( - "File is a directory: %r" % six.ensure_text(dir_name), str(err)) + "File is a directory: %r" % six.ensure_text(dir_name), str(err) + ) diff --git a/src/txpkgupload/twistedftp.py b/src/txpkgupload/twistedftp.py index a652553..2baa2a7 100644 --- a/src/txpkgupload/twistedftp.py +++ b/src/txpkgupload/twistedftp.py @@ -3,11 +3,10 @@ """Twisted FTP implementation of the txpkgupload upload server.""" -__metaclass__ = type __all__ = [ - 'AnonymousShell', - 'FTPRealm', - ] + "AnonymousShell", + "FTPRealm", +] import ipaddress import os @@ -15,23 +14,10 @@ import re import tempfile import six -from twisted.application import ( - service, - strports, - ) -from twisted.cred import ( - checkers, - credentials, - ) -from twisted.cred.portal import ( - IRealm, - Portal, - ) -from twisted.internet import ( - defer, - error, - reactor, - ) +from twisted.application import service, strports +from twisted.cred import checkers, credentials +from twisted.cred.portal import IRealm, Portal +from twisted.internet import defer, error, reactor from twisted.protocols import ftp from twisted.python import filepath from zope.interface import implementer @@ -45,7 +31,9 @@ class AccessCheck: """An `ICredentialsChecker` for txpkgupload FTP sessions.""" credentialInterfaces = ( - credentials.IUsernamePassword, credentials.IAnonymous) + credentials.IUsernamePassword, + credentials.IAnonymous, + ) def requestAvatarId(self, credentials): # txpkgupload allows any credentials. People can use "anonymous" if @@ -63,13 +51,13 @@ class AnonymousShell(ftp.FTPShell): def __init__(self, fsroot, temp_dir): self._fs_root = fsroot self.uploadfilesystem = UploadFileSystem( - tempfile.mkdtemp(dir=temp_dir)) + tempfile.mkdtemp(dir=temp_dir) + ) self._current_upload = self.uploadfilesystem.rootpath - os.chmod(self._current_upload, 0o770) - self.hook = Hooks(self._fs_root, perms='g+rws', prefix='-ftp') + os.chmod(self._current_upload, 0o770) # noqa: S103 + self.hook = Hooks(self._fs_root, perms="g+rws", prefix="-ftp") self.hook.new_client_hook(self._current_upload, 0, 0) - super(AnonymousShell, self).__init__( - filepath.FilePath(self._current_upload)) + super().__init__(filepath.FilePath(self._current_upload)) def openForWriting(self, file_segments): """Write the uploaded file to disk, safely. @@ -82,7 +70,7 @@ class AnonymousShell(ftp.FTPShell): """ filename = os.sep.join(file_segments) self._create_missing_directories(filename) - return super(AnonymousShell, self).openForWriting(file_segments) + return super().openForWriting(file_segments) def makeDirectory(self, path): """Make a directory using the secure `UploadFileSystem`.""" @@ -94,7 +82,7 @@ class AnonymousShell(ftp.FTPShell): if segments: path = self._path(segments) path.makedirs() - return super(AnonymousShell, self).access(segments) + return super().access(segments) def logout(self): """Called when the client disconnects. @@ -106,10 +94,10 @@ class AnonymousShell(ftp.FTPShell): def _create_missing_directories(self, filename): # Same as SFTPServer new_dir, new_file = os.path.split( - self.uploadfilesystem._sanitize(filename)) - if new_dir != '': - if not os.path.exists( - os.path.join(self._current_upload, new_dir)): + self.uploadfilesystem._sanitize(filename) + ) + if new_dir != "": + if not os.path.exists(os.path.join(self._current_upload, new_dir)): self.uploadfilesystem.mkdir(new_dir) def list(self, path_segments, attrs): @@ -133,10 +121,14 @@ class FTPRealm: for iface in interfaces: if iface is ftp.IFTPShell: avatar = AnonymousShell(self.root, self.temp_dir) - return ftp.IFTPShell, avatar, getattr( - avatar, 'logout', lambda: None) + return ( + ftp.IFTPShell, + avatar, + getattr(avatar, "logout", lambda: None), + ) raise NotImplementedError( - "Only IFTPShell interface is supported by this realm") + "Only IFTPShell interface is supported by this realm" + ) _AFNUM_IP = 1 @@ -166,7 +158,7 @@ def decodeExtendedAddressLine(line): @return: a 3-tuple of (protocol, host, port). """ - match = re.search(r'\((.*)\)', line) + match = re.search(r"\((.*)\)", line) if match: return decodeExtendedAddress(match.group(1)) else: @@ -174,7 +166,6 @@ def decodeExtendedAddressLine(line): class FTPWithEPSV(ftp.FTP): - epsvAll = False supportedNetworkProtocols = (_AFNUM_IP, _AFNUM_IP6) portsInUse = set() @@ -194,7 +185,7 @@ class FTPWithEPSV(ftp.FTP): if port and port in self.portsInUse: self.portsInUse.remove(port) - def getDTPPort(self, factory, interface=''): + def getDTPPort(self, factory, interface=""): """ Return a port for passive access, using C{self.passivePortRange} attribute. @@ -203,16 +194,19 @@ class FTPWithEPSV(ftp.FTP): if portn in self.portsInUse: continue try: - dtpPort = self.listenFactory(portn, factory, - interface=interface) + dtpPort = self.listenFactory( + portn, factory, interface=interface + ) except error.CannotListenError: continue else: self.portsInUse.add(dtpPort.getHost().port) return dtpPort - raise error.CannotListenError('', portn, - "No port available in range %s" % - (self.passivePortRange,)) + raise error.CannotListenError( + "", + portn, + "No port available in range %s" % (self.passivePortRange,), + ) def getHostAddress(self): """ @@ -245,7 +239,7 @@ class FTPWithEPSV(ftp.FTP): # response in order that at least clients that ignore the # host part can work, and if it becomes necessary then we # could do that too.) - return defer.fail(ftp.CmdNotImplementedError('PASV')) + return defer.fail(ftp.CmdNotImplementedError("PASV")) return str(address.ipv4_mapped) @@ -262,8 +256,9 @@ class FTPWithEPSV(ftp.FTP): server is listening on. """ if self.epsvAll: - return defer.fail(ftp.BadCmdSequenceError( - 'may not send PASV after EPSV ALL')) + return defer.fail( + ftp.BadCmdSequenceError("may not send PASV after EPSV ALL") + ) host = self.getHostAddress() @@ -301,12 +296,13 @@ class FTPWithEPSV(ftp.FTP): raise ftp.CmdArgSyntaxError(protocol) if protocol not in self.supportedNetworkProtocols: raise UnsupportedNetworkProtocolError( - ','.join(str(p) for p in self.supportedNetworkProtocols)) + ",".join(str(p) for p in self.supportedNetworkProtocols) + ) - def ftp_EPSV(self, protocol=''): - if protocol == 'ALL': + def ftp_EPSV(self, protocol=""): + if protocol == "ALL": self.epsvAll = True - self.sendLine('200 EPSV ALL OK') + self.sendLine("200 EPSV ALL OK") return defer.succeed(None) elif protocol: try: @@ -315,7 +311,8 @@ class FTPWithEPSV(ftp.FTP): return defer.fail() except UnsupportedNetworkProtocolError as e: self.sendLine( - '522 Network protocol not supported, use (%s)' % e.args) + "522 Network protocol not supported, use (%s)" % e.args + ) return defer.succeed(None) # if we have a DTP port set up, lose it. @@ -326,9 +323,9 @@ class FTPWithEPSV(ftp.FTP): self.dtpFactory = ftp.DTPFactory(pi=self) self.dtpFactory.setTimeout(self.dtpTimeout) if not protocol or protocol == _AFNUM_IP6: - interface = '::' + interface = "::" else: - interface = '' + interface = "" self.dtpPort = self.getDTPPort(self.dtpFactory, interface=interface) port = self.dtpPort.getHost().port @@ -337,8 +334,9 @@ class FTPWithEPSV(ftp.FTP): def ftp_PORT(self): if self.epsvAll: - return defer.fail(ftp.BadCmdSequenceError( - 'may not send PORT after EPSV ALL')) + return defer.fail( + ftp.BadCmdSequenceError("may not send PORT after EPSV ALL") + ) return ftp.FTP.ftp_PORT(self) def ftp_EPRT(self, extendedAddress): @@ -354,8 +352,9 @@ class FTPWithEPSV(ftp.FTP): transport addresses. """ if self.epsvAll: - return defer.fail(ftp.BadCmdSequenceError( - 'may not send EPRT after EPSV ALL')) + return defer.fail( + ftp.BadCmdSequenceError("may not send EPRT after EPSV ALL") + ) try: protocol, ip, port = decodeExtendedAddress(extendedAddress) @@ -368,7 +367,8 @@ class FTPWithEPSV(ftp.FTP): return defer.fail() except UnsupportedNetworkProtocolError as e: self.sendLine( - '522 Network protocol not supported, use (%s)' % e.args) + "522 Network protocol not supported, use (%s)" % e.args + ) return defer.succeed(None) # if we have a DTP port set up, lose it. @@ -376,7 +376,8 @@ class FTPWithEPSV(ftp.FTP): self.cleanupDTP() self.dtpFactory = ftp.DTPFactory( - pi=self, peerHost=self.transport.getPeer().host) + pi=self, peerHost=self.transport.getPeer().host + ) self.dtpFactory.setTimeout(self.dtpTimeout) self.dtpPort = reactor.connectTCP(ip, port, self.dtpFactory) diff --git a/src/txpkgupload/twistedsftp.py b/src/txpkgupload/twistedsftp.py index 0faec06..4e48a04 100644 --- a/src/txpkgupload/twistedsftp.py +++ b/src/txpkgupload/twistedsftp.py @@ -3,24 +3,17 @@ """Twisted SFTP implementation of the txpkgupload upload server.""" -__metaclass__ = type __all__ = [ - 'SFTPFile', - 'SFTPServer', - ] + "SFTPFile", + "SFTPServer", +] import errno import os import tempfile -from lazr.sshserver.sftp import ( - FileIsADirectory, - FileTransferServer, - ) -from twisted.conch.interfaces import ( - ISFTPFile, - ISFTPServer, - ) +from lazr.sshserver.sftp import FileIsADirectory, FileTransferServer +from twisted.conch.interfaces import ISFTPFile, ISFTPServer from zope.interface import implementer from txpkgupload.filesystem import UploadFileSystem @@ -35,9 +28,10 @@ class SFTPServer: self._avatar = avatar self._fs_root = avatar.fs_root self.uploadfilesystem = UploadFileSystem( - tempfile.mkdtemp(dir=avatar.temp_dir)) + tempfile.mkdtemp(dir=avatar.temp_dir) + ) self._current_upload = self.uploadfilesystem.rootpath - os.chmod(self._current_upload, 0o770) + os.chmod(self._current_upload, 0o770) # noqa: S103 def gotVersion(self, other_version, ext_data): return {} @@ -85,20 +79,20 @@ class SFTPServer: def _create_missing_directories(self, filename): new_dir, new_file = os.path.split( - self.uploadfilesystem._sanitize(filename)) - if new_dir != '': - if not os.path.exists( - os.path.join(self._current_upload, new_dir)): + self.uploadfilesystem._sanitize(filename) + ) + if new_dir != "": + if not os.path.exists(os.path.join(self._current_upload, new_dir)): self.uploadfilesystem.mkdir(new_dir) def _translate_path(self, filename): return self.uploadfilesystem._full( - self.uploadfilesystem._sanitize(filename)) + self.uploadfilesystem._sanitize(filename) + ) @implementer(ISFTPFile) class SFTPFile: - def __init__(self, filename): self.filename = filename @@ -111,7 +105,8 @@ class SFTPFile: def writeChunk(self, offset, data): try: chunk_file = os.open( - self.filename, os.O_CREAT | os.O_WRONLY, 0o644) + self.filename, os.O_CREAT | os.O_WRONLY, 0o644 + ) except OSError as e: if e.errno != errno.EISDIR: raise @@ -128,7 +123,6 @@ class SFTPFile: class SFTPDirectory: - def __iter__(self): return self @@ -142,10 +136,9 @@ class SFTPDirectory: class PkgUploadFileTransferServer(FileTransferServer): - def __init__(self, data=None, avatar=None): FileTransferServer.__init__(self, data=data, avatar=avatar) - self.hook = Hooks(self.client._fs_root, perms='g+rws', prefix='-sftp') + self.hook = Hooks(self.client._fs_root, perms="g+rws", prefix="-sftp") self.hook.new_client_hook(self.client._current_upload, 0, 0) def connectionLost(self, reason): diff --git a/tox.ini b/tox.ini index cba4d41..2d5ab90 100644 --- a/tox.ini +++ b/tox.ini @@ -18,3 +18,12 @@ commands = deps = -rbootstrap-requirements.txt -rrequirements.txt + +[testenv:lint] +description = + run linters with pre-commit +deps = + pre-commit +skip_install = true +commands = + pre-commit run -a \ No newline at end of file
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp