From c82843f85341ed7001a813291b729c034a21e881 Mon Sep 17 00:00:00 2001 From: Aleksei Stepanov Date: Thu, 29 Oct 2026 13:59:40 +0100 Subject: [PATCH 1/8] Format pyproject.toml with pyproject-fmt Formatter output only, so the next change shows only real edits to it: - Tables and keys are reordered and sorted by the tool. - Lists of values are sorted where order does not matter. - No value is added, removed or changed. --- pyproject.toml | 162 ++++++++++++++++++++++++------------------------- 1 file changed, 81 insertions(+), 81 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 2a44aa9..c24d317 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,18 +17,20 @@ license = "Apache-2.0" # Use SPDX classifier license-files = [ "LICENSE" ] requires-python = ">=3.11.0" dynamic = [ "classifiers", "dependencies", "version" ] -[[project.authors]] -name = "Aleksei Stepanov" -email = "penguinolog@gmail.com" [[project.maintainers]] name = "Aleksei Stepanov" email = "penguinolog@gmail.com" + [[project.maintainers]] name = "Antonio Esposito" email = "esposito.cloud@gmail.com" + [[project.maintainers]] name = "Dennis Dmitriev" email = "dis-xcom@gmail.com" +[[project.authors]] +name = "Aleksei Stepanov" +email = "penguinolog@gmail.com" [project.optional-dependencies] xml = [] lxml = [ "lxml>=4.6.2" ] @@ -101,18 +103,18 @@ tox = [ [tool.setuptools] zip-safe = false -[tool.setuptools.dynamic] -dependencies = { file = [ "requirements.txt" ] } -classifiers = { file = [ "classifiers.txt" ] } +[tool.setuptools.packages.find] +exclude = [ ".*", "bin", "doc*", "examples", "test*" ] +namespaces = false [tool.setuptools.package-data] exec_helpers = [ - "py.typed", "*.pyi", "*/*.pyi", + "py.typed", ] -[tool.setuptools.packages.find] -exclude = [ "doc*", "examples", "test*", "bin", ".*" ] -namespaces = false +[tool.setuptools.dynamic] +classifiers = { file = [ "classifiers.txt" ] } +dependencies = { file = [ "requirements.txt" ] } [tool.distutils.bdist_wheel] universal = 0 @@ -129,6 +131,13 @@ line-length = 120 output-format = "full" [tool.ruff.format] docstring-code-format = true +[tool.ruff.lint.isort] +force-single-line = true +known-third-party = [ "logwrap", "paramiko", "tenacity", "pyyaml", "ruamel.yaml", "psutil" ] +[tool.ruff.lint.pydocstyle] +convention = "pep257" +[tool.ruff.lint.pylint] +allow-dunder-method-names = [ "__pretty_repr__", "__pretty_str__", "__rich_repr__" ] [tool.ruff.lint] extend-select = [ "A", # flake8-builtins @@ -147,6 +156,7 @@ extend-select = [ "PERF", "PGH", # flake8-import-conventions, pygrep-hooks "PL", + "PTH", # flake8-use-pathlib "PYI", # flake8-pyi "Q", # quotes "RET", @@ -154,7 +164,6 @@ extend-select = [ "S", # flake8-bandit "SIM", "SLOT", # flake8-slots - "PTH", # flake8-use-pathlib "T10", "TC", # flake8-type-checking "TID", # flake8-tidy-imports @@ -173,30 +182,45 @@ extend-ignore = [ "PLR2004", "PLR6301", # "maybe staticmethod" "RET504", # Unnecessary variable assignment before return statement - "SIM108", # Use ternary operator, "S314", # use "defusedxml" which is stale - "TRY003", # long messages prepare outside, ... + "SIM108", # Use ternary operator, "TID252", # relative imports from parent modules + "TRY003", # long messages prepare outside, ... ] -[tool.ruff.lint.isort] -force-single-line = true -known-third-party = [ "logwrap", "paramiko", "tenacity", "pyyaml", "ruamel.yaml", "psutil" ] -[tool.ruff.lint.pydocstyle] -convention = "pep257" -[tool.ruff.lint.pylint] -allow-dunder-method-names = [ "__pretty_repr__", "__pretty_str__", "__rich_repr__" ] [tool.isort] line_length = 120 multi_line_output = 3 force_single_line = true -known_third_party = [ "logwrap", "paramiko", "tenacity", "pyyaml", "ruamel.yaml", "psutil" ] +known_third_party = [ "logwrap", "paramiko", "psutil", "pyyaml", "ruamel.yaml", "tenacity" ] [tool.pylint] +reports = false +disable = [ + "broad-except", + "file-ignored", + "invalid-name", + "locally-disabled", + "logging-format-interpolation", + "logging-fstring-interpolation", + "similarities", + "suppressed-message", + "too-few-public-methods", + "too-many-ancestors", + "too-many-arguments", + "too-many-branches", + "too-many-instance-attributes", + "too-many-lines", + "too-many-locals", + "too-many-positional-arguments", + "too-many-public-methods", + "too-many-return-statements", + "too-many-statements" +] +enable = "all" extension-pkg-whitelist = [ "lxml.etree" ] -ignore = [ "CVS", "_version.py" ] +ignore = [ "_version.py", "CVS" ] jobs = 0 -py-version = "3.11" load-plugins = [ "pylint.extensions.docparams", "pylint.extensions.docstyle", @@ -209,30 +233,8 @@ load-plugins = [ "pylint.extensions.empty_comment", "pylint.extensions.dunder", ] -enable = "all" -disable = [ - "locally-disabled", - "file-ignored", - "suppressed-message", - "similarities", - "too-many-ancestors", - "too-few-public-methods", - "too-many-public-methods", - "too-many-return-statements", - "too-many-branches", - "too-many-arguments", - "too-many-positional-arguments", - "too-many-locals", - "too-many-statements", - "too-many-instance-attributes", - "too-many-lines", - "broad-except", - "logging-fstring-interpolation", - "logging-format-interpolation", - "invalid-name" -] max-line-length = 120 -reports = false +py-version = "3.11" [tool.pylint.dunder] good-dunder-names = [ "__pretty_repr__", @@ -266,29 +268,30 @@ collapse_tables = [] skip_wrap_for_keys = [] [tool.mypy] -strict = true -warn_unused_configs = true warn_redundant_casts = true +strict = true show_error_context = true show_column_numbers = true -show_error_codes = true pretty = true +warn_unused_configs = true +show_error_codes = true [[tool.mypy.overrides]] -module = [ "psutil", "lxml.*" ] +module = [ "lxml.*", "psutil" ] ignore_missing_imports = true [tool.pytest] [tool.pytest.ini_options] minversion = "6.0" -addopts = "-vvv -s -p no:django -p no:ipdb" testpaths = [ "test" ] -mock_use_standalone_module = false +addopts = "-vvv -s -p no:django -p no:ipdb" junit_family = "xunit2" asyncio_default_fixture_loop_scope = "function" +mock_use_standalone_module = false [tool.coverage] -[tool.coverage.json] -pretty_print = true +[tool.coverage.run] +omit = [ "test/*" ] +branch = true [tool.coverage.report] exclude_lines = [ # Exclude methods marked as abstract @@ -321,37 +324,36 @@ exclude_lines = [ # Don't complain if tests don't hit defensive assertion code: "raise NotImplementedError", ] -[tool.coverage.run] -branch = true -omit = [ "test/*" ] +[tool.coverage.json] +pretty_print = true [tool.tox] requires = [ "tox>=4.53", ] env_list = [ - "ruff", - "pylint", + "doc8", + "docs", "mypy", "pep257", { prefix = "py3", start = 11, stop = 15 }, { prefix = "pypy3", start = 11, stop = 15 }, + "pylint", "readme", - "doc8", - "docs", + "ruff", ] skip_missing_interpreters = true [tool.tox.env] -py311.depends = [ "pylint", "mypy", "ruff", "ruff-format", "pep257", "bandit" ] -py312.depends = [ "pylint", "mypy", "ruff", "ruff-format", "pep257", "bandit" ] -py313.depends = [ "pylint", "mypy", "ruff", "ruff-format", "pep257", "bandit" ] -py314.depends = [ "pylint", "mypy", "ruff", "ruff-format", "pep257", "bandit" ] -py315.depends = [ "pylint", "mypy", "ruff", "ruff-format", "pep257", "bandit" ] -pypy311.depends = [ "pylint", "mypy", "ruff", "ruff-format", "pep257", "bandit" ] -pypy312.depends = [ "pylint", "mypy", "ruff", "ruff-format", "pep257", "bandit" ] -pypy313.depends = [ "pylint", "mypy", "ruff", "ruff-format", "pep257", "bandit" ] -pypy314.depends = [ "pylint", "mypy", "ruff", "ruff-format", "pep257", "bandit" ] -pypy315.depends = [ "pylint", "mypy", "ruff", "ruff-format", "pep257", "bandit" ] +py311.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] +py312.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] +py313.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] +py314.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] +py315.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] +pypy311.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] +pypy312.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] +pypy313.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] +pypy314.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] +pypy315.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] [tool.tox.env.bandit] dependency_groups = [ "bandit" ] ommands = [ [ "bandit", "-r", "exec_helpers" ] ] @@ -360,25 +362,24 @@ package = "skip" dependency_groups = [ "doc8" ] commands = [ [ "doc8", "README.rst", "doc/source" ] ] [tool.tox.env.docs] -depends = [ "doc8", "readme" ] dependency_groups = [ "docs" ] commands = [ [ "sphinx-build", "doc/source/", "doc/build" ] ] +depends = [ "doc8", "readme" ] [tool.tox.env.install] dependency_groups = [] commands = [ [ "pip", "install", "./", "-vvv", "-U" ] ] [tool.tox.env.mypy] -depends = [ "ruff", "ruff-format", "pep257", "bandit" ] +deps = [ "-r {toxinidir}/CI_REQUIREMENTS.txt" ] dependency_groups = [ "mypy" ] -deps = ["-r {toxinidir}/CI_REQUIREMENTS.txt"] commands = [ [ "mypy", "--strict", "--install-types", "--non-interactive", "--show-error-codes", "-p", "exec_helpers" ], ] - +depends = [ "bandit", "pep257", "ruff", "ruff-format" ] [tool.tox.env.pylint] -depends = [ "ruff", "ruff-format", "pep257", "bandit" ] +deps = [ "-r {toxinidir}/CI_REQUIREMENTS.txt" ] dependency_groups = [ "pylint" ] -deps = ["-r {toxinidir}/CI_REQUIREMENTS.txt"] commands = [ [ "pylint", "exec_helpers" ] ] +depends = [ "bandit", "pep257", "ruff", "ruff-format" ] [tool.tox.env.readme] package = "skip" dependency_groups = [ "readme" ] @@ -386,12 +387,11 @@ commands = [ [ "python", "-m", "build", "-s" ], [ "twine", "check", "{tox_root}{/}dist{/}*" ], ] - [tool.tox.env.ruff] package = "skip" -depends = [ "ruff-format" ] dependency_groups = [ "ruff" ] commands = [ [ "ruff", "check", "." ] ] +depends = [ "ruff-format" ] [tool.tox.env.ruff-format] package = "skip" dependency_groups = [ "ruff" ] @@ -399,8 +399,9 @@ commands = [ [ "ruff", "format", "--check", "." ] ] [tool.tox.env.venv] commands = [ [ "{posargs}" ] ] [tool.tox.env_run_base] -recreate = true package = "editable" +dependency_groups = [ "pytest" ] +recreate = true pass_env = [ "http_proxy", "HTTP_PROXY", @@ -410,7 +411,6 @@ pass_env = [ "NO_PROXY", ] set_env = { PYTHONDONTWRITEBYTECODE = "1" } -dependency_groups = [ "pytest" ] commands = [ [ "pip", "freeze" ], [ "py.test" ], From 91e2db912418ffe3e8b72b2701a0232abfc9cc39 Mon Sep 17 00:00:00 2001 From: Aleksei Stepanov Date: Thu, 29 Oct 2026 14:02:05 +0100 Subject: [PATCH 2/8] Update static checkers and enforce the lint baseline Raise the checker floors to the current releases and bring the project up to the baseline rule set. ruff and pyproject-fmt revs in pre-commit match the dependency groups; CI installs pylint and mypy from the same groups. - Enable the D (pydocstyle) and PIE rules; fix the docstrings they report. Tests ignore only the missing-docstring rules D1. - Fix ICN001 (datetime imported as dt), FURB107 and BLE001 (narrow noqa with a reason), pylint arguments-differ against Callable and a useless suppression. - Suppress S602 on Subprocess.Popen: running a shell command line is the purpose of the class (approved by the maintainer). - Add types-paramiko, types-psutil and types-pyyaml to the mypy group instead of --install-types; drop the psutil ignore_missing_imports. - Enable mypy ignore-without-code; strict comes from [tool.mypy], not flags. - Fix the "ommands" typo in the bandit tox env, so bandit now actually runs. - Remove the unused [tool.isort], the wheel build requirement, the dangling pep257 tox env, and options that restate a default. | what | from | to | | --- | --- | --- | | ruff | >=0.15.13 | >=0.17.3 | | mypy | >=2.1 | >=2.4.1 | | pylint | >=4.0.5 | >=4.1.3 | | pyproject-fmt | >=2.21.2 | >=2.30.3 | | prek | >=0.4.1 | >=0.5.7 | | ruff-pre-commit rev | v0.15.13 | v0.17.3 | | pyproject-fmt rev | v2.21.2 | v2.30.3 | | CI pylint | >=3.0.0 | dependency group | --- .github/workflows/pythonpackage.yml | 4 +- .pre-commit-config.yaml | 4 +- doc/source/conf.py | 2 + exec_helpers/__init__.py | 5 +- exec_helpers/_ssh_base.py | 17 +++--- exec_helpers/_ssh_helpers.py | 2 +- exec_helpers/api.py | 13 ++--- exec_helpers/async_api/api.py | 10 ++-- exec_helpers/async_api/exec_result.py | 2 +- exec_helpers/async_api/subprocess.py | 4 +- exec_helpers/exec_result.py | 24 ++++---- exec_helpers/ssh_auth.py | 10 ++-- exec_helpers/subprocess.py | 7 ++- pyproject.toml | 60 +++++++++----------- test/async_api/test_subprocess.py | 8 +-- test/async_api/test_subprocess_special.py | 6 +- test/pyproject.toml | 1 + test/test_exec_result.py | 4 +- test/test_ssh_client_execute.py | 10 ++-- test/test_ssh_client_execute_special.py | 6 +- test/test_ssh_client_execute_through_host.py | 2 +- test/test_ssh_client_init_basic.py | 4 +- test/test_sshauth.py | 4 +- test/test_subprocess.py | 6 +- test/test_subprocess_special.py | 6 +- 25 files changed, 108 insertions(+), 113 deletions(-) diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index d00e70a..41a14f0 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -63,8 +63,8 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip + pip install --upgrade --group pylint pip install --upgrade -r CI_REQUIREMENTS.txt - pip install --upgrade "pylint >= 3.0.0" - name: Install develop run: | pip install -e . @@ -93,7 +93,7 @@ jobs: pip install -e . - name: Lint with MyPy run: | - mypy --strict --install-types --non-interactive exec_helpers + mypy -p exec_helpers Test: permissions: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 21398f0..82f9c0a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -10,7 +10,7 @@ repos: - repo: https://github.com/astral-sh/ruff-pre-commit # Ruff version. - rev: v0.15.13 + rev: v0.17.3 hooks: # Run the linter. - id: ruff-check @@ -29,6 +29,6 @@ repos: - repo: https://github.com/tox-dev/pyproject-fmt # Use the sha / tag you want to point at # or use `pre-commit autoupdate` to get the latest version - rev: "v2.21.2" + rev: "v2.30.3" hooks: - id: pyproject-fmt diff --git a/doc/source/conf.py b/doc/source/conf.py index faf1d7a..1b465c9 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -1,3 +1,5 @@ +"""Sphinx documentation build configuration.""" + # exec-helpers documentation build configuration file, created by # sphinx-quickstart on Sun Oct 30 13:40:52 2016. # diff --git a/exec_helpers/__init__.py b/exec_helpers/__init__.py index f553f85..d1c1a13 100644 --- a/exec_helpers/__init__.py +++ b/exec_helpers/__init__.py @@ -16,14 +16,13 @@ from __future__ import annotations +import contextlib import importlib import typing import warnings -try: # noqa: SIM105,S314,RUF100 +with contextlib.suppress(ImportError): from ._version import version as __version__ # noqa: F401 -except ImportError: - pass if typing.TYPE_CHECKING: from collections.abc import Sequence diff --git a/exec_helpers/_ssh_base.py b/exec_helpers/_ssh_base.py index 88474b0..87e8125 100644 --- a/exec_helpers/_ssh_base.py +++ b/exec_helpers/_ssh_base.py @@ -21,7 +21,7 @@ import concurrent.futures import contextlib import copy -import datetime +import datetime as dt import getpass import logging import pathlib @@ -308,7 +308,7 @@ def __enter__(self) -> SshExecuteAsyncResult: stderr = None _stdin: paramiko.channel.ChannelFile with chan.makefile("wb") as _stdin: - started = datetime.datetime.now(tz=datetime.UTC) + started = dt.datetime.now(tz=dt.UTC) if self.__sudo_mode: chan.exec_command(self.command) # nosec # Sanitize on caller side if not stdout.channel.closed: @@ -520,7 +520,7 @@ def __init__( allow_ssh_agent: bool = True, missing_host_key_policy: type[paramiko.MissingHostKeyPolicy] = paramiko.WarningPolicy, ) -> None: - """Main SSH Client helper.""" + """Initialize the SSH client helper.""" self.__sudo_mode = False self.__keepalive_period: int = int(keepalive) self.__keepalive_mode = False @@ -756,7 +756,7 @@ def _ssh(self) -> paramiko.SSHClient: reraise=True, ) def __connect(self) -> None: - """Main method for connection open.""" + """Open the connection.""" with self.lock: if self.__sock is not None: self.__ssh = paramiko.SSHClient() @@ -785,7 +785,6 @@ def __get_client(self) -> paramiko.SSHClient: :raises RuntimeError: Unexpected state. :raises ConnectionError: Cannot get SSH transport. """ - last_ssh_client: paramiko.SSHClient = paramiko.SSHClient() last_ssh_client.load_system_host_keys() last_ssh_client.set_missing_host_key_policy(self._missing_host_key_policy) @@ -877,7 +876,7 @@ def __del__(self) -> None: """ try: self.__ssh.close() - except BaseException as e: # pragma: no cover # NOSONAR + except BaseException as e: # noqa: BLE001 # pragma: no cover # NOSONAR with contextlib.suppress(AttributeError): self.logger.debug(f"Exception in {self!s} destructor call: {e}") self.__sftp = None @@ -908,7 +907,7 @@ def sudo_mode(self) -> bool: @sudo_mode.setter def sudo_mode(self, mode: bool) -> None: - """Persistent sudo mode change for connection object. + """Change the persistent sudo mode of the connection object. :param mode: Sudo status: enabled | disabled. :type mode: bool @@ -1061,7 +1060,7 @@ def poll_streams() -> None: result.read_stderr(src=async_result.stderr, log=self.logger if log_stderr else None, verbose=verbose) def poll_pipes() -> None: - """Polling task for FIFO buffers.""" + """Poll FIFO buffers.""" while not async_result.interface.status_event.is_set(): time.sleep(0.1) if async_result.stdout or async_result.stderr: @@ -1787,7 +1786,7 @@ def get_result(remote: SSHClientBase) -> exec_result.ExecResult: results[remote.hostname, remote.port] = result if result.exit_code not in prep_expected: errors[remote.hostname, remote.port] = result - except Exception as e: + except Exception as e: # noqa: BLE001 # stored and reported to the caller raised_exceptions[remote.hostname, remote.port] = e if raised_exceptions: # always raise diff --git a/exec_helpers/_ssh_helpers.py b/exec_helpers/_ssh_helpers.py index 5f0f981..8038be8 100644 --- a/exec_helpers/_ssh_helpers.py +++ b/exec_helpers/_ssh_helpers.py @@ -38,7 +38,7 @@ def _parse_ssh_config_file(file_path: pathlib.Path) -> paramiko.SSHConfig | None config = paramiko.SSHConfig() with file_path.open() as f_obj: config.parse(f_obj) - except Exception: + except Exception: # noqa: BLE001 # any parse failure means no usable config config = None return config diff --git a/exec_helpers/api.py b/exec_helpers/api.py index 2b8f4d9..dcabae7 100644 --- a/exec_helpers/api.py +++ b/exec_helpers/api.py @@ -40,7 +40,7 @@ from . import _helpers if typing.TYPE_CHECKING: - import datetime + import datetime as dt from collections.abc import Sequence from types import TracebackType from typing import Self @@ -74,7 +74,7 @@ class ExecuteAsyncResult(typing.NamedTuple): stdin: typing.Any | None stderr: typing.Any | None stdout: typing.Any | None - started: datetime.datetime + started: dt.datetime class ExecuteContext(contextlib.AbstractContextManager[ExecuteAsyncResult], abc.ABC): @@ -310,7 +310,7 @@ def _chroot_path(self) -> None: @property def _chroot_exe(self) -> str | None: - """Exe for chroot + """Return the exe for chroot. :rtype: str | None .. versionadded:: 8.1.0 @@ -319,7 +319,7 @@ def _chroot_exe(self) -> str | None: @_chroot_exe.setter def _chroot_exe(self, new_state: str | None) -> None: - """Executable for chroot if set. + """Set the executable for chroot. :param new_state: New exe. :type new_state: str | None @@ -394,7 +394,6 @@ def _mask_command(self, cmd: str, log_mask_re: LogMaskReT = None) -> str: .. versionadded:: 1.2.0 """ - return _helpers.mask_command(cmd.rstrip(), self.log_mask_re, log_mask_re) def _prepare_command(self, cmd: str, chroot_path: str | None = None, chroot_exe: str | None = None) -> str: @@ -588,7 +587,7 @@ def execute( self.logger.log(level=log_level, msg=f"Command {result.cmd!r} exit code: {result.exit_code!s}") return result - def __call__( + def __call__( # pylint: disable=arguments-differ # Callable[..., X] declares no fixed signature self, command: CommandT, verbose: bool = False, @@ -823,7 +822,7 @@ def _handle_stderr( expected: ExpectedExitCodesT, exception_class: CalledProcessErrorSubClassT, ) -> exec_result.ExecResult: - """Internal check_stderr logic (synchronous). + """Check STDERR of the result (synchronous). :param result: Execution result for validation. :type result: exec_result.ExecResult diff --git a/exec_helpers/async_api/api.py b/exec_helpers/async_api/api.py index 5a8d424..7743912 100644 --- a/exec_helpers/async_api/api.py +++ b/exec_helpers/async_api/api.py @@ -272,7 +272,7 @@ def _chroot_path(self) -> None: @property def _chroot_exe(self) -> str | None: - """Exe for chroot + """Return the exe for chroot. :rtype: str | None .. versionadded:: 8.1.0 @@ -281,7 +281,7 @@ def _chroot_exe(self) -> str | None: @_chroot_exe.setter def _chroot_exe(self, new_state: str | None) -> None: - """Executable for chroot if set. + """Set the executable for chroot. :param new_state: New exe. :type new_state: str | None @@ -350,7 +350,6 @@ def _mask_command(self, cmd: str, log_mask_re: LogMaskReT = None) -> str: .. versionadded:: 1.2.0 """ - return _helpers.mask_command(cmd.rstrip(), self.log_mask_re, log_mask_re) def _prepare_command(self, cmd: str, chroot_path: str | None = None, chroot_exe: str | None = None) -> str: @@ -541,7 +540,8 @@ async def execute( self.logger.log(level=log_level, msg=f"Command {result.cmd!r} exit code: {result.exit_code!s}") return result - async def __call__( # pylint: disable=invalid-overridden-method + # Callable[..., X] declares no fixed signature, so arguments-differ is a false positive. + async def __call__( # pylint: disable=invalid-overridden-method,arguments-differ self, command: CommandT, verbose: bool = False, @@ -772,7 +772,7 @@ def _handle_stderr( expected: ExpectedExitCodesT, exception_class: CalledProcessErrorSubClassT, ) -> exec_result.ExecResult: - """Internal check_stderr logic (synchronous). + """Check STDERR of the result (synchronous). :param result: Execution result for validation. :type result: exec_result.ExecResult diff --git a/exec_helpers/async_api/exec_result.py b/exec_helpers/async_api/exec_result.py index a982053..9a07a3c 100644 --- a/exec_helpers/async_api/exec_result.py +++ b/exec_helpers/async_api/exec_result.py @@ -26,7 +26,7 @@ if typing.TYPE_CHECKING: from collections.abc import AsyncIterable -from exec_helpers import exec_result # pylint: disable=wrong-import-position +from exec_helpers import exec_result __all__ = ("ExecResult",) diff --git a/exec_helpers/async_api/subprocess.py b/exec_helpers/async_api/subprocess.py index 2aa8f94..6b2ec78 100644 --- a/exec_helpers/async_api/subprocess.py +++ b/exec_helpers/async_api/subprocess.py @@ -22,7 +22,7 @@ import asyncio import contextlib import copy -import datetime +import datetime as dt import errno import logging import os @@ -183,7 +183,7 @@ async def __aenter__(self) -> SubprocessExecuteAsyncResult: The Command is executed only in the context manager to be sure that everything will be cleaned up properly. """ - started = datetime.datetime.now(tz=datetime.UTC) + started = dt.datetime.now(tz=dt.UTC) self.__process = await asyncio.create_subprocess_shell( cmd=self.command, diff --git a/exec_helpers/exec_result.py b/exec_helpers/exec_result.py index 832cf4f..b0b8fbd 100644 --- a/exec_helpers/exec_result.py +++ b/exec_helpers/exec_result.py @@ -19,7 +19,7 @@ from __future__ import annotations import contextlib -import datetime +import datetime as dt import functools import json import logging @@ -68,13 +68,13 @@ def _handle_deserialize( fmt: str, ) -> Callable[[Callable[[ExecResult], _T]], Callable[[ExecResult], _T]]: - """Decorator fabric for decoder getters. + """Create a decorator for decoder getters. :return: real decorator """ def decorator(method: Callable[[ExecResult], _T]) -> Callable[[ExecResult], _T]: - """Decorator for decoder getter. + """Decorate a decoder getter. :return: Wrapped to try/except getter. """ @@ -167,7 +167,7 @@ def __getitem__( raise TypeError(f"Unexpected key type: {item!r}") def __len__(self) -> int: # pragma: no cover - """Data len. + """Return the data length. :return: Strings count. :rtype: int @@ -218,7 +218,7 @@ def __init__( stderr: Iterable[bytes] | None = None, exit_code: ExitCodeT = proc_enums.INVALID, *, - started: datetime.datetime | None = None, + started: dt.datetime | None = None, ) -> None: """Command execution result. @@ -257,10 +257,10 @@ def __init__( self._stderr = () self.__exit_code: ExitCodeT = proc_enums.INVALID - self.__timestamp: datetime.datetime | None = None + self.__timestamp: dt.datetime | None = None self.exit_code = exit_code - self.__started: datetime.datetime | None = started + self.__started: dt.datetime | None = started # By default is none: self._stdout_str: str | None = None @@ -291,7 +291,7 @@ def stderr_lock(self) -> threading.RLock: return self.__stderr_lock @property - def timestamp(self) -> datetime.datetime | None: + def timestamp(self) -> dt.datetime | None: """Timestamp. :return: Exit code timestamp. @@ -307,7 +307,7 @@ def set_timestamp(self) -> None: .. versionadded:: 4.0.0 """ if self.timestamp is None: - self.__timestamp = datetime.datetime.now(tz=datetime.UTC) + self.__timestamp = dt.datetime.now(tz=dt.UTC) @classmethod def _get_brief(cls, data: tuple[bytes, ...]) -> str: @@ -558,7 +558,7 @@ def exit_code(self, new_val: ExitCodeT) -> None: with self.stdout_lock, self.stderr_lock: self.__exit_code = proc_enums.exit_code_to_enum(new_val) if self.__exit_code != proc_enums.INVALID: - self.__timestamp = datetime.datetime.now(tz=datetime.UTC) + self.__timestamp = dt.datetime.now(tz=dt.UTC) @property def ok(self) -> bool: @@ -627,7 +627,7 @@ def raise_for_status( raise exception_class(self, expected_codes) @property - def started(self) -> datetime.datetime | None: + def started(self) -> dt.datetime | None: """Timestamp of command start. :return: Timestamp from command start, if applicable. @@ -726,7 +726,7 @@ def __dir__(self) -> list[str]: return content def __getitem__(self, item: str) -> typing.Any: - """Dict like get data. + """Get data like from a dict. :param item: Key. :type item: str diff --git a/exec_helpers/ssh_auth.py b/exec_helpers/ssh_auth.py index e562f8b..41b1137 100644 --- a/exec_helpers/ssh_auth.py +++ b/exec_helpers/ssh_auth.py @@ -115,7 +115,7 @@ def username(self) -> str | None: @staticmethod def __get_public_key(key: paramiko.PKey | None) -> str | None: - """Internal method for get public key from private. + """Get the public key from the private one. :param key: SSH private key. :type key: paramiko.PKey @@ -259,7 +259,7 @@ def __ne__(self, other: object) -> bool: return not self.__eq__(other) def __deepcopy__(self, memo: typing.Any) -> SSHAuth: - """Helper for copy.deepcopy. + """Copy the object for copy.deepcopy. :param memo: copy.deepcopy() memodict. :type memo: typing.Any @@ -398,8 +398,7 @@ def get_with_alt_hostname( hostname: str, *host_names: str, default: SSHAuth, - ) -> SSHAuth: - """Try to guess hostname with credentials.""" + ) -> SSHAuth: ... @typing.overload def get_with_alt_hostname( @@ -407,8 +406,7 @@ def get_with_alt_hostname( hostname: str, *host_names: str, default: None = None, - ) -> SSHAuth | None: - """Try to guess hostname with credentials.""" + ) -> SSHAuth | None: ... def get_with_alt_hostname( self, diff --git a/exec_helpers/subprocess.py b/exec_helpers/subprocess.py index 868591c..4bb92bb 100644 --- a/exec_helpers/subprocess.py +++ b/exec_helpers/subprocess.py @@ -21,7 +21,7 @@ import concurrent.futures import contextlib import copy -import datetime +import datetime as dt import errno import logging import os @@ -180,9 +180,10 @@ def __enter__(self) -> SubprocessExecuteAsyncResult: The Command is executed only in the context manager to be sure that everything will be cleaned up properly. """ - started = datetime.datetime.now(tz=datetime.UTC) + started = dt.datetime.now(tz=dt.UTC) - self.__process = subprocess.Popen( + # Running a shell command line is the purpose of this class. + self.__process = subprocess.Popen( # noqa: S602 args=self.command, stdout=subprocess.PIPE if self.open_stdout else subprocess.DEVNULL, stderr=subprocess.PIPE if self.open_stderr else subprocess.DEVNULL, diff --git a/pyproject.toml b/pyproject.toml index c24d317..13fcf40 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,6 @@ build-backend = "setuptools.build_meta" requires = [ "setuptools>=80.3", "setuptools-scm[toml]>=10.0.5", - "wheel>=0.47.0", ] [project] @@ -70,17 +69,20 @@ doc8 = [ "pygments>=2.20.0", ] mypy = [ - "mypy[faster-cache,reports]>=2.1", + "mypy[faster-cache,reports]>=2.4.1", "rich>=15.0.0", + "types-paramiko>=5.0.0", + "types-psutil>=7.2.2", + "types-pyyaml>=6.0.12", ] prek = [ - "prek>=0.4.1", + "prek>=0.5.7", ] pylint = [ - "pylint>=4.0.5", + "pylint>=4.1.3", ] pyproject-fmt = [ - "pyproject-fmt>=2.21.2", + "pyproject-fmt>=2.30.3", ] pytest = [ "asynctest>=0.13", @@ -95,7 +97,7 @@ readme = [ { include-group = "build" }, ] ruff = [ - "ruff>=0.15.13", + "ruff>=0.17.3", ] tox = [ "tox>=4.53.1", @@ -143,7 +145,8 @@ extend-select = [ "A", # flake8-builtins "ASYNC", # flake8-async "B", - "C4", # flake8-return, flake8-simplify, flake8-comprehensions + "C4", # flake8-comprehensions + "D", # pydocstyle "DTZ", # flake8-datetimez "E", "EXE", # flake8-bugbear, flake8-debugger, flake8-executable @@ -154,7 +157,8 @@ extend-select = [ "ICN", "ISC", # flake8-implicit-str-concat "PERF", - "PGH", # flake8-import-conventions, pygrep-hooks + "PGH", # pygrep-hooks + "PIE", # flake8-pie "PL", "PTH", # flake8-use-pathlib "PYI", # flake8-pyi @@ -188,14 +192,7 @@ extend-ignore = [ "TRY003", # long messages prepare outside, ... ] -[tool.isort] -line_length = 120 -multi_line_output = 3 -force_single_line = true -known_third_party = [ "logwrap", "paramiko", "psutil", "pyyaml", "ruamel.yaml", "tenacity" ] - [tool.pylint] -reports = false disable = [ "broad-except", "file-ignored", @@ -268,7 +265,7 @@ collapse_tables = [] skip_wrap_for_keys = [] [tool.mypy] -warn_redundant_casts = true +enable_error_code = [ "ignore-without-code" ] strict = true show_error_context = true show_column_numbers = true @@ -276,7 +273,7 @@ pretty = true warn_unused_configs = true show_error_codes = true [[tool.mypy.overrides]] -module = [ "lxml.*", "psutil" ] +module = [ "lxml.*" ] ignore_missing_imports = true [tool.pytest] @@ -335,7 +332,6 @@ env_list = [ "doc8", "docs", "mypy", - "pep257", { prefix = "py3", start = 11, stop = 15 }, { prefix = "pypy3", start = 11, stop = 15 }, "pylint", @@ -344,19 +340,19 @@ env_list = [ ] skip_missing_interpreters = true [tool.tox.env] -py311.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] -py312.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] -py313.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] -py314.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] -py315.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] -pypy311.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] -pypy312.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] -pypy313.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] -pypy314.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] -pypy315.depends = [ "bandit", "mypy", "pep257", "pylint", "ruff", "ruff-format" ] +py311.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] +py312.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] +py313.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] +py314.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] +py315.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] +pypy311.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] +pypy312.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] +pypy313.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] +pypy314.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] +pypy315.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] [tool.tox.env.bandit] dependency_groups = [ "bandit" ] -ommands = [ [ "bandit", "-r", "exec_helpers" ] ] +commands = [ [ "bandit", "-r", "exec_helpers" ] ] [tool.tox.env.doc8] package = "skip" dependency_groups = [ "doc8" ] @@ -372,14 +368,14 @@ commands = [ [ "pip", "install", "./", "-vvv", "-U" ] ] deps = [ "-r {toxinidir}/CI_REQUIREMENTS.txt" ] dependency_groups = [ "mypy" ] commands = [ - [ "mypy", "--strict", "--install-types", "--non-interactive", "--show-error-codes", "-p", "exec_helpers" ], + [ "mypy", "-p", "exec_helpers" ], ] -depends = [ "bandit", "pep257", "ruff", "ruff-format" ] +depends = [ "bandit", "ruff", "ruff-format" ] [tool.tox.env.pylint] deps = [ "-r {toxinidir}/CI_REQUIREMENTS.txt" ] dependency_groups = [ "pylint" ] commands = [ [ "pylint", "exec_helpers" ] ] -depends = [ "bandit", "pep257", "ruff", "ruff-format" ] +depends = [ "bandit", "ruff", "ruff-format" ] [tool.tox.env.readme] package = "skip" dependency_groups = [ "readme" ] diff --git a/test/async_api/test_subprocess.py b/test/async_api/test_subprocess.py index fc9f949..8adc81b 100644 --- a/test/async_api/test_subprocess.py +++ b/test/async_api/test_subprocess.py @@ -43,7 +43,7 @@ def __init__(self, *args): self.__src = list(args) def __aiter__(self): - """Normally we iter over source.""" + """Iterate over the source normally.""" return self async def __anext__(self): @@ -130,7 +130,7 @@ async def read_stream(stream: FakeFileStream): def pytest_generate_tests(metafunc): - """Tests parametrization.""" + """Test parametrization.""" if "run_parameters" in metafunc.fixturenames: metafunc.parametrize( "run_parameters", @@ -150,7 +150,7 @@ def pytest_generate_tests(metafunc): @pytest.fixture def run_parameters(request): - """Tests configuration apply.""" + """Test configuration apply.""" return configs[request.param] @@ -222,7 +222,7 @@ def create_mock( @pytest.fixture def logger(mocker): - """Simple mock of logger instance.""" + """Mock a logger instance.""" return mocker.patch("exec_helpers.async_api.subprocess.Subprocess.logger", autospec=True) diff --git a/test/async_api/test_subprocess_special.py b/test/async_api/test_subprocess_special.py index 0304cf3..cb6e36c 100644 --- a/test/async_api/test_subprocess_special.py +++ b/test/async_api/test_subprocess_special.py @@ -44,7 +44,7 @@ def __init__(self, *args): self.__src = list(args) def __aiter__(self): - """Normally we iter over source.""" + """Iterate over the source normally.""" return self async def __anext__(self): @@ -189,7 +189,7 @@ class MockParameters(typing.NamedTuple): def pytest_generate_tests(metafunc): - """Tests parametrization.""" + """Test parametrization.""" if "run_parameters" in metafunc.fixturenames: metafunc.parametrize( "run_parameters", @@ -216,7 +216,7 @@ def pytest_generate_tests(metafunc): @pytest.fixture def run_parameters(request): - """Tests configuration apply.""" + """Test configuration apply.""" return configs[request.param] diff --git a/test/pyproject.toml b/test/pyproject.toml index c2d19d6..d296d47 100644 --- a/test/pyproject.toml +++ b/test/pyproject.toml @@ -5,6 +5,7 @@ package = false # Extend the `pyproject.toml` file in the parent directory. extend = "../pyproject.toml" lint.extend-ignore = [ + "D1", # a test name is its description "PLC2701", # private members import "PLR0911", "PLR0912", diff --git a/test/test_exec_result.py b/test/test_exec_result.py index c5d521d..20331b6 100644 --- a/test/test_exec_result.py +++ b/test/test_exec_result.py @@ -20,7 +20,7 @@ from __future__ import annotations -import datetime +import datetime as dt import unittest import xml.etree.ElementTree as ET from unittest import mock @@ -232,7 +232,7 @@ def test_stdin_bytearray(self): def test_started(self): """Test timestamp.""" - started = datetime.datetime.now(tz=datetime.UTC) + started = dt.datetime.now(tz=dt.UTC) result = exec_helpers.ExecResult(cmd, exit_code=0, started=started) spent = (result.timestamp - started).seconds self.assertIs(result.started, started) diff --git a/test/test_ssh_client_execute.py b/test/test_ssh_client_execute.py index 6f97854..af115ce 100644 --- a/test/test_ssh_client_execute.py +++ b/test/test_ssh_client_execute.py @@ -14,7 +14,7 @@ from __future__ import annotations -import datetime +import datetime as dt import logging from unittest import mock @@ -35,7 +35,7 @@ def __init__(self, *args): self.closed = False def __iter__(self): - """Normally we iter over source.""" + """Iterate over the source normally.""" for _ in range(len(self.__src)): yield self.__src.pop(0) @@ -153,7 +153,7 @@ def read_stream(stream: FakeFileStream) -> tuple[bytes, ...]: def pytest_generate_tests(metafunc): - """Tests parametrization.""" + """Test parametrization.""" if "run_parameters" in metafunc.fixturenames: metafunc.parametrize( "run_parameters", @@ -175,7 +175,7 @@ def pytest_generate_tests(metafunc): @pytest.fixture def run_parameters(request): - """Tests configuration apply.""" + """Test configuration apply.""" return configs[request.param] @@ -287,7 +287,7 @@ def get_patched_execute_async_retval( stdin=mock.Mock, stdout=stdout_part, stderr=stderr_part, - started=datetime.datetime.now(tz=datetime.UTC), + started=dt.datetime.now(tz=dt.UTC), ) return mocker.patch( diff --git a/test/test_ssh_client_execute_special.py b/test/test_ssh_client_execute_special.py index 70e9a08..4bca25a 100644 --- a/test/test_ssh_client_execute_special.py +++ b/test/test_ssh_client_execute_special.py @@ -15,7 +15,7 @@ from __future__ import annotations import base64 -import datetime +import datetime as dt import logging import threading from unittest import mock @@ -38,7 +38,7 @@ def __init__(self, *args): self.channel = None def __iter__(self): - """Normally we iter over source.""" + """Iterate over the source normally.""" for _ in range(len(self.__src)): yield self.__src.pop(0) @@ -258,7 +258,7 @@ def get_patched_execute_async_retval() -> SshExecuteAsyncResult: stdin=mock.Mock, stdout=stdout_part, stderr=stderr_part, - started=datetime.datetime.now(tz=datetime.UTC), + started=dt.datetime.now(tz=dt.UTC), ) execute_async = mocker.patch( diff --git a/test/test_ssh_client_execute_through_host.py b/test/test_ssh_client_execute_through_host.py index 616c575..710acb8 100644 --- a/test/test_ssh_client_execute_through_host.py +++ b/test/test_ssh_client_execute_through_host.py @@ -34,7 +34,7 @@ def __init__(self, *args): self.channel = None def __iter__(self): - """Normally we iter over source.""" + """Iterate over the source normally.""" for _ in range(len(self.__src)): yield self.__src.pop(0) diff --git a/test/test_ssh_client_init_basic.py b/test/test_ssh_client_init_basic.py index 7eb32b1..56b587d 100644 --- a/test/test_ssh_client_init_basic.py +++ b/test/test_ssh_client_init_basic.py @@ -76,7 +76,7 @@ def __iter__(self): def pytest_generate_tests(metafunc): - """Tests parametrization.""" + """Test parametrization.""" if "run_parameters" in metafunc.fixturenames: metafunc.parametrize( "run_parameters", @@ -94,7 +94,7 @@ def pytest_generate_tests(metafunc): @pytest.fixture def run_parameters(request): - """Tests configuration apply.""" + """Test configuration apply.""" return configs[request.param] diff --git a/test/test_sshauth.py b/test/test_sshauth.py index 60aff31..99bf692 100644 --- a/test/test_sshauth.py +++ b/test/test_sshauth.py @@ -90,7 +90,7 @@ def get_internal_keys( def pytest_generate_tests(metafunc): - """Tests parametrization.""" + """Test parametrization.""" if "run_parameters" in metafunc.fixturenames: metafunc.parametrize( "run_parameters", @@ -108,7 +108,7 @@ def pytest_generate_tests(metafunc): @pytest.fixture def run_parameters(request): - """Tests configuration apply.""" + """Test configuration apply.""" return configs[request.param] diff --git a/test/test_subprocess.py b/test/test_subprocess.py index 9c261f1..eba3014 100644 --- a/test/test_subprocess.py +++ b/test/test_subprocess.py @@ -42,7 +42,7 @@ def __init__(self, *args): self.closed = False def __iter__(self): - """Normally we iter over source.""" + """Iterate over the source normally.""" for _ in range(len(self.__src)): yield self.__src.pop(0) @@ -127,7 +127,7 @@ def read_stream(stream: FakeFileStream): def pytest_generate_tests(metafunc): - """Tests parametrization.""" + """Test parametrization.""" if "run_parameters" in metafunc.fixturenames: metafunc.parametrize( "run_parameters", @@ -147,7 +147,7 @@ def pytest_generate_tests(metafunc): @pytest.fixture def run_parameters(request): - """Tests configuration apply.""" + """Test configuration apply.""" return configs[request.param] diff --git a/test/test_subprocess_special.py b/test/test_subprocess_special.py index c465c18..45d1925 100644 --- a/test/test_subprocess_special.py +++ b/test/test_subprocess_special.py @@ -43,7 +43,7 @@ def __init__(self, *args): self.closed = False def __iter__(self): - """Normally we iter over source.""" + """Iterate over the source normally.""" for _ in range(len(self.__src)): val = self.__src.pop(0) if isinstance(val, bytes): @@ -183,7 +183,7 @@ class MockParameters(typing.NamedTuple): def pytest_generate_tests(metafunc): - """Tests parametrization.""" + """Test parametrization.""" if "run_parameters" in metafunc.fixturenames: metafunc.parametrize( "run_parameters", @@ -210,7 +210,7 @@ def pytest_generate_tests(metafunc): @pytest.fixture def run_parameters(request): - """Tests configuration apply.""" + """Test configuration apply.""" return configs[request.param] From 2d5a721b10979b78490f8895b035f0f7f9a195b5 Mon Sep 17 00:00:00 2001 From: Aleksei Stepanov Date: Thu, 29 Oct 2026 14:03:27 +0100 Subject: [PATCH 3/8] Restore a closed SSH session before opening SFTP SSHClient closes the paramiko client on the outermost context exit (and on reconnect()), leaving it without a transport. Commands reconnect through _ssh_transport, but SFTP called SSHClient.open_sftp() directly, so the first SFTP call after a close failed with: AttributeError: 'NoneType' object has no attribute 'open_sftp_client' - __connect_sftp opens the channel with _ssh_transport.open_sftp_client(), which reconnects under the client lock when the transport is missing. - A failed reconnect propagates as is; it is not reported as unavailable SFTP. - Add regression tests for SFTP after close and for a failed reconnect; the existing mocks provide open_sftp_client on the transport. --- exec_helpers/_ssh_base.py | 10 +++- test/test_sftp.py | 4 +- test/test_ssh_client_init_basic.py | 2 +- test/test_ssh_client_init_special.py | 81 ++++++++++++++++++++++++++-- 4 files changed, 87 insertions(+), 10 deletions(-) diff --git a/exec_helpers/_ssh_base.py b/exec_helpers/_ssh_base.py index 87e8125..7e4afad 100644 --- a/exec_helpers/_ssh_base.py +++ b/exec_helpers/_ssh_base.py @@ -830,10 +830,11 @@ def __get_client(self) -> paramiko.SSHClient: return last_ssh_client def __connect_sftp(self) -> None: - """SFTP connection opener.""" + """Open the SFTP channel, reconnecting the SSH session first if it was closed.""" with self.lock: + transport = self._ssh_transport # a failed reconnect is not "SFTP unavailable": let it propagate try: - self.__sftp = self.__ssh.open_sftp() + self.__sftp = transport.open_sftp_client() except paramiko.SSHException: self.logger.warning("SFTP enable failed! SSH only is accessible.") @@ -843,6 +844,11 @@ def _sftp(self) -> paramiko.sftp_client.SFTPClient: :rtype: paramiko.sftp_client.SFTPClient :raises paramiko.SSHException: SFTP connection failed. + :raises ConnectionError: SSH session was closed and the reconnect failed. + :raises paramiko.SSHException: SSH session was closed and the reconnect failed (the original exception). + + .. versionchanged:: 8.2.1 The SFTP channel is opened through the reconnecting transport getter, + so a closed SSH session is restored instead of failing with `AttributeError`. """ if self.__sftp is not None: return self.__sftp diff --git a/test/test_sftp.py b/test/test_sftp.py index 7964b3a..9b5fa51 100644 --- a/test/test_sftp.py +++ b/test/test_sftp.py @@ -43,8 +43,8 @@ def prepare_sftp_file_tests(client): ssh_ = mock.Mock() client.return_value = ssh_ sftp = mock.Mock() - open_sftp = mock.Mock(parent=ssh_, return_value=sftp) - ssh_.attach_mock(open_sftp, "open_sftp") + open_sftp = mock.Mock(parent=ssh_.get_transport(), return_value=sftp) + ssh_.get_transport().attach_mock(open_sftp, "open_sftp_client") with ( mock.patch("exec_helpers._ssh_helpers.SSH_CONFIG_FILE_SYSTEM", autospec=True) as conf_sys, diff --git a/test/test_ssh_client_init_basic.py b/test/test_ssh_client_init_basic.py index 56b587d..7dbe68a 100644 --- a/test/test_ssh_client_init_basic.py +++ b/test/test_ssh_client_init_basic.py @@ -142,7 +142,7 @@ def test_init_base( ssh_auth_logger.assert_not_called() sftp = ssh._sftp - assert sftp == paramiko_ssh_client().open_sftp() + assert sftp == paramiko_ssh_client().get_transport().open_sftp_client() assert ssh._ssh == paramiko_ssh_client() assert ssh.hostname == host assert ssh.port == port diff --git a/test/test_ssh_client_init_special.py b/test/test_ssh_client_init_special.py index 0907fde..7b9cb15 100644 --- a/test/test_ssh_client_init_special.py +++ b/test/test_ssh_client_init_special.py @@ -330,7 +330,7 @@ def test_010_context( assert ssh.auth == exec_helpers.SSHAuth() sftp = ssh._sftp - assert sftp == paramiko_ssh_client().open_sftp() + assert sftp == paramiko_ssh_client().get_transport().open_sftp_client() assert ssh._ssh == paramiko_ssh_client() @@ -355,7 +355,7 @@ def test_011_clear_failed( mock.Mock(side_effect=[Exception("Mocked SFTP close()"), mock.Mock()]), "close", ) - ssh_.attach_mock(mock.Mock(return_value=sftp_), "open_sftp") + ssh_.get_transport().attach_mock(mock.Mock(return_value=sftp_), "open_sftp_client") paramiko_ssh_client.return_value = ssh_ ssh_logger = get_logger(exec_helpers.SSHClient.__name__) @@ -372,7 +372,7 @@ def test_011_clear_failed( assert ssh.auth == exec_helpers.SSHAuth() sftp = ssh._sftp - assert sftp == paramiko_ssh_client().open_sftp() + assert sftp == paramiko_ssh_client().get_transport().open_sftp_client() assert ssh._ssh == paramiko_ssh_client() @@ -438,7 +438,7 @@ def test_013_no_sftp( open_sftp = mock.Mock(side_effect=paramiko.SSHException) ssh_ = mock.Mock() - ssh_.attach_mock(open_sftp, "open_sftp") + ssh_.get_transport().attach_mock(open_sftp, "open_sftp_client") paramiko_ssh_client.return_value = ssh_ ssh_logger = get_logger(exec_helpers.SSHClient.__name__) @@ -472,7 +472,7 @@ def test_014_sftp_repair( open_sftp = mock.Mock(side_effect=[paramiko.SSHException, sftp_, sftp_]) ssh_ = mock.Mock() - ssh_.attach_mock(open_sftp, "open_sftp") + ssh_.get_transport().attach_mock(open_sftp, "open_sftp_client") paramiko_ssh_client.return_value = ssh_ ssh_logger = get_logger(exec_helpers.SSHClient.__name__) @@ -499,3 +499,74 @@ def test_014_sftp_repair( sftp = ssh._sftp assert sftp == open_sftp() log.assert_has_calls((mock.call.debug("SFTP is not connected, try to connect..."),)) + + +class ClosableSSHClient: + """Mimic the paramiko behavior: a closed client has no transport, so SFTP can not be opened without a connect.""" + + def __init__(self) -> None: + self.closed = False + self.transport = mock.Mock() + self.sftp = mock.Mock() + self.connect = mock.Mock(side_effect=self.__connect) + self.close = mock.Mock(side_effect=self.__close) + self.transport.open_sftp_client.side_effect = self.__open_sftp_client + + def __connect(self, *args: object, **kwargs: object) -> None: + self.closed = False + + def __close(self) -> None: + self.closed = True + + def __open_sftp_client(self) -> mock.Mock: + if self.closed: + raise AttributeError("'NoneType' object has no attribute 'open_sftp_client'") + return self.sftp + + def get_transport(self) -> mock.Mock | None: + return None if self.closed else self.transport + + def set_missing_host_key_policy(self, *args: object) -> None: + """Ignore host key policy.""" + + def load_system_host_keys(self, *args: object) -> None: + """Ignore system host keys.""" + + +def test_015_sftp_after_close( + paramiko_ssh_client, + ssh_auth_logger, + get_logger, +): + """SFTP channel is opened after the closed SSH session was restored.""" + fake = ClosableSSHClient() + paramiko_ssh_client.return_value = fake + + ssh = exec_helpers.SSHClient(host=host, auth=exec_helpers.SSHAuth(password=password)) + ssh.close() + assert not ssh.is_alive + assert fake.connect.call_count == 1 + + assert ssh._sftp == fake.sftp + assert ssh.is_alive + assert fake.connect.call_count == 2 + + +def test_016_sftp_after_close_reconnect_failed( + paramiko_ssh_client, + ssh_auth_logger, + get_logger, +): + """Failed reconnect is reported as is and no SFTP channel is opened.""" + fake = ClosableSSHClient() + paramiko_ssh_client.return_value = fake + + ssh = exec_helpers.SSHClient(host=host, auth=exec_helpers.SSHAuth(password=password)) + ssh.close() + fake.connect.side_effect = paramiko.SSHException("connect failed") + + with pytest.raises(paramiko.SSHException, match="connect failed"): + # noinspection PyStatementEffect + ssh._sftp # pylint: disable=pointless-statement # noqa: B018 + + fake.transport.open_sftp_client.assert_not_called() From 4495eddb1646f5cb766443a69611cf8c150cb0d0 Mon Sep 17 00:00:00 2001 From: Aleksei Stepanov Date: Thu, 29 Oct 2026 14:24:11 +0100 Subject: [PATCH 4/8] Do not expose secrets in execute context debug strings The repr of the SSH, Subprocess and async Subprocess execute contexts printed the command, the STDIN data and the environment as they are, although they commonly hold passwords and tokens. - The command is shown with the client's log_mask_re applied; ExecuteContext takes the rule as an optional argument. - STDIN is shown by its size and the environment by variable names only. - Add tests for the three contexts, including environments with mixed key types. --- exec_helpers/_helpers.py | 23 ++++++++ exec_helpers/_ssh_base.py | 5 +- exec_helpers/api.py | 14 +++++ exec_helpers/async_api/api.py | 14 +++++ exec_helpers/async_api/subprocess.py | 8 +-- exec_helpers/subprocess.py | 8 +-- test/test_context_repr.py | 78 ++++++++++++++++++++++++++++ 7 files changed, 142 insertions(+), 8 deletions(-) create mode 100644 test/test_context_repr.py diff --git a/exec_helpers/_helpers.py b/exec_helpers/_helpers.py index 617e3aa..21ffa07 100644 --- a/exec_helpers/_helpers.py +++ b/exec_helpers/_helpers.py @@ -9,6 +9,7 @@ if typing.TYPE_CHECKING: from collections.abc import Iterable + from collections.abc import Mapping def string_bytes_bytearray_as_bytes(src: str | bytes | bytearray) -> bytes: @@ -96,3 +97,25 @@ def chroot_command(command: str, chroot_path: str | None = None, chroot_exe: str quoted_command = shlex.quote(command) return f"{chroot_exe} {chroot_dst} sh -c {shlex.quote(f'eval {quoted_command}')}" return command + + +def repr_stdin(stdin: bytes | None) -> str: + """Describe STDIN data for a debug string without exposing it. + + :param stdin: Data for STDIN. + :type stdin: bytes | None + :return: Size of the data, as the content can hold secrets. + :rtype: str + """ + return "None" if stdin is None else f"<{len(stdin)} bytes>" + + +def repr_env(env: Mapping[str, str] | None) -> str: + """Describe an environment for a debug string without exposing the values. + + :param env: Process environment. + :type env: Mapping[str, str] | None + :return: Variable names only, as the values can hold secrets. + :rtype: str + """ + return "None" if env is None else f"" diff --git a/exec_helpers/_ssh_base.py b/exec_helpers/_ssh_base.py index 7e4afad..8b1f43a 100644 --- a/exec_helpers/_ssh_base.py +++ b/exec_helpers/_ssh_base.py @@ -222,8 +222,8 @@ def __repr__(self) -> str: """ return ( f" None: """Execute async context manager. @@ -110,6 +112,8 @@ def __init__( :type open_stderr: bool :param logger: Logger instance. :type logger: logging.Logger + :param log_mask_re: Regex lookup rule to mask the command in the debug string. + :type log_mask_re: str | re.Pattern[str] | None :param kwargs: Additional parameters for call. :type kwargs: typing.Any """ @@ -118,6 +122,7 @@ def __init__( self.__open_stdout = open_stdout self.__open_stderr = open_stderr self.__logger = logger + self.__log_mask_re = log_mask_re if kwargs: self.__logger.warning(f"Unexpected arguments: {kwargs!r}.", stack_info=True) @@ -130,6 +135,15 @@ def logger(self) -> logging.Logger: """ return self.__logger + @property + def _masked_command(self) -> str: + """Command with the log mask applied, for debug output. + + :return: Command without the masked parts. + :rtype: str + """ + return _helpers.mask_command(self.__command.rstrip(), self.__log_mask_re) + @property def command(self) -> str: """Command for execution (fully formatted). diff --git a/exec_helpers/async_api/api.py b/exec_helpers/async_api/api.py index 7743912..8bfa243 100644 --- a/exec_helpers/async_api/api.py +++ b/exec_helpers/async_api/api.py @@ -69,6 +69,7 @@ class ExecuteContext(contextlib.AbstractAsyncContextManager[api.ExecuteAsyncResu __slots__ = ( "__command", + "__log_mask_re", "__logger", "__open_stderr", "__open_stdout", @@ -83,6 +84,7 @@ def __init__( open_stdout: bool = True, open_stderr: bool = True, logger: logging.Logger, + log_mask_re: LogMaskReT = None, **kwargs: typing.Any, ) -> None: """Execute async context manager. @@ -97,6 +99,8 @@ def __init__( :type open_stderr: bool :param logger: Logger instance. :type logger: logging.Logger + :param log_mask_re: Regex lookup rule to mask the command in the debug string. + :type log_mask_re: str | re.Pattern[str] | None :param kwargs: Additional parameters for call. :type kwargs: typing.Any """ @@ -105,6 +109,7 @@ def __init__( self.__open_stdout = open_stdout self.__open_stderr = open_stderr self.__logger = logger + self.__log_mask_re = log_mask_re if kwargs: self.__logger.warning(f"Unexpected arguments: {kwargs!r}.", stack_info=True) @@ -117,6 +122,15 @@ def logger(self) -> logging.Logger: """ return self.__logger + @property + def _masked_command(self) -> str: + """Command with the log mask applied, for debug output. + + :return: Command without the masked parts. + :rtype: str + """ + return _helpers.mask_command(self.__command.rstrip(), self.__log_mask_re) + @property def command(self) -> str: """Command for execution (fully formatted). diff --git a/exec_helpers/async_api/subprocess.py b/exec_helpers/async_api/subprocess.py index 6b2ec78..d92be07 100644 --- a/exec_helpers/async_api/subprocess.py +++ b/exec_helpers/async_api/subprocess.py @@ -36,6 +36,7 @@ from exec_helpers.async_api import api from exec_helpers.async_api import exec_result +from .. import _helpers from .. import _log_templates from .. import _subprocess_helpers @@ -164,12 +165,12 @@ def __repr__(self) -> str: """ return ( f"" ) @@ -401,6 +402,7 @@ def open_execute_context( cwd=cwd, env=env, logger=self.logger, + log_mask_re=self.log_mask_re, **kwargs, ) diff --git a/exec_helpers/subprocess.py b/exec_helpers/subprocess.py index 4bb92bb..d4f1b73 100644 --- a/exec_helpers/subprocess.py +++ b/exec_helpers/subprocess.py @@ -37,6 +37,7 @@ from exec_helpers import exec_result from exec_helpers import proc_enums +from . import _helpers from . import _log_templates from . import _subprocess_helpers @@ -161,12 +162,12 @@ def __repr__(self) -> str: """ return ( f"" ) @@ -422,6 +423,7 @@ def open_execute_context( cwd=cwd, env=env, logger=self.logger, + log_mask_re=self.log_mask_re, **kwargs, ) diff --git a/test/test_context_repr.py b/test/test_context_repr.py new file mode 100644 index 0000000..7ba8076 --- /dev/null +++ b/test/test_context_repr.py @@ -0,0 +1,78 @@ +# Copyright 2018 - 2026 Alexey Stepanov aka penguinolog. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Debug strings of execute contexts do not expose secrets.""" + +from __future__ import annotations + +import exec_helpers +from exec_helpers import async_api + +SECRET = "s3cr3t-value" + + +def check_masked(text: str) -> None: + assert SECRET not in text + assert "<*masked*>" in text + assert f"stdin=<{len(SECRET)} bytes>" in text + + +def test_subprocess_context_repr(): + """Subprocess context hides the command mask, STDIN data and environment values.""" + ctx = exec_helpers.Subprocess(log_mask_re=r"--password (\S+)").open_execute_context( + f"tool --password {SECRET}", + stdin=SECRET, + env={"TOKEN": SECRET}, + ) + text = repr(ctx) + check_masked(text) + assert "env=" in text + + +def test_async_subprocess_context_repr(): + """Async subprocess context hides the command mask, STDIN data and environment values.""" + ctx = async_api.Subprocess(log_mask_re=r"--password (\S+)").open_execute_context( + f"tool --password {SECRET}", + stdin=SECRET, + env={"TOKEN": SECRET}, + ) + text = repr(ctx) + check_masked(text) + assert "env=" in text + + +def test_ssh_context_repr(paramiko_ssh_client, ssh_auth_logger): + """SSH context hides the command mask and STDIN data.""" + ssh = exec_helpers.SSHClient( + host="192.0.2.1", + auth=exec_helpers.SSHAuth(password="pass"), + ) + ssh.log_mask_re = r"--password (\S+)" + ctx = ssh.open_execute_context(f"tool --password {SECRET}", stdin=SECRET) + check_masked(repr(ctx)) + + +def test_context_repr_no_stdin(): + """Absent STDIN and environment are shown as None.""" + text = repr(exec_helpers.Subprocess().open_execute_context("true")) + assert "stdin=None" in text + assert "env=None" in text + + +def test_context_repr_mixed_env_keys(): + """Names of an environment with mixed key types are listed without an error.""" + ctx = exec_helpers.Subprocess().open_execute_context("true", env={b"A": SECRET, "B": SECRET}) + text = repr(ctx) + assert SECRET not in text + assert "env=" in text From 060141377e4a42b0e159d2ad69ee60626baf7280 Mon Sep 17 00:00:00 2001 From: Aleksei Stepanov Date: Thu, 29 Oct 2026 16:21:18 +0100 Subject: [PATCH 5/8] Allow executing commands without a shell in Subprocess Subprocess and async_api.Subprocess always ran the command line through a shell. Add an opt-in shell argument (default True, nothing changes). - With shell=False the command must be a list of arguments: execute() raises TypeError for a string and ValueError for an empty list. - The command stays a joined string in logs and ExecResult.cmd and is split into arguments only when the process is created (Popen without a shell, asyncio.create_subprocess_exec). - chroot is applied to the arguments, without sh -c inside the chroot. - SSHClient is not changed: the remote side always uses a shell. - The execute contexts take use_shell, so the shell= keyword is not passed around (bandit reports B604 on it). - Accept the S603 finding on Popen: the arguments come from the caller. --- README.rst | 6 +- exec_helpers/_helpers.py | 17 ++++ exec_helpers/api.py | 12 ++- exec_helpers/async_api/api.py | 12 ++- exec_helpers/async_api/subprocess.py | 89 +++++++++++++++++--- exec_helpers/subprocess.py | 72 ++++++++++++++-- test/test_shell_option.py | 121 +++++++++++++++++++++++++++ 7 files changed, 309 insertions(+), 20 deletions(-) create mode 100644 test/test_shell_option.py diff --git a/README.rst b/README.rst index 88e4719..7985b71 100644 --- a/README.rst +++ b/README.rst @@ -403,7 +403,11 @@ Keyword arguments: - cwd - working directory. - env - environment variables dict. -.. note:: `shell=true` is always set. +.. note:: The command is executed by the shell by default. + `Subprocess(shell=False)` (and `async_api.Subprocess(shell=False)`) executes it directly instead: + the command must be a list of arguments (a string raises `TypeError`), + and no shell syntax, expansion or `sh -c` inside a chroot is involved. + `SSHClient` can not do this: the remote side always passes the command to a shell. async_api.Subprocess specific ----------------------------- diff --git a/exec_helpers/_helpers.py b/exec_helpers/_helpers.py index 21ffa07..367ae07 100644 --- a/exec_helpers/_helpers.py +++ b/exec_helpers/_helpers.py @@ -99,6 +99,23 @@ def chroot_command(command: str, chroot_path: str | None = None, chroot_exe: str return command +def chroot_argv_command(command: str, chroot_path: str | None = None, chroot_exe: str | None = None) -> str: + """Prepare command for chroot execution without a shell, only the arguments are joined. + + :param command: Original command, a result of `shlex.join`. + :type command: str + :param chroot_path: chroot path. + :type chroot_path: str | None + :param chroot_exe: chroot executable, may carry its own arguments. + :type chroot_exe: str | None + :return: Command to be executed with chroot rules if applicable. + :rtype: str + """ + if chroot_path and chroot_path != "/": + return shlex.join([*shlex.split(chroot_exe or "chroot"), chroot_path.strip(), *shlex.split(command)]) + return command + + def repr_stdin(stdin: bytes | None) -> str: """Describe STDIN data for a debug string without exposing it. diff --git a/exec_helpers/api.py b/exec_helpers/api.py index c93edb3..3502608 100644 --- a/exec_helpers/api.py +++ b/exec_helpers/api.py @@ -395,6 +395,16 @@ def __exit__( self.__context_count -= 1 self.lock.release() + def _command_to_string(self, command: CommandT) -> str: + """Convert the command to the string form used for execution and logs. + + :param command: Original command. + :type command: str | Iterable[str] + :return: Command as a single string. + :rtype: str + """ + return _helpers.cmd_to_string(command) + def _mask_command(self, cmd: str, log_mask_re: LogMaskReT = None) -> str: """Log command with masking and return parsed cmd. @@ -570,7 +580,7 @@ def execute( .. versionchanged:: 8.1.0 chroot_exe added. """ log_level: int = logging.INFO if verbose else logging.DEBUG - cmd = _helpers.cmd_to_string(command) + cmd = self._command_to_string(command) self._log_command_execute( command=cmd, log_mask_re=log_mask_re, diff --git a/exec_helpers/async_api/api.py b/exec_helpers/async_api/api.py index 8bfa243..3f2ad36 100644 --- a/exec_helpers/async_api/api.py +++ b/exec_helpers/async_api/api.py @@ -351,6 +351,16 @@ async def __aexit__( if self.__alock is not None: self.__alock.release() + def _command_to_string(self, command: CommandT) -> str: + """Convert the command to the string form used for execution and logs. + + :param command: Original command. + :type command: str | Iterable[str] + :return: Command as a single string. + :rtype: str + """ + return _helpers.cmd_to_string(command) + def _mask_command(self, cmd: str, log_mask_re: LogMaskReT = None) -> str: """Log command with masking and return parsed cmd. @@ -523,7 +533,7 @@ async def execute( .. versionchanged:: 8.1.0 chroot_exe added. """ log_level: int = logging.INFO if verbose else logging.DEBUG - cmd = _helpers.cmd_to_string(command) + cmd = self._command_to_string(command) self._log_command_execute( command=cmd, log_mask_re=log_mask_re, diff --git a/exec_helpers/async_api/subprocess.py b/exec_helpers/async_api/subprocess.py index d92be07..878e9ff 100644 --- a/exec_helpers/async_api/subprocess.py +++ b/exec_helpers/async_api/subprocess.py @@ -26,6 +26,7 @@ import errno import logging import os +import shlex import typing import warnings @@ -112,7 +113,7 @@ class _SubprocessExecuteContext( ): """Subprocess Execute context.""" - __slots__ = ("__cwd", "__env", "__process") + __slots__ = ("__cwd", "__env", "__process", "__shell") def __init__( self, @@ -123,6 +124,7 @@ def __init__( open_stderr: bool = True, cwd: CwdT = None, env: EnvT = None, + use_shell: bool = True, logger: logging.Logger, **kwargs: typing.Any, ) -> None: @@ -140,6 +142,9 @@ def __init__( :type cwd: str | bytes | pathlib.Path | None :param env: Defines the environment variables for the new process. :type env: Mapping[str | bytes, str | bytes] | None + :param use_shell: Run the command by the shell. + Otherwise, the command is split to arguments and executed directly. + :type use_shell: bool :param logger: Logger instance. :type logger: logging.Logger :param kwargs: Additional parameters for call. @@ -155,6 +160,7 @@ def __init__( ) self.__cwd = cwd self.__env = env + self.__shell = use_shell self.__process: asyncio.subprocess.Process | None = None def __repr__(self) -> str: @@ -171,6 +177,7 @@ def __repr__(self) -> str: f"open_stderr={self.open_stderr!r}, " f"cwd={self.__cwd!r}, " f"env={_helpers.repr_env(self.__env)}, " + f"shell={self.__shell!r}, " f"logger={self.logger!r}) " f"at {id(self)}>" ) @@ -186,16 +193,20 @@ async def __aenter__(self) -> SubprocessExecuteAsyncResult: """ started = dt.datetime.now(tz=dt.UTC) - self.__process = await asyncio.create_subprocess_shell( - cmd=self.command, - stdout=asyncio.subprocess.PIPE if self.open_stdout else asyncio.subprocess.DEVNULL, - stderr=asyncio.subprocess.PIPE if self.open_stderr else asyncio.subprocess.DEVNULL, - stdin=asyncio.subprocess.PIPE, - cwd=self.__cwd, - env=self.__env, - universal_newlines=False, - **_subprocess_helpers.subprocess_kw, - ) + kwargs: dict[str, typing.Any] = { + "stdout": asyncio.subprocess.PIPE if self.open_stdout else asyncio.subprocess.DEVNULL, + "stderr": asyncio.subprocess.PIPE if self.open_stderr else asyncio.subprocess.DEVNULL, + "stdin": asyncio.subprocess.PIPE, + "cwd": self.__cwd, + "env": self.__env, + "universal_newlines": False, + } + kwargs.update(_subprocess_helpers.subprocess_kw) + # Running a shell command line is the default mode of this class, `shell=False` is the opt-out. + if self.__shell: + self.__process = await asyncio.create_subprocess_shell(self.command, **kwargs) + else: + self.__process = await asyncio.create_subprocess_exec(*shlex.split(self.command), **kwargs) process = self.__process if self.stdin is not None: @@ -250,24 +261,77 @@ class Subprocess(api.ExecHelper): :type log_mask_re: str | re.Pattern[str] | None :param logger: Logger instance to use. :type logger: logging.Logger + :param shell: Run commands by the shell. If disabled, commands are accepted only as a list of arguments and + are executed directly, so no shell syntax or expansion is applied to them (chroot included). + The low level `open_execute_context` takes the already joined command and splits it with `shlex`. + :type shell: bool .. versionchanged:: 3.1.0 Not singleton anymore. Only lock is shared between all instances. .. versionchanged:: 3.2.0 Logger can be enforced. .. versionchanged:: 4.1.0 Support chroot .. versionchanged:: 4.3.0 Lock is not shared anymore: allow parallel call of different instances. + .. versionchanged:: 8.2.1 Added shell option: commands can be executed without a shell. """ - __slots__ = () + __slots__ = ("__shell",) def __init__( self, log_mask_re: LogMaskReT = None, *, logger: logging.Logger = logging.getLogger(__name__), # noqa: B008 + shell: bool = True, ) -> None: """Subprocess helper with timeouts and lock-free FIFO.""" + self.__shell = shell super().__init__(logger=logger, log_mask_re=log_mask_re) + @property + def shell(self) -> bool: + """Run commands by the shell. + + :return: False if commands are executed directly, without a shell. + :rtype: bool + """ + return self.__shell + + def _command_to_string(self, command: CommandT) -> str: + """Convert the command to the string form used for execution and logs. + + :param command: Original command. + :type command: str | Iterable[str] + :return: Command as a single string. + :rtype: str + :raises TypeError: Command is a string, but shell is disabled. + :raises ValueError: Command has no arguments, but shell is disabled. + """ + if not self.__shell and isinstance(command, str): + raise TypeError("Without a shell the command is expected as a list of arguments, not as a string") + cmd = super()._command_to_string(command) + if not self.__shell and not cmd: + raise ValueError("Without a shell the command needs at least the program to execute") + return cmd + + def _prepare_command(self, cmd: str, chroot_path: str | None = None, chroot_exe: str | None = None) -> str: + """Prepare command: cover chroot and other cases. + + :param cmd: Main command. + :type cmd: str + :param chroot_path: Path to make chroot for execution. + :type chroot_path: str | None + :param chroot_exe: chroot exe override. + :type chroot_exe: str | None + :return: Final command, includes chroot, if required. + :rtype: str + """ + if self.__shell: + return super()._prepare_command(cmd, chroot_path=chroot_path, chroot_exe=chroot_exe) + return _helpers.chroot_argv_command( + cmd, + chroot_path=chroot_path or self._chroot_path, + chroot_exe=chroot_exe or self._chroot_exe, + ) + async def _exec_command( # type: ignore[override] self, command: str, @@ -401,6 +465,7 @@ def open_execute_context( open_stderr=open_stderr, cwd=cwd, env=env, + use_shell=self.__shell, logger=self.logger, log_mask_re=self.log_mask_re, **kwargs, diff --git a/exec_helpers/subprocess.py b/exec_helpers/subprocess.py index d4f1b73..492609e 100644 --- a/exec_helpers/subprocess.py +++ b/exec_helpers/subprocess.py @@ -26,6 +26,7 @@ import logging import os import pathlib +import shlex import subprocess # nosec # Expected usage import typing import warnings @@ -109,7 +110,7 @@ def stdout(self) -> typing.IO[bytes] | None: class _SubprocessExecuteContext(api.ExecuteContext, contextlib.AbstractContextManager[SubprocessExecuteAsyncResult]): """Subprocess Execute context.""" - __slots__ = ("__cwd", "__env", "__process") + __slots__ = ("__cwd", "__env", "__process", "__shell") def __init__( self, @@ -120,6 +121,7 @@ def __init__( open_stderr: bool = True, cwd: CwdT = None, env: EnvT = None, + use_shell: bool = True, logger: logging.Logger, **kwargs: typing.Any, ) -> None: @@ -137,6 +139,9 @@ def __init__( :type cwd: str | bytes | pathlib.Path | None :param env: Defines the environment variables for the new process. :type env: Mapping[str | bytes, str | bytes] | None + :param use_shell: Run the command by the shell. + Otherwise, the command is split to arguments and executed directly. + :type use_shell: bool :param logger: Logger instance. :type logger: logging.Logger :param kwargs: Additional parameters for call. @@ -152,6 +157,7 @@ def __init__( ) self.__cwd = cwd self.__env = env + self.__shell = use_shell self.__process: subprocess.Popen[bytes] | None = None def __repr__(self) -> str: @@ -168,6 +174,7 @@ def __repr__(self) -> str: f"open_stderr={self.open_stderr!r}, " f"cwd={self.__cwd!r}, " f"env={_helpers.repr_env(self.__env)}, " + f"shell={self.__shell!r}, " f"logger={self.logger!r}) " f"at {id(self)}>" ) @@ -183,13 +190,13 @@ def __enter__(self) -> SubprocessExecuteAsyncResult: """ started = dt.datetime.now(tz=dt.UTC) - # Running a shell command line is the purpose of this class. - self.__process = subprocess.Popen( # noqa: S602 - args=self.command, + # Running a shell command line is the default mode of this class, `shell=False` is the opt-out. + self.__process = subprocess.Popen( # noqa: S603 # the arguments come from the caller by design + args=self.command if self.__shell else shlex.split(self.command), stdout=subprocess.PIPE if self.open_stdout else subprocess.DEVNULL, stderr=subprocess.PIPE if self.open_stderr else subprocess.DEVNULL, stdin=subprocess.PIPE, - shell=True, + shell=self.__shell, cwd=self.__cwd, env=self.__env, universal_newlines=False, @@ -252,25 +259,79 @@ class Subprocess(api.ExecHelper): :param log_mask_re: Regex lookup rule to mask command for logger. All MATCHED groups will be replaced by '<*masked*>'. :type log_mask_re: str | re.Pattern[str] | None + :param shell: Run commands by the shell. If disabled, commands are accepted only as a list of arguments and + are executed directly, so no shell syntax or expansion is applied to them (chroot included). + The low level `open_execute_context` takes the already joined command and splits it with `shlex`. + :type shell: bool .. versionchanged:: 1.2.0 log_mask_re regex rule for masking cmd. .. versionchanged:: 3.1.0 Not singleton anymore. Only lock is shared between all instances. .. versionchanged:: 3.2.0 Logger can be enforced. .. versionchanged:: 4.1.0 support chroot. .. versionchanged:: 4.3.0 Lock is not shared anymore: allow parallel call of different instances. + .. versionchanged:: 8.2.1 Added shell option: commands can be executed without a shell. """ def __init__( self, log_mask_re: LogMaskReT = None, + *, + shell: bool = True, ) -> None: """Subprocess helper with timeouts and lock-free FIFO.""" + self.__shell = shell mod_name = "exec_helpers" if self.__module__.startswith("exec_helpers") else self.__module__ super().__init__( logger=logging.getLogger(f"{mod_name}.{self.__class__.__name__}"), log_mask_re=log_mask_re, ) + @property + def shell(self) -> bool: + """Run commands by the shell. + + :return: False if commands are executed directly, without a shell. + :rtype: bool + """ + return self.__shell + + def _command_to_string(self, command: CommandT) -> str: + """Convert the command to the string form used for execution and logs. + + :param command: Original command. + :type command: str | Iterable[str] + :return: Command as a single string. + :rtype: str + :raises TypeError: Command is a string, but shell is disabled. + :raises ValueError: Command has no arguments, but shell is disabled. + """ + if not self.__shell and isinstance(command, str): + raise TypeError("Without a shell the command is expected as a list of arguments, not as a string") + cmd = super()._command_to_string(command) + if not self.__shell and not cmd: + raise ValueError("Without a shell the command needs at least the program to execute") + return cmd + + def _prepare_command(self, cmd: str, chroot_path: str | None = None, chroot_exe: str | None = None) -> str: + """Prepare command: cover chroot and other cases. + + :param cmd: Main command. + :type cmd: str + :param chroot_path: Path to make chroot for execution. + :type chroot_path: str | None + :param chroot_exe: chroot exe override. + :type chroot_exe: str | None + :return: Final command, includes chroot, if required. + :rtype: str + """ + if self.__shell: + return super()._prepare_command(cmd, chroot_path=chroot_path, chroot_exe=chroot_exe) + return _helpers.chroot_argv_command( + cmd, + chroot_path=chroot_path or self._chroot_path, + chroot_exe=chroot_exe or self._chroot_exe, + ) + def _exec_command( # type: ignore[override] self, command: str, @@ -422,6 +483,7 @@ def open_execute_context( open_stderr=open_stderr, cwd=cwd, env=env, + use_shell=self.__shell, logger=self.logger, log_mask_re=self.log_mask_re, **kwargs, diff --git a/test/test_shell_option.py b/test/test_shell_option.py new file mode 100644 index 0000000..5d9cd16 --- /dev/null +++ b/test/test_shell_option.py @@ -0,0 +1,121 @@ +# Copyright 2018 - 2026 Alexey Stepanov aka penguinolog. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +"""Execution without a shell.""" + +from __future__ import annotations + +import shlex + +import pytest + +import exec_helpers +from exec_helpers import async_api + +ARGS = ["echo", "a b; c", "$HOME"] +COMMAND = shlex.join(ARGS) + + +@pytest.fixture +def async_spawn(mocker): + """Mock process creation of asyncio: shell and exec variants, the same finished process.""" + create_shell = mocker.patch("asyncio.create_subprocess_shell") + create_exec = mocker.patch("asyncio.create_subprocess_exec") + process = mocker.MagicMock(returncode=0, stdin=None, wait=mocker.AsyncMock()) + create_shell.return_value = create_exec.return_value = process + mocker.patch("exec_helpers._subprocess_helpers.kill_proc_tree") + return create_shell, create_exec + + +def test_shell_default(mocker): + """The shell is used by default and the command is passed as is.""" + popen = mocker.patch("subprocess.Popen", autospec=True) + + with exec_helpers.Subprocess().open_execute_context("echo a | cat"): + pass + + assert popen.call_args.kwargs["shell"] is True + assert popen.call_args.kwargs["args"] == "echo a | cat\n" + + +def test_shell_disabled(mocker): + """The command is split to the arguments and executed directly.""" + popen = mocker.patch("subprocess.Popen", autospec=True) + + with exec_helpers.Subprocess(shell=False).open_execute_context(COMMAND): + pass + + assert popen.call_args.kwargs["shell"] is False + assert popen.call_args.kwargs["args"] == ARGS + + +def test_shell_disabled_chroot(mocker): + """Chroot is applied to the arguments, without a shell inside.""" + popen = mocker.patch("subprocess.Popen", autospec=True) + + with exec_helpers.Subprocess(shell=False).open_execute_context(COMMAND, chroot_path="/mnt/target dir"): + pass + + assert popen.call_args.kwargs["args"] == ["chroot", "/mnt/target dir", *ARGS] + + +def test_shell_disabled_string_rejected(): + """A string command is not accepted without a shell: its shell syntax would silently change the meaning.""" + with pytest.raises(TypeError, match="list of arguments"): + exec_helpers.Subprocess(shell=False).execute("echo a | cat") + + +def test_shell_disabled_empty_command(): + """A command without a program is rejected with a clear error.""" + with pytest.raises(ValueError, match="at least the program"): + exec_helpers.Subprocess(shell=False).execute([]) + + +@pytest.mark.asyncio +async def test_async_shell_default(async_spawn): + """The shell is used by default.""" + create_shell, create_exec = async_spawn + + async with async_api.Subprocess().open_execute_context("echo a | cat"): + pass + + assert create_shell.call_args.args == ("echo a | cat\n",) + create_exec.assert_not_called() + + +@pytest.mark.asyncio +async def test_async_shell_disabled(async_spawn): + """The command is split to the arguments and executed directly.""" + create_shell, create_exec = async_spawn + + async with async_api.Subprocess(shell=False).open_execute_context(COMMAND, chroot_path="/mnt"): + pass + + assert create_exec.call_args.args == ("chroot", "/mnt", *ARGS) + create_shell.assert_not_called() + + +@pytest.mark.asyncio +async def test_async_shell_disabled_string_rejected(): + """A string command is not accepted without a shell.""" + with pytest.raises(TypeError, match="list of arguments"): + await async_api.Subprocess(shell=False).execute("echo a | cat") + + +@pytest.mark.asyncio +async def test_async_shell_disabled_empty_command(): + """A command without a program is rejected with a clear error.""" + with pytest.raises(ValueError, match="at least the program"): + await async_api.Subprocess(shell=False).execute([]) From a9836b0a4dc2307674393e092e8f2636715bbfcb Mon Sep 17 00:00:00 2001 From: Aleksei Stepanov Date: Thu, 29 Oct 2026 16:24:12 +0100 Subject: [PATCH 6/8] Make failed SSH connections bounded and clean A stalled or rejected connection attempt blocked connect for a long time and leaked paramiko's transport thread and socket. - SSHClient takes connect_timeout, default None (nothing changes). SSHAuth.connect takes timeout and passes it to paramiko as timeout, banner_timeout and auth_timeout. It is applied at every connection step: the direct connection, a caller socket, each jump host of a chain and the clients created by proxy_to and execute_through_host. A value that is not positive raises ValueError. - SSHAuth.connect closes the client after every failed attempt, before the next key is tried and before an error is raised. A caller socket is closed with the failed attempt, so no further key is tried on it. - The connection chain closes every client created for it, in reverse order, if a host of the chain cannot be reached. - A connection on a supplied socket is not retried: the failed attempt has closed it, so a retry would only fail with OSError and hide the first error. --- doc/source/SSHClient.rst | 6 ++- exec_helpers/_ssh_base.py | 62 ++++++++++++++++++++++++-- exec_helpers/ssh_auth.py | 17 +++++++- test/test_ssh_client_init_basic.py | 65 ++++++++++++++++++++++++++++ test/test_ssh_client_init_special.py | 3 ++ test/test_sshauth.py | 53 +++++++++++++++++++++++ 6 files changed, 200 insertions(+), 6 deletions(-) diff --git a/doc/source/SSHClient.rst b/doc/source/SSHClient.rst index 98f9a3e..1d291a8 100644 --- a/doc/source/SSHClient.rst +++ b/doc/source/SSHClient.rst @@ -10,7 +10,7 @@ API: SSHClient and SSHAuth. SSHClient helper. - .. py:method:: __init__(host, port=22, username=None, password=None, *, auth=None, verbose=True, ssh_config=None, ssh_auth_map=None, sock=None, keepalive=1, allow_ssh_agent=True) + .. py:method:: __init__(host, port=22, username=None, password=None, *, auth=None, verbose=True, ssh_config=None, ssh_auth_map=None, sock=None, keepalive=1, allow_ssh_agent=True, connect_timeout=None) :param host: remote hostname :type host: ``str`` @@ -34,6 +34,9 @@ API: SSHClient and SSHAuth. :type keepalive: int | bool :param allow_ssh_agent: use SSH Agent if available :type allow_ssh_agent: bool + :param connect_timeout: timeout of the TCP connection, the SSH banner, the key exchange and the authentication, + applied to every connection step (a connection attempt is retried 3 times with a 3 s pause) + :type connect_timeout: float | None .. note:: auth has priority over username/password/private_keys .. note:: @@ -52,6 +55,7 @@ API: SSHClient and SSHAuth. .. versionchanged:: 7.0.0 keepalive_mode is removed .. versionchanged:: 7.4.0 return of keepalive_mode to prevent mix with keepalive period. Default is `False` .. versionchanged:: 8.0.0 expose SSH Agent usage override + .. versionchanged:: 8.2.1 added connect_timeout .. py:attribute:: log_mask_re diff --git a/exec_helpers/_ssh_base.py b/exec_helpers/_ssh_base.py index 8b1f43a..983b1b5 100644 --- a/exec_helpers/_ssh_base.py +++ b/exec_helpers/_ssh_base.py @@ -458,6 +458,13 @@ class SSHClientBase(api.ExecHelper): :type allow_ssh_agent: bool :param missing_host_key_policy: Missing host key policy class. See paramiko.MissingHostKeyPolicy for details. :type missing_host_key_policy: type[paramiko.MissingHostKeyPolicy] + :param connect_timeout: Timeout in seconds for the TCP connection, the SSH banner, the key exchange and the + authentication of every connection step, including hosts reached through `proxy_to`. + An authentication timeout is reported as an authentication failure. By default: None + (paramiko defaults: no TCP and key exchange limit, banner 15 s, authentication 30 s). A connection attempt + is retried 3 times with a 3 s pause, so a hung host takes up to 3 times the timeout plus 6 s to report. + :type connect_timeout: float | None + :raises ValueError: connect_timeout is not positive. .. note:: auth has priority over username/password/private_keys. .. note:: @@ -477,12 +484,14 @@ class SSHClientBase(api.ExecHelper): .. versionchanged:: 7.4.0 Return of keepalive_mode to prevent mix with a keepalive period. Default is `False`. .. versionchanged:: 8.0.0 Expose SSH Agent usage override. .. versionchanged:: 8.1.4 Allow specifying missing_host_key_policy. + .. versionchanged:: 8.2.1 Added connect_timeout. """ __slots__ = ( "__allow_agent", "__auth_mapping", "__conn_chain", + "__connect_timeout", "__hostname", "__keepalive_mode", "__keepalive_period", @@ -519,6 +528,7 @@ def __init__( keepalive: KeepAlivePeriodT = 1, allow_ssh_agent: bool = True, missing_host_key_policy: type[paramiko.MissingHostKeyPolicy] = paramiko.WarningPolicy, + connect_timeout: float | None = None, ) -> None: """Initialize the SSH client helper.""" self.__sudo_mode = False @@ -527,6 +537,9 @@ def __init__( self.__verbose: bool = verbose self.__sock = sock self._missing_host_key_policy = missing_host_key_policy + if connect_timeout is not None and connect_timeout <= 0: + raise ValueError(f"connect_timeout should be positive, got {connect_timeout!r}") + self.__connect_timeout = connect_timeout self.__ssh: paramiko.SSHClient self.__sftp: paramiko.SFTPClient | None = None @@ -749,14 +762,30 @@ def _ssh(self) -> paramiko.SSHClient: """ return self.__ssh + def __connect(self) -> None: + """Open the connection, with retries unless a socket was supplied. + + A failed attempt closes the supplied socket, so there is nothing to retry on: + the next attempt would only fail on the closed socket and hide the original error. + """ + with self.lock: + if self.__sock is None: + self.__connect_with_retries() + else: + self.__connect_once() + @tenacity.retry( retry=RetryOnExceptions(retry_on=paramiko.SSHException, reraise=paramiko.AuthenticationException), stop=tenacity.stop.stop_after_attempt(3), wait=tenacity.wait.wait_fixed(3), reraise=True, ) - def __connect(self) -> None: - """Open the connection.""" + def __connect_with_retries(self) -> None: + """Open the connection, retry on the connection errors.""" + self.__connect_once() + + def __connect_once(self) -> None: + """Open the connection, one attempt.""" with self.lock: if self.__sock is not None: self.__ssh = paramiko.SSHClient() @@ -769,6 +798,7 @@ def __connect(self) -> None: log=self.__verbose, sock=self.__sock, allow_ssh_agent=self.allow_ssh_agent, + timeout=self.__connect_timeout, ) else: self.__ssh = self.__get_client() @@ -785,22 +815,44 @@ def __get_client(self) -> paramiko.SSHClient: :raises RuntimeError: Unexpected state. :raises ConnectionError: Cannot get SSH transport. """ + opened: list[paramiko.SSHClient] = [] + try: + return self.__connect_chain(opened) + except BaseException: + for client in reversed(opened): # paramiko leaves the transports of a failed connect running + client.close() + raise + + def __connect_chain(self, opened: list[paramiko.SSHClient]) -> paramiko.SSHClient: + """Connect the hosts of the connection chain one by one. + + :param opened: Every client created here is added to the list at once, for the cleanup on failure. + :type opened: list[paramiko.SSHClient] + :return: Paramiko ssh connection object of the last host. + :rtype: paramiko.SSHClient + :raises ValueError: ProxyCommand found in a connection chain after the first host reached an. + :raises RuntimeError: Unexpected state. + :raises ConnectionError: Cannot get SSH transport. + """ + config, auth = self.__conn_chain[0] + last_ssh_client: paramiko.SSHClient = paramiko.SSHClient() + opened.append(last_ssh_client) last_ssh_client.load_system_host_keys() last_ssh_client.set_missing_host_key_policy(self._missing_host_key_policy) - config, auth = self.__conn_chain[0] - auth.connect( last_ssh_client, hostname=config.hostname, port=config.port or 22, sock=paramiko.ProxyCommand(config.proxycommand) if config.proxycommand else None, allow_ssh_agent=self.allow_ssh_agent, + timeout=self.__connect_timeout, ) for config, auth in self.__conn_chain[1:]: # start has another logic, so do it out of cycle ssh = paramiko.SSHClient() + opened.append(ssh) ssh.load_system_host_keys() ssh.set_missing_host_key_policy(self._missing_host_key_policy) @@ -819,6 +871,7 @@ def __get_client(self) -> paramiko.SSHClient: port=config.port or 22, sock=sock, allow_ssh_agent=self.allow_ssh_agent, + timeout=self.__connect_timeout, ) last_ssh_client = ssh continue @@ -1570,6 +1623,7 @@ def proxy_to( sock=sock, ssh_auth_map=ssh_auth_map if ssh_auth_map is not None else self.__auth_mapping, keepalive=int(keepalive), + connect_timeout=self.__connect_timeout, ) def execute_through_host( diff --git a/exec_helpers/ssh_auth.py b/exec_helpers/ssh_auth.py index 41b1137..d7b3e06 100644 --- a/exec_helpers/ssh_auth.py +++ b/exec_helpers/ssh_auth.py @@ -165,6 +165,7 @@ def connect( *, sock: paramiko.ProxyCommand | paramiko.Channel | socket.socket | None = None, allow_ssh_agent: bool = True, + timeout: float | None = None, ) -> None: """Connect the SSH client object using credentials. @@ -180,8 +181,13 @@ def connect( :type sock: paramiko.ProxyCommand | paramiko.Channel | socket.socket | None :param allow_ssh_agent: Use SSH Agent if available. :type allow_ssh_agent: bool + :param timeout: Timeout in seconds for the TCP connection, the SSH banner and the authentication. + :type timeout: float | None :raises PasswordRequiredException: No password has been set, but required. - :raises AuthenticationException: Authentication failed. + :raises AuthenticationException: Authentication failed (an authentication timeout is reported the same way). + :raises TimeoutError: TCP connection timeout. + :raises paramiko.SSHException: Banner or key exchange timeout. + :raises BaseException: Any other failure of a connection attempt; the client is closed first. """ kwargs: dict[str, typing.Any] = {} @@ -189,6 +195,8 @@ def connect( kwargs["passphrase"] = self.__passphrase if sock is not None: kwargs["sock"] = sock + if timeout is not None: + kwargs.update(timeout=timeout, banner_timeout=timeout, auth_timeout=timeout) for index, key in sorted(enumerate(self.__keys), key=lambda i_k: i_k[0] != self.__key_index): kwargs["pkey"] = key @@ -207,13 +215,20 @@ def connect( self.__key_index = index LOGGER.debug(f"Main key has been updated, public key is: \n{self.public_key}") except paramiko.PasswordRequiredException: + client.close() # paramiko leaves the transport of a failed connect running if self.__password is None: LOGGER.exception("No password has been set!") raise LOGGER.critical("Unexpected PasswordRequiredException, when password is set!") raise except (paramiko.AuthenticationException, paramiko.BadHostKeyException): + client.close() # this also closes the supplied sock, so it can not be used for the next key + if sock is not None: + break continue + except BaseException: + client.close() + raise else: return msg: str = "Connection using stored authentication info failed!" diff --git a/test/test_ssh_client_init_basic.py b/test/test_ssh_client_init_basic.py index 7dbe68a..cfbe0dc 100644 --- a/test/test_ssh_client_init_basic.py +++ b/test/test_ssh_client_init_basic.py @@ -155,3 +155,68 @@ def test_init_base( assert ssh.ssh_config[host] == expected_config_dict[host] assert ssh.ssh_config == expected_config_dict assert ssh.ssh_config[host].hostname == host + + +@pytest.mark.parametrize("through", ("direct", "sock", "proxyjump", "proxy_to")) +def test_connect_timeout_every_step(paramiko_ssh_client, ssh_auth_logger, through): + """The connect timeout is applied to every connection step.""" + kwargs = {} + if through == "sock": + kwargs["sock"] = mock.Mock() + elif through == "proxyjump": + kwargs["ssh_config"] = { + "jump": {"hostname": "192.0.2.1"}, + host: {"hostname": host, "proxyjump": "jump"}, + } + kwargs["ssh_auth_map"] = {"192.0.2.1": exec_helpers.SSHAuth(), host: exec_helpers.SSHAuth()} + + ssh = exec_helpers.SSHClient(host=host, auth=exec_helpers.SSHAuth(), connect_timeout=7, **kwargs) + if through == "proxy_to": + ssh.proxy_to("192.0.2.2", auth=exec_helpers.SSHAuth()) + + connects = [call for call in paramiko_ssh_client.return_value.mock_calls if call[0] == "connect"] + assert len(connects) >= (2 if through in {"proxyjump", "proxy_to"} else 1) + for call in connects: + assert (call.kwargs["timeout"], call.kwargs["banner_timeout"], call.kwargs["auth_timeout"]) == (7, 7, 7) + + +@pytest.mark.parametrize("value", (0, -1)) +def test_connect_timeout_not_positive(paramiko_ssh_client, value): + """A timeout that disables waiting is rejected.""" + with pytest.raises(ValueError, match="connect_timeout"): + exec_helpers.SSHClient(host=host, auth=exec_helpers.SSHAuth(), connect_timeout=value) + + +def test_failed_chain_closes_every_reached_host(paramiko_ssh_client, ssh_auth_logger): + """A failure on a jump host chain closes the clients of all the hosts created so far.""" + first = mock.Mock() + second = mock.Mock() + second.connect.side_effect = TimeoutError + paramiko_ssh_client.side_effect = [first, second] + config = { + "jump": {"hostname": "192.0.2.1"}, + host: {"hostname": host, "proxyjump": "jump"}, + } + + with pytest.raises(TimeoutError): + exec_helpers.SSHClient( + host=host, + auth=exec_helpers.SSHAuth(), + ssh_config=config, + ssh_auth_map={"192.0.2.1": exec_helpers.SSHAuth(), host: exec_helpers.SSHAuth()}, + ) + + first.close.assert_called() + second.close.assert_called() + + +@pytest.mark.parametrize(("supplied_sock", "attempts"), ((False, 3), (True, 1))) +def test_connect_retries(paramiko_ssh_client, ssh_auth_logger, supplied_sock, attempts): + """A failed connection is retried, except on a supplied socket: the failed attempt has closed it.""" + paramiko_ssh_client.return_value.connect.side_effect = paramiko.SSHException("banner") + kwargs = {"sock": mock.Mock()} if supplied_sock else {} + + with pytest.raises(paramiko.SSHException, match="banner"): + exec_helpers.SSHClient(host=host, auth=exec_helpers.SSHAuth(), **kwargs) + + assert paramiko_ssh_client.return_value.connect.call_count == attempts diff --git a/test/test_ssh_client_init_special.py b/test/test_ssh_client_init_special.py index 7b9cb15..d2abe79 100644 --- a/test/test_ssh_client_init_special.py +++ b/test/test_ssh_client_init_special.py @@ -95,6 +95,7 @@ def test_001_require_key( ssh_.load_system_host_keys(), ssh_.set_missing_host_key_policy(paramiko.WarningPolicy), ssh_.connect(**kwargs_full), + ssh_.close(), # the failed attempt is closed before the next one ssh_.connect(**kwargs_no_key), ssh_.get_transport(), ssh_.get_transport().set_keepalive(1), @@ -152,7 +153,9 @@ def test_002_use_next_key( ssh_.load_system_host_keys(), ssh_.set_missing_host_key_policy(paramiko.WarningPolicy), ssh_.connect(**kwargs_key_0), + ssh_.close(), ssh_.connect(**kwargs_key_1), + ssh_.close(), ssh_.connect(**kwargs_no_key), ssh_.get_transport(), ssh_.get_transport().set_keepalive(1), diff --git a/test/test_sshauth.py b/test/test_sshauth.py index 99bf692..7a7e76c 100644 --- a/test/test_sshauth.py +++ b/test/test_sshauth.py @@ -163,3 +163,56 @@ def test_002_equality_copy(): assert auth3 is not copy.copy(auth3) assert auth3 == copy.deepcopy(auth3) assert auth3 is not copy.deepcopy(auth3) + + +def test_connect_timeout(mocker): + """The connect timeout limits the TCP connection, the banner and the authentication.""" + client = mocker.Mock() + auth = exec_helpers.SSHAuth(username="user", password="pass") + + auth.connect(client, hostname="192.0.2.1", timeout=5) + kwargs = client.connect.call_args.kwargs + assert (kwargs["timeout"], kwargs["banner_timeout"], kwargs["auth_timeout"]) == (5, 5, 5) + + client.reset_mock() + auth.connect(client, hostname="192.0.2.1") + kwargs = client.connect.call_args.kwargs + assert not {"timeout", "banner_timeout", "auth_timeout"} & set(kwargs) + + +@pytest.mark.parametrize("error", (TimeoutError, paramiko.SSHException, paramiko.PasswordRequiredException)) +def test_connect_failure_closes_client(mocker, error): + """The transport of a failed connection attempt is closed: paramiko leaves it running.""" + client = mocker.Mock() + client.connect.side_effect = error + auth = exec_helpers.SSHAuth(username="user", password="pass") + + with pytest.raises(error): + auth.connect(client, hostname="192.0.2.1") + + client.close.assert_called_once_with() + + +def test_connect_all_attempts_failed_closes_client(mocker): + """Every failed attempt of the authentication loop is closed.""" + client = mocker.Mock() + client.connect.side_effect = paramiko.AuthenticationException + auth = exec_helpers.SSHAuth(username="user", password="pass") + + with pytest.raises(paramiko.AuthenticationException): + auth.connect(client, hostname="192.0.2.1", log=False) + + assert client.close.call_count == client.connect.call_count >= 1 + + +def test_connect_supplied_sock_single_attempt(mocker): + """A supplied socket is closed with the failed attempt, so the next key is not tried on it.""" + client = mocker.Mock() + client.connect.side_effect = paramiko.AuthenticationException + auth = exec_helpers.SSHAuth(username="user", keys=[paramiko.RSAKey.generate(1024), paramiko.RSAKey.generate(1024)]) + + with pytest.raises(paramiko.AuthenticationException, match="failed"): + auth.connect(client, hostname="192.0.2.1", sock=mocker.Mock(), log=False) + + client.connect.assert_called_once() + client.close.assert_called_once_with() From 87505b4569184bd541771c87f3da009ae3d3a0de Mon Sep 17 00:00:00 2001 From: Aleksei Stepanov Date: Thu, 29 Oct 2026 16:24:22 +0100 Subject: [PATCH 7/8] Do not warn about the unknown host key of the local host Connecting to the own machine (127.0.0.1, localhost, the own host name) printed the paramiko WarningPolicy warning every time, because the local host is normally absent from known_hosts. Nothing that worked before may fail, so the host key is still checked by the configured policy. - Add is_local_host: the loopback range, localhost, own host name or its short form, and addresses of local interfaces. It does no name resolution, treats an unscoped link-local address as not local, and answers "not local" if the interface list cannot be read. - Add local_host_keys: the public keys of this machine's SSH server (/etc/ssh/ssh_host_*_key.pub), read on every call; unreadable and malformed files are skipped. - A direct connection to a local host (no proxy command, jump host or caller socket) uses LocalHostKeyPolicy: a server key equal to a host key of this machine is accepted, every other key goes to the configured policy. The default policy still warns and RejectPolicy still rejects, on any address and port. A known_hosts entry is checked first, as before. - A failed lookup of the local host or its keys leaves the configured policy. - Tests do not read the host keys of the machine running them. --- exec_helpers/_ssh_base.py | 27 ++++- exec_helpers/_ssh_helpers.py | 95 +++++++++++++++ test/conftest.py | 6 + test/test_ssh_client_init_basic.py | 183 +++++++++++++++++++++++++++++ 4 files changed, 310 insertions(+), 1 deletion(-) diff --git a/exec_helpers/_ssh_base.py b/exec_helpers/_ssh_base.py index 983b1b5..1d54ecc 100644 --- a/exec_helpers/_ssh_base.py +++ b/exec_helpers/_ssh_base.py @@ -485,6 +485,8 @@ class SSHClientBase(api.ExecHelper): .. versionchanged:: 8.0.0 Expose SSH Agent usage override. .. versionchanged:: 8.1.4 Allow specifying missing_host_key_policy. .. versionchanged:: 8.2.1 Added connect_timeout. + .. versionchanged:: 8.2.1 Direct connection to the local host accepts the host keys of this machine without + the unknown host key warning; a key of another server is handled by the missing host key policy as before. """ __slots__ = ( @@ -806,6 +808,27 @@ def __connect_once(self) -> None: transport: paramiko.Transport = self._ssh_transport transport.set_keepalive(1 if self.__keepalive_period else 0) # send keepalive packets + def __host_key_policy(self, hostname: str) -> paramiko.MissingHostKeyPolicy | type[paramiko.MissingHostKeyPolicy]: + """Select the missing host key policy for a host which is connected to directly. + + :param hostname: Host to connect to, without a proxy or a jump host. + :type hostname: str + :return: Configured policy; for the local host a policy, which accepts the keys of this machine first. + :rtype: paramiko.MissingHostKeyPolicy | type[paramiko.MissingHostKeyPolicy] + + The local host is normally absent from `known_hosts`, so its own keys are the reference. A key of another + server (any local address can be a container, a tunnel or another server) goes to the configured policy as + before, so nothing which worked fails because of it. Without readable keys the configured policy decides. + The own key is accepted without asking the configured policy, so that policy does not see it. + """ + try: + keys = _ssh_helpers.local_host_keys() if _ssh_helpers.is_local_host(hostname) else () + except OSError: # the answer only silences a warning, so a failure of the lookup must not fail the connection + keys = () + if keys: + return _ssh_helpers.LocalHostKeyPolicy(keys, fallback=self._missing_host_key_policy) + return self._missing_host_key_policy + def __get_client(self) -> paramiko.SSHClient: """Connect using connection chain information. @@ -839,7 +862,9 @@ def __connect_chain(self, opened: list[paramiko.SSHClient]) -> paramiko.SSHClien last_ssh_client: paramiko.SSHClient = paramiko.SSHClient() opened.append(last_ssh_client) last_ssh_client.load_system_host_keys() - last_ssh_client.set_missing_host_key_policy(self._missing_host_key_policy) + last_ssh_client.set_missing_host_key_policy( + self._missing_host_key_policy if config.proxycommand else self.__host_key_policy(config.hostname) + ) auth.connect( last_ssh_client, diff --git a/exec_helpers/_ssh_helpers.py b/exec_helpers/_ssh_helpers.py index 8038be8..a543c20 100644 --- a/exec_helpers/_ssh_helpers.py +++ b/exec_helpers/_ssh_helpers.py @@ -3,16 +3,23 @@ from __future__ import annotations import functools +import ipaddress import pathlib +import socket import typing from collections.abc import Collection +from collections.abc import Iterable import paramiko +import psutil if typing.TYPE_CHECKING: # noinspection PyPackageRequirements import logwrap +LOCAL_HOST_KEYS_PATTERN = "ssh_host_*_key.pub" +LOCAL_HOST_KEYS_DIR = pathlib.Path("/etc/ssh") + SSHConfigDictLikeT = dict[str, str | int | bool | Collection[str]] SSHConfigsDictT = dict[str, SSHConfigDictLikeT] @@ -44,6 +51,94 @@ def _parse_ssh_config_file(file_path: pathlib.Path) -> paramiko.SSHConfig | None return config +def is_local_host(hostname: str) -> bool: + """Check if the hostname points to the machine we are running on. + + :param hostname: Host name or IP address from the connection parameters. + :type hostname: str + :return: Loopback address, `localhost`, own host name or an address of a local interface. + :rtype: bool + + No name resolution is done: a name is local only if it is the own host name or its short form. + A link-local address without a scope id is ambiguous (any link has such addresses), so it is not local. + """ + name = hostname.strip("[]").lower() + try: + address = ipaddress.ip_address(name.partition("%")[0]) + except ValueError: + own_name = socket.gethostname().lower() + return name in {"localhost", own_name, own_name.partition(".")[0]} + + if address.is_loopback: + return True + if address.is_link_local and "%" not in name: + return False + try: + return any( + address == ipaddress.ip_address(addr.address.partition("%")[0]) + for addrs in psutil.net_if_addrs().values() + for addr in addrs + if addr.family in {socket.AF_INET, socket.AF_INET6} + ) + except (OSError, ValueError): # the answer only silences a warning: no verdict means "not local" + return False + + +def local_host_keys() -> tuple[paramiko.PKey, ...]: + """Read the public host keys of the SSH server of this machine. + + :return: Parsed keys, empty if there are none or they are not readable. + :rtype: tuple[paramiko.PKey, ...] + + The files are read on every call: a key rotation must not leave stale keys, and the read costs less than + the connection itself. + """ + keys: list[paramiko.PKey] = [] + for key_file in sorted(LOCAL_HOST_KEYS_DIR.glob(LOCAL_HOST_KEYS_PATTERN)): + try: + # known_hosts line format: " " + entry = paramiko.hostkeys.HostKeyEntry.from_line(f"local {key_file.read_text(encoding='ascii')}") + except (OSError, ValueError, paramiko.SSHException, paramiko.hostkeys.InvalidHostKey): + continue + if entry is not None and entry.key is not None: + keys.append(entry.key) + return tuple(keys) + + +class LocalHostKeyPolicy(paramiko.MissingHostKeyPolicy): + """Missing host key policy for the local host: accept the host keys of this machine, ask the fallback otherwise. + + :param keys: Public host keys of the SSH server of this machine. + :type keys: collections.abc.Iterable[paramiko.PKey] + :param fallback: Policy for any other key, so a server which is not this machine is treated as it was before. + :type fallback: paramiko.MissingHostKeyPolicy | type[paramiko.MissingHostKeyPolicy] + """ + + __slots__ = ("__fallback", "__keys") + + def __init__( + self, + keys: Iterable[paramiko.PKey], + fallback: paramiko.MissingHostKeyPolicy | type[paramiko.MissingHostKeyPolicy], + ) -> None: + """Remember the keys and the fallback policy.""" + self.__keys = frozenset(key.asbytes() for key in keys) + self.__fallback = fallback() if isinstance(fallback, type) else fallback + + def missing_host_key(self, client: paramiko.SSHClient, hostname: str, key: paramiko.PKey) -> None: + """Accept the key of this machine, ask the fallback policy about any other key. + + :param client: SSH client (low level). + :type client: paramiko.SSHClient + :param hostname: Name of the host in `known_hosts` format. + :type hostname: str + :param key: Key offered by the server. + :type key: paramiko.PKey + """ + if key.asbytes() not in self.__keys: + self.__fallback.missing_host_key(client, hostname, key) + + class SSHConfig: """Parsed SSH Config for creation connection.""" diff --git a/test/conftest.py b/test/conftest.py index 0519783..8c40b50 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -21,6 +21,12 @@ import pytest +@pytest.fixture(autouse=True) +def no_real_local_host_keys(mocker): + """Do not allow real host SSH server keys usage.""" + mocker.patch("exec_helpers._ssh_helpers.local_host_keys", return_value=()) + + @pytest.fixture def no_real_ssh_config(mocker): """Do not allow real host ssh config usage.""" diff --git a/test/test_ssh_client_init_basic.py b/test/test_ssh_client_init_basic.py index cfbe0dc..f8bcdba 100644 --- a/test/test_ssh_client_init_basic.py +++ b/test/test_ssh_client_init_basic.py @@ -14,6 +14,8 @@ from __future__ import annotations +import socket +import warnings from unittest import mock import paramiko @@ -46,6 +48,7 @@ def __iter__(self): yield self.__src.pop(0) +REAL_LOCAL_HOST_KEYS = exec_helpers._ssh_helpers.local_host_keys # the autouse fixture replaces the module attribute host = "127.0.0.1" @@ -157,6 +160,131 @@ def test_init_base( assert ssh.ssh_config[host].hostname == host +@pytest.mark.parametrize( + ("hostname", "expected"), + ( + ("127.0.0.1", True), + ("127.1.2.3", True), + ("::1", True), + ("[::1]", True), + ("localhost", True), + ("LOCALHOST", True), + ("192.0.2.1", False), + ("remote.example.com", False), + ), +) +def test_is_local_host(hostname, expected): + """Local host detection does not depend on name resolution.""" + assert exec_helpers._ssh_helpers.is_local_host(hostname) is expected + + +def test_is_local_host_own_names_and_interfaces(mocker): + """Own host name, its short form and own interface addresses are local.""" + mocker.patch("socket.gethostname", return_value="Node001.cluster.local") + mocker.patch( + "psutil.net_if_addrs", + return_value={ + "eth0": [ + mock.Mock(family=socket.AF_INET, address="192.0.2.10"), + mock.Mock(family=socket.AF_INET6, address="2001:db8::10%eth0"), + mock.Mock(family=socket.AF_INET6, address="fe80::10%eth0"), + mock.Mock(family=socket.AF_PACKET, address="aa:bb:cc:dd:ee:ff"), + ] + }, + ) + + for local in ("node001", "node001.cluster.local", "192.0.2.10", "2001:db8::10", "fe80::10%eth0"): + assert exec_helpers._ssh_helpers.is_local_host(local) + for remote in ("node002", "node001.example.com", "192.0.2.11", "fe80::10"): + assert not exec_helpers._ssh_helpers.is_local_host(remote) + + +def test_is_local_host_interface_enumeration_failed(mocker): + """No verdict about interfaces means the host is not local.""" + mocker.patch("psutil.net_if_addrs", side_effect=OSError) + assert not exec_helpers._ssh_helpers.is_local_host("192.0.2.10") + + +def test_unknown_host_key_policy(paramiko_ssh_client, ssh_auth_logger): + """Warning policy stays by default and a custom policy is never replaced.""" + ssh_ = mock.call + + exec_helpers.SSHClient(host="192.0.2.1", auth=exec_helpers.SSHAuth()) + assert ssh_.set_missing_host_key_policy(paramiko.WarningPolicy) in paramiko_ssh_client.return_value.mock_calls + + paramiko_ssh_client.reset_mock() + exec_helpers.SSHClient(host=host, auth=exec_helpers.SSHAuth(), missing_host_key_policy=paramiko.RejectPolicy) + assert ssh_.set_missing_host_key_policy(paramiko.RejectPolicy) in paramiko_ssh_client.return_value.mock_calls + + +@pytest.mark.parametrize("local_host", ("127.0.0.1", "127.0.0.2", "127.0.0.254", "localhost")) +@pytest.mark.parametrize("port", (22, 2222)) +def test_local_host_policy_selected(mocker, paramiko_ssh_client, ssh_auth_logger, local_host, port): + """Any local address and port gets the policy, which accepts the keys of this machine.""" + mocker.patch("exec_helpers._ssh_helpers.local_host_keys", return_value=(mock.Mock(),)) + + exec_helpers.SSHClient(host=local_host, port=port, auth=exec_helpers.SSHAuth()) + + (policy,) = paramiko_ssh_client.return_value.set_missing_host_key_policy.call_args.args + assert isinstance(policy, exec_helpers._ssh_helpers.LocalHostKeyPolicy) + + +@pytest.mark.parametrize("keys", ((), (mock.Mock(),))) +def test_local_host_policy_not_selected(mocker, paramiko_ssh_client, ssh_auth_logger, keys): + """A remote host and a local host without readable keys keep the configured policy.""" + mocker.patch("exec_helpers._ssh_helpers.local_host_keys", return_value=keys) + + for name in ("192.0.2.1", "remote.example.com") + (() if keys else (host,)): + paramiko_ssh_client.reset_mock() + exec_helpers.SSHClient(host=name, auth=exec_helpers.SSHAuth(), missing_host_key_policy=paramiko.RejectPolicy) + assert ( + mock.call.set_missing_host_key_policy(paramiko.RejectPolicy) in paramiko_ssh_client.return_value.mock_calls + ) + + +def test_local_host_keys_file_reading(mocker, tmp_path): + """Public keys of the SSH server are read, unreadable and broken files are skipped.""" + keys = [paramiko.RSAKey.generate(1024), paramiko.RSAKey.generate(1024)] + for number, key in enumerate(keys): + (tmp_path / f"ssh_host_rsa{number}_key.pub").write_text(f"{key.get_name()} {key.get_base64()} comment\n") + (tmp_path / "ssh_host_broken_key.pub").write_text("not a key\n") + (tmp_path / "ssh_host_truncated_key.pub").write_text("ssh-ed25519 AAAA\n") + (tmp_path / "ssh_host_directory_key.pub").mkdir() + mocker.patch("exec_helpers._ssh_helpers.LOCAL_HOST_KEYS_DIR", tmp_path) + + assert [key.get_base64() for key in REAL_LOCAL_HOST_KEYS()] == [key.get_base64() for key in keys] + + +@pytest.mark.parametrize("through", ("proxycommand", "proxyjump", "sock")) +def test_unknown_host_key_policy_behind_proxy(mocker, paramiko_ssh_client, ssh_auth_logger, through): + """The local host name behind a proxy or jump host is resolved elsewhere, so the warning stays.""" + mocker.patch("exec_helpers._ssh_helpers.local_host_keys", return_value=(mock.Mock(),)) + ssh_ = mock.call + kwargs = {} + config = {host: {"hostname": host}} + if through == "proxycommand": + config = {host: {"hostname": host, "proxycommand": "nc far-host 22"}} + elif through == "proxyjump": + config = { + "jump": {"hostname": "192.0.2.1"}, + host: {"hostname": host, "proxyjump": "jump"}, + } + kwargs["ssh_auth_map"] = {"192.0.2.1": exec_helpers.SSHAuth(), host: exec_helpers.SSHAuth()} + else: + kwargs["sock"] = mock.Mock() + + if through != "sock": + kwargs["ssh_config"] = config + exec_helpers.SSHClient(host=host, auth=exec_helpers.SSHAuth(), **kwargs) + + calls = paramiko_ssh_client.return_value.mock_calls + assert ssh_.set_missing_host_key_policy(paramiko.WarningPolicy) in calls + assert not any( + isinstance(call.args[0], exec_helpers._ssh_helpers.LocalHostKeyPolicy) + for call in paramiko_ssh_client.return_value.set_missing_host_key_policy.call_args_list + ) + + @pytest.mark.parametrize("through", ("direct", "sock", "proxyjump", "proxy_to")) def test_connect_timeout_every_step(paramiko_ssh_client, ssh_auth_logger, through): """The connect timeout is applied to every connection step.""" @@ -187,6 +315,34 @@ def test_connect_timeout_not_positive(paramiko_ssh_client, value): exec_helpers.SSHClient(host=host, auth=exec_helpers.SSHAuth(), connect_timeout=value) +def test_local_host_key_policy(): + """The key of this machine is accepted, any other key goes to the fallback policy as it was before.""" + key = paramiko.RSAKey.generate(1024) + other = paramiko.RSAKey.generate(1024) + client = mock.Mock() + fallback = mock.Mock() + + policy = exec_helpers._ssh_helpers.LocalHostKeyPolicy((key,), fallback=fallback) + policy.missing_host_key(client, host, key) + fallback.missing_host_key.assert_not_called() + + policy.missing_host_key(client, host, other) + fallback.missing_host_key.assert_called_once_with(client, host, other) + + +def test_local_host_key_policy_fallback_class(): + """A fallback policy class is instantiated and still decides about the keys of other servers.""" + key = paramiko.RSAKey.generate(1024) + other = paramiko.RSAKey.generate(1024) + policy = exec_helpers._ssh_helpers.LocalHostKeyPolicy((key,), fallback=paramiko.RejectPolicy) + + client = mock.Mock() + + policy.missing_host_key(client, host, key) + with pytest.raises(paramiko.SSHException): + policy.missing_host_key(client, host, other) + + def test_failed_chain_closes_every_reached_host(paramiko_ssh_client, ssh_auth_logger): """A failure on a jump host chain closes the clients of all the hosts created so far.""" first = mock.Mock() @@ -220,3 +376,30 @@ def test_connect_retries(paramiko_ssh_client, ssh_auth_logger, supplied_sock, at exec_helpers.SSHClient(host=host, auth=exec_helpers.SSHAuth(), **kwargs) assert paramiko_ssh_client.return_value.connect.call_count == attempts + + +def test_local_host_key_policy_warns_about_other_keys(): + """The default policy still warns about a key which is not a key of this machine, and not about the own one.""" + key = paramiko.RSAKey.generate(1024) + other = paramiko.RSAKey.generate(1024) + client = mock.Mock() + policy = exec_helpers._ssh_helpers.LocalHostKeyPolicy((key,), fallback=paramiko.WarningPolicy) + + with warnings.catch_warnings(): + warnings.simplefilter("error") + policy.missing_host_key(client, host, key) + + with pytest.warns(UserWarning, match="Unknown"): + policy.missing_host_key(client, host, other) + + +@pytest.mark.parametrize("failing", ("is_local_host", "local_host_keys")) +def test_local_host_lookup_failure_is_not_a_connection_failure(mocker, paramiko_ssh_client, ssh_auth_logger, failing): + """A failed lookup of the local host keys leaves the configured policy, the connection is made as before.""" + mocker.patch("exec_helpers._ssh_helpers.is_local_host", return_value=True) + mocker.patch("exec_helpers._ssh_helpers.local_host_keys", return_value=(mock.Mock(),)) + mocker.patch(f"exec_helpers._ssh_helpers.{failing}", side_effect=OSError) + + exec_helpers.SSHClient(host=host, auth=exec_helpers.SSHAuth()) + + assert mock.call.set_missing_host_key_policy(paramiko.WarningPolicy) in paramiko_ssh_client.return_value.mock_calls From c229b04f78e02e7f969fb4540322bfdf338654bb Mon Sep 17 00:00:00 2001 From: Aleksei Stepanov Date: Thu, 29 Oct 2026 16:24:29 +0100 Subject: [PATCH 8/8] Check the project with pyrefly in tox and CI pyrefly runs next to mypy: it reports different errors, and the team uses its language server, so a finding of it shows up in their editors. - [tool.pyrefly] checks the package and the tests for the project's minimum Python version; the version file written by setuptools-scm is not required. - Add the pyrefly dependency group and a pyrefly tox environment; the stub packages move to a types group shared with mypy. Add a PyRefly CI job that the Test job waits for. - Make the code and the tests pass it, without behaviour changes: format the ExecResult error messages with format_map; name the SSHAuthMapping item method parameters as UserDict does; drop the redundant explicit context manager bases of the execute contexts; build the execute results with _make, as their field types are narrowed by properties; mark the narrowing properties and the optional imports with pyrefly: ignore[...] and the reason; annotate dictionaries in the tests. --- .github/workflows/pythonpackage.yml | 25 +++++++++++- exec_helpers/_ssh_base.py | 15 ++++---- exec_helpers/async_api/subprocess.py | 13 ++----- exec_helpers/exec_result.py | 12 +++--- exec_helpers/ssh_auth.py | 44 ++++++++++----------- exec_helpers/subprocess.py | 16 ++++---- pyproject.toml | 47 ++++++++++++++++------- test/async_api/test_subprocess_special.py | 2 +- test/test_context_repr.py | 5 ++- test/test_exec_result.py | 12 +++--- test/test_ssh_client_execute.py | 9 +---- test/test_ssh_client_execute_special.py | 9 +---- test/test_ssh_client_init_basic.py | 9 +++-- test/test_ssh_client_init_special.py | 5 ++- test/test_sshauth.py | 2 +- 15 files changed, 128 insertions(+), 97 deletions(-) diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 41a14f0..2916e70 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -95,11 +95,34 @@ jobs: run: | mypy -p exec_helpers + PyRefly: + permissions: + contents: read + pull-requests: read + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - name: Set up Python + uses: actions/setup-python@v7 + with: + python-version: '3.x' + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install --upgrade --group pyrefly + pip install --upgrade -r CI_REQUIREMENTS.txt + - name: Install develop + run: | + pip install -e . + - name: Lint with PyRefly + run: | + pyrefly check + Test: permissions: contents: read pull-requests: read - needs: [PEP8, Formatting, PyLint, MyPy] + needs: [PEP8, Formatting, PyLint, MyPy, PyRefly] runs-on: ${{ matrix.os }} strategy: max-parallel: 6 diff --git a/exec_helpers/_ssh_base.py b/exec_helpers/_ssh_base.py index 1d54ecc..67adbc0 100644 --- a/exec_helpers/_ssh_base.py +++ b/exec_helpers/_ssh_base.py @@ -94,6 +94,7 @@ class SshExecuteAsyncResult(api.ExecuteAsyncResult): __slots__ = () @property + # pyrefly: ignore[bad-override] # the property narrows the NamedTuple field type def interface(self) -> paramiko.Channel: """Override original NamedTuple with proper typing. @@ -103,6 +104,7 @@ def interface(self) -> paramiko.Channel: return super().interface # type: ignore[no-any-return] @property + # pyrefly: ignore[bad-override] # the property narrows the NamedTuple field type def stdin(self) -> paramiko.channel.ChannelStdinFile: """Override original NamedTuple with proper typing. @@ -117,6 +119,7 @@ def stdin(self) -> paramiko.channel.ChannelStdinFile: return super().stdin # type: ignore[return-value] @property + # pyrefly: ignore[bad-override] # the property narrows the NamedTuple field type def stderr(self) -> paramiko.channel.ChannelStderrFile | None: """Override original NamedTuple with proper typing. @@ -126,6 +129,7 @@ def stderr(self) -> paramiko.channel.ChannelStderrFile | None: return super().stderr @property + # pyrefly: ignore[bad-override] # the property narrows the NamedTuple field type def stdout(self) -> paramiko.channel.ChannelFile | None: """Override original NamedTuple with proper typing. @@ -135,7 +139,7 @@ def stdout(self) -> paramiko.channel.ChannelFile | None: return super().stdout -class _SSHExecuteContext(api.ExecuteContext, contextlib.AbstractContextManager[SshExecuteAsyncResult]): +class _SSHExecuteContext(api.ExecuteContext): """SSH Execute context.""" __slots__ = ( @@ -333,13 +337,8 @@ def __enter__(self) -> SshExecuteAsyncResult: res_stdout = None # noinspection PyArgumentList - return SshExecuteAsyncResult( - interface=chan, - stdin=_stdin, - stderr=stderr, - stdout=res_stdout, - started=started, - ) + # The fields are typed by properties of the subclass, which type checkers do not accept as constructor arguments + return SshExecuteAsyncResult._make((chan, _stdin, stderr, res_stdout, started)) def __exit__( self, diff --git a/exec_helpers/async_api/subprocess.py b/exec_helpers/async_api/subprocess.py index 878e9ff..facc194 100644 --- a/exec_helpers/async_api/subprocess.py +++ b/exec_helpers/async_api/subprocess.py @@ -108,9 +108,7 @@ def stdout(self) -> AsyncIterable[bytes] | None: # type: ignore[override] return super().stdout # type: ignore[return-value] -class _SubprocessExecuteContext( - api.ExecuteContext, contextlib.AbstractAsyncContextManager[SubprocessExecuteAsyncResult] -): +class _SubprocessExecuteContext(api.ExecuteContext): """Subprocess Execute context.""" __slots__ = ("__cwd", "__env", "__process", "__shell") @@ -231,13 +229,8 @@ async def __aenter__(self) -> SubprocessExecuteAsyncResult: raise # noinspection PyArgumentList - return SubprocessExecuteAsyncResult( - interface=process, - stdin=None, - stderr=process.stderr, - stdout=process.stdout, - started=started, - ) + # The fields are typed by properties of the subclass, which type checkers do not accept as constructor arguments + return SubprocessExecuteAsyncResult._make((process, None, process.stderr, process.stdout, started)) async def __aexit__( self, diff --git a/exec_helpers/exec_result.py b/exec_helpers/exec_result.py index b0b8fbd..ddac9c8 100644 --- a/exec_helpers/exec_result.py +++ b/exec_helpers/exec_result.py @@ -89,11 +89,11 @@ def wrapper(self: ExecResult) -> _T: return method(self) except Exception as exc: tmpl: str = f"{{self.cmd}} stdout is not valid {fmt}:\n{{stdout!r}}\n" - LOGGER.exception(tmpl.format(self=self, stdout=self.stdout_str)) + LOGGER.exception(tmpl.format_map({"self": self, "stdout": self.stdout_str})) - raise exceptions.DeserializeValueError(tmpl.format(self=self, stdout=self.stdout_brief)).with_traceback( - exc.__traceback__ - ) from exc + raise exceptions.DeserializeValueError( + tmpl.format_map({"self": self, "stdout": self.stdout_brief}) + ).with_traceback(exc.__traceback__) from exc return wrapper @@ -666,7 +666,7 @@ def stdout_yaml(self) -> typing.Any: if yaml.__with_libyaml__: # pragma: no cover return yaml.load(self.stdout_str, Loader=yaml.CSafeLoader) # nosec # Safe return yaml.safe_load(self.stdout_str) # pragma: no cover - return ruamel_yaml.YAML(typ="safe").load(self.stdout_str) # nosec # Safe + return ruamel_yaml.YAML(typ="safe").load(self.stdout_str) # nosec # pyrefly: ignore[missing-attribute] @property @_handle_deserialize("xml") @@ -694,7 +694,7 @@ def stdout_lxml(self) -> lxml.etree.Element: .. note:: Can be insecure. """ with self.stdout_lock: - return lxml.etree.fromstring(b"".join(self.stdout)) # nosec[B314] + return lxml.etree.fromstring(b"".join(self.stdout)) # nosec[B314] # pyrefly: ignore[missing-attribute] def __dir__(self) -> list[str]: """Override dir for IDE and as source for getitem checks. diff --git a/exec_helpers/ssh_auth.py b/exec_helpers/ssh_auth.py index d7b3e06..d405fc7 100644 --- a/exec_helpers/ssh_auth.py +++ b/exec_helpers/ssh_auth.py @@ -379,33 +379,33 @@ def __init__( else: # pragma: no cover raise TypeError(f"Auth object have incorrect type: (got {auth!r})") - def __setitem__(self, hostname: str, auth: SSHAuth) -> None: + def __setitem__(self, key: str, item: SSHAuth) -> None: """Dict-like access. - :param hostname: Key - hostname. - :type hostname: str - :param auth: value - SSHAuth object. - :type auth: SSHAuth + :param key: Key - hostname. + :type key: str + :param item: value - SSHAuth object. + :type item: SSHAuth :raises TypeError: Key is not string or value is not SSHAuth. """ - if not isinstance(hostname, str): # pragma: no cover - raise TypeError(f"Hostname should be string only! Got: {hostname!r}") - if not isinstance(auth, SSHAuth): # pragma: no cover - raise TypeError(f"Value {auth!r} is not SSHAuth object!") - super().__setitem__(hostname.lower(), auth) + if not isinstance(key, str): # pragma: no cover + raise TypeError(f"Hostname should be string only! Got: {key!r}") + if not isinstance(item, SSHAuth): # pragma: no cover + raise TypeError(f"Value {item!r} is not SSHAuth object!") + super().__setitem__(key.lower(), item) - def __getitem__(self, hostname: str) -> SSHAuth: + def __getitem__(self, key: str) -> SSHAuth: """Dict-like access. - :param hostname: Key - hostname. - :type hostname: str + :param key: Key - hostname. + :type key: str :return: Associated SSHAuth object. :rtype: SSHAuth :raises TypeError: Key is not a string. """ - if not isinstance(hostname, str): # pragma: no cover - raise TypeError(f"Hostname should be string only! Got: {hostname!r}") - return super().__getitem__(hostname.lower()) + if not isinstance(key, str): # pragma: no cover + raise TypeError(f"Hostname should be string only! Got: {key!r}") + return super().__getitem__(key.lower()) @typing.overload def get_with_alt_hostname( @@ -452,13 +452,13 @@ def get_with_alt_hostname( return self[host] return default - def __delitem__(self, hostname: str) -> None: + def __delitem__(self, key: str) -> None: """Dict-like access. - :param hostname: Key - hostname. - :type hostname: str + :param key: Key - hostname. + :type key: str :raises TypeError: Key is not a string. """ - if not isinstance(hostname, str): # pragma: no cover - raise TypeError(f"Hostname should be string only! Got: {hostname!r}") - super().__delitem__(hostname.lower()) + if not isinstance(key, str): # pragma: no cover + raise TypeError(f"Hostname should be string only! Got: {key!r}") + super().__delitem__(key.lower()) diff --git a/exec_helpers/subprocess.py b/exec_helpers/subprocess.py index 492609e..dfd8001 100644 --- a/exec_helpers/subprocess.py +++ b/exec_helpers/subprocess.py @@ -19,7 +19,6 @@ from __future__ import annotations import concurrent.futures -import contextlib import copy import datetime as dt import errno @@ -66,6 +65,7 @@ class SubprocessExecuteAsyncResult(api.ExecuteAsyncResult): __slots__ = () @property + # pyrefly: ignore[bad-override] # the property narrows the NamedTuple field type def interface(self) -> subprocess.Popen[bytes]: """Override original NamedTuple with proper typing. @@ -75,6 +75,7 @@ def interface(self) -> subprocess.Popen[bytes]: return super().interface # type: ignore[no-any-return] @property + # pyrefly: ignore[bad-override] # the property narrows the NamedTuple field type def stdin(self) -> typing.IO[bytes] | None: """Override original NamedTuple with proper typing. @@ -89,6 +90,7 @@ def stdin(self) -> typing.IO[bytes] | None: return super().stdin @property + # pyrefly: ignore[bad-override] # the property narrows the NamedTuple field type def stderr(self) -> typing.IO[bytes] | None: """Override original NamedTuple with proper typing. @@ -98,6 +100,7 @@ def stderr(self) -> typing.IO[bytes] | None: return super().stderr @property + # pyrefly: ignore[bad-override] # the property narrows the NamedTuple field type def stdout(self) -> typing.IO[bytes] | None: """Override original NamedTuple with proper typing. @@ -107,7 +110,7 @@ def stdout(self) -> typing.IO[bytes] | None: return super().stdout -class _SubprocessExecuteContext(api.ExecuteContext, contextlib.AbstractContextManager[SubprocessExecuteAsyncResult]): +class _SubprocessExecuteContext(api.ExecuteContext): """Subprocess Execute context.""" __slots__ = ("__cwd", "__env", "__process", "__shell") @@ -232,13 +235,8 @@ def __enter__(self) -> SubprocessExecuteAsyncResult: raise # noinspection PyArgumentList - return SubprocessExecuteAsyncResult( - interface=process, - stdin=None, - stderr=process.stderr, - stdout=process.stdout, - started=started, - ) + # The fields are typed by properties of the subclass, which type checkers do not accept as constructor arguments + return SubprocessExecuteAsyncResult._make((process, None, process.stderr, process.stdout, started)) def __exit__( self, diff --git a/pyproject.toml b/pyproject.toml index 13fcf40..6cd3026 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -51,6 +51,7 @@ dev = [ { include-group = "prek" }, { include-group = "pylint" }, { include-group = "pyproject-fmt" }, + { include-group = "pyrefly" }, { include-group = "pytest" }, { include-group = "ruff" }, { include-group = "tox" }, @@ -71,9 +72,7 @@ doc8 = [ mypy = [ "mypy[faster-cache,reports]>=2.4.1", "rich>=15.0.0", - "types-paramiko>=5.0.0", - "types-psutil>=7.2.2", - "types-pyyaml>=6.0.12", + { include-group = "types" }, ] prek = [ "prek>=0.5.7", @@ -84,6 +83,11 @@ pylint = [ pyproject-fmt = [ "pyproject-fmt>=2.30.3", ] +pyrefly = [ + "pyrefly>=1.4.0", + { include-group = "pytest" }, + { include-group = "types" }, +] pytest = [ "asynctest>=0.13", "coverage[toml]>=7.14", @@ -102,6 +106,11 @@ ruff = [ tox = [ "tox>=4.53.1", ] +types = [ + "types-paramiko>=5.0.0", + "types-psutil>=7.2.2", + "types-pyyaml>=6.0.12", +] [tool.setuptools] zip-safe = false @@ -276,6 +285,12 @@ show_error_codes = true module = [ "lxml.*" ] ignore_missing_imports = true +[tool.pyrefly] +python-version = "3.11" +project-includes = [ "exec_helpers", "test" ] +# Written by setuptools-scm at build time +ignore-missing-imports = [ "exec_helpers._version" ] + [tool.pytest] [tool.pytest.ini_options] minversion = "6.0" @@ -335,21 +350,22 @@ env_list = [ { prefix = "py3", start = 11, stop = 15 }, { prefix = "pypy3", start = 11, stop = 15 }, "pylint", + "pyrefly", "readme", "ruff", ] skip_missing_interpreters = true [tool.tox.env] -py311.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] -py312.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] -py313.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] -py314.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] -py315.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] -pypy311.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] -pypy312.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] -pypy313.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] -pypy314.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] -pypy315.depends = [ "bandit", "mypy", "pylint", "ruff", "ruff-format" ] +py311.depends = [ "bandit", "mypy", "pylint", "pyrefly", "ruff", "ruff-format" ] +py312.depends = [ "bandit", "mypy", "pylint", "pyrefly", "ruff", "ruff-format" ] +py313.depends = [ "bandit", "mypy", "pylint", "pyrefly", "ruff", "ruff-format" ] +py314.depends = [ "bandit", "mypy", "pylint", "pyrefly", "ruff", "ruff-format" ] +py315.depends = [ "bandit", "mypy", "pylint", "pyrefly", "ruff", "ruff-format" ] +pypy311.depends = [ "bandit", "mypy", "pylint", "pyrefly", "ruff", "ruff-format" ] +pypy312.depends = [ "bandit", "mypy", "pylint", "pyrefly", "ruff", "ruff-format" ] +pypy313.depends = [ "bandit", "mypy", "pylint", "pyrefly", "ruff", "ruff-format" ] +pypy314.depends = [ "bandit", "mypy", "pylint", "pyrefly", "ruff", "ruff-format" ] +pypy315.depends = [ "bandit", "mypy", "pylint", "pyrefly", "ruff", "ruff-format" ] [tool.tox.env.bandit] dependency_groups = [ "bandit" ] commands = [ [ "bandit", "-r", "exec_helpers" ] ] @@ -376,6 +392,11 @@ deps = [ "-r {toxinidir}/CI_REQUIREMENTS.txt" ] dependency_groups = [ "pylint" ] commands = [ [ "pylint", "exec_helpers" ] ] depends = [ "bandit", "ruff", "ruff-format" ] +[tool.tox.env.pyrefly] +deps = [ "-r {toxinidir}/CI_REQUIREMENTS.txt" ] +dependency_groups = [ "pyrefly" ] +commands = [ [ "pyrefly", "check" ] ] +depends = [ "bandit", "ruff", "ruff-format" ] [tool.tox.env.readme] package = "skip" dependency_groups = [ "readme" ] diff --git a/test/async_api/test_subprocess_special.py b/test/async_api/test_subprocess_special.py index cb6e36c..04e7084 100644 --- a/test/async_api/test_subprocess_special.py +++ b/test/async_api/test_subprocess_special.py @@ -94,7 +94,7 @@ class MockParameters(typing.NamedTuple): write: Exception | None = None stdin_close: Exception | None = None - ec: typing.Sequence[int | Exception] = (0,) + ec: typing.Sequence[int | Exception | None] = (0,) configs = { diff --git a/test/test_context_repr.py b/test/test_context_repr.py index 7ba8076..4e23e6b 100644 --- a/test/test_context_repr.py +++ b/test/test_context_repr.py @@ -16,6 +16,8 @@ from __future__ import annotations +import typing + import exec_helpers from exec_helpers import async_api @@ -72,7 +74,8 @@ def test_context_repr_no_stdin(): def test_context_repr_mixed_env_keys(): """Names of an environment with mixed key types are listed without an error.""" - ctx = exec_helpers.Subprocess().open_execute_context("true", env={b"A": SECRET, "B": SECRET}) + env: dict[typing.Any, str] = {b"A": SECRET, "B": SECRET} + ctx = exec_helpers.Subprocess().open_execute_context("true", env=env) text = repr(ctx) assert SECRET not in text assert "env=" in text diff --git a/test/test_exec_result.py b/test/test_exec_result.py index 20331b6..f6a4ef4 100644 --- a/test/test_exec_result.py +++ b/test/test_exec_result.py @@ -134,7 +134,7 @@ def test_setters(self): self.assertEqual(result.stderr, result["stderr"]) with self.assertRaises(TypeError): - result.exit_code = "code" + result.exit_code = "code" # pyrefly: ignore[bad-argument-type] # a wrong type is the point result.exit_code = 0 self.assertEqual(result.exit_code, 0) @@ -234,7 +234,7 @@ def test_started(self): """Test timestamp.""" started = dt.datetime.now(tz=dt.UTC) result = exec_helpers.ExecResult(cmd, exit_code=0, started=started) - spent = (result.timestamp - started).seconds + spent = (result.timestamp - started).seconds # pyrefly: ignore[unsupported-operation] # set by exit_code self.assertIs(result.started, started) self.assertEqual( str(result), @@ -285,9 +285,9 @@ def test_indexed_lines_access(self): self.assertEqual(result.stdout_lines[0, ..., 2], "line0\n...\nline2") self.assertEqual(result.stdout_lines[:1, ..., 2], "line0\n...\nline2") with self.assertRaises(TypeError): - _ = result.stdout_lines["aaa"] + _ = result.stdout_lines["aaa"] # pyrefly: ignore[bad-index] # a wrong index is the point with self.assertRaises(TypeError): - _ = result.stdout_lines[1, "aaa"] + _ = result.stdout_lines[1, "aaa"] # pyrefly: ignore[bad-index] # a wrong index is the point def test_stdout_xml(self): """Test xml etree decode.""" @@ -302,7 +302,9 @@ def test_stdout_xml(self): def test_stdout_lxml(self): """Test lxml etree decode.""" result = exec_helpers.ExecResult("test", stdout=[b"\n", b"123\n"]) + # pyrefly: ignore[missing-attribute] # the test is skipped without lxml expect = lxml.etree.fromstring(b"\n123\n") + # pyrefly: ignore[missing-attribute] # the test is skipped without lxml self.assertEqual(lxml.etree.tostring(expect), lxml.etree.tostring(result.stdout_lxml)) @unittest.skipUnless(yaml is not None, "PyYAML parser should be installed") @@ -316,7 +318,7 @@ def test_stdout_yaml_pyyaml(self): def test_pretty_repr(self): """Test repr with logwrap.""" result = exec_helpers.ExecResult("test", stdout=[b"{test: data}"], stderr=[b"{test: stderr}"]) - pretty_repr = logwrap.pretty_repr(result) + pretty_repr = logwrap.pretty_repr(result) # pyrefly: ignore[missing-attribute] # skipped without logwrap self.assertEqual( f"ExecResult(\n" f" cmd='test',\n" diff --git a/test/test_ssh_client_execute.py b/test/test_ssh_client_execute.py index af115ce..d63c589 100644 --- a/test/test_ssh_client_execute.py +++ b/test/test_ssh_client_execute.py @@ -282,13 +282,8 @@ def get_patched_execute_async_retval( status_event.attach_mock(mock.Mock(), "wait") chan.attach_mock(status_event, "status_event") chan.configure_mock(exit_status=exit_code) - return SshExecuteAsyncResult( - interface=chan, - stdin=mock.Mock, - stdout=stdout_part, - stderr=stderr_part, - started=dt.datetime.now(tz=dt.UTC), - ) + # the fields are typed by properties of the class, so they are passed in the order of the fields + return SshExecuteAsyncResult._make((chan, mock.Mock, stderr_part, stdout_part, dt.datetime.now(tz=dt.UTC))) return mocker.patch( "exec_helpers.ssh.SSHClient._execute_async", diff --git a/test/test_ssh_client_execute_special.py b/test/test_ssh_client_execute_special.py index 4bca25a..0dbde80 100644 --- a/test/test_ssh_client_execute_special.py +++ b/test/test_ssh_client_execute_special.py @@ -253,13 +253,8 @@ def get_patched_execute_async_retval() -> SshExecuteAsyncResult: status_event.attach_mock(mock.Mock(), "wait") chan.attach_mock(status_event, "status_event") chan.configure_mock(exit_status=exit_code) - return SshExecuteAsyncResult( - interface=chan, - stdin=mock.Mock, - stdout=stdout_part, - stderr=stderr_part, - started=dt.datetime.now(tz=dt.UTC), - ) + # the fields are typed by properties of the class, so they are passed in the order of the fields + return SshExecuteAsyncResult._make((chan, mock.Mock, stderr_part, stdout_part, dt.datetime.now(tz=dt.UTC))) execute_async = mocker.patch( "exec_helpers.ssh.SSHClient._execute_async", diff --git a/test/test_ssh_client_init_basic.py b/test/test_ssh_client_init_basic.py index f8bcdba..d02bfb5 100644 --- a/test/test_ssh_client_init_basic.py +++ b/test/test_ssh_client_init_basic.py @@ -15,6 +15,7 @@ from __future__ import annotations import socket +import typing import warnings from unittest import mock @@ -260,8 +261,8 @@ def test_unknown_host_key_policy_behind_proxy(mocker, paramiko_ssh_client, ssh_a """The local host name behind a proxy or jump host is resolved elsewhere, so the warning stays.""" mocker.patch("exec_helpers._ssh_helpers.local_host_keys", return_value=(mock.Mock(),)) ssh_ = mock.call - kwargs = {} - config = {host: {"hostname": host}} + kwargs: dict[str, typing.Any] = {} + config: dict[str, typing.Any] = {host: {"hostname": host}} if through == "proxycommand": config = {host: {"hostname": host, "proxycommand": "nc far-host 22"}} elif through == "proxyjump": @@ -288,7 +289,7 @@ def test_unknown_host_key_policy_behind_proxy(mocker, paramiko_ssh_client, ssh_a @pytest.mark.parametrize("through", ("direct", "sock", "proxyjump", "proxy_to")) def test_connect_timeout_every_step(paramiko_ssh_client, ssh_auth_logger, through): """The connect timeout is applied to every connection step.""" - kwargs = {} + kwargs: dict[str, typing.Any] = {} if through == "sock": kwargs["sock"] = mock.Mock() elif through == "proxyjump": @@ -349,7 +350,7 @@ def test_failed_chain_closes_every_reached_host(paramiko_ssh_client, ssh_auth_lo second = mock.Mock() second.connect.side_effect = TimeoutError paramiko_ssh_client.side_effect = [first, second] - config = { + config: dict[str, typing.Any] = { "jump": {"hostname": "192.0.2.1"}, host: {"hostname": host, "proxyjump": "jump"}, } diff --git a/test/test_ssh_client_init_special.py b/test/test_ssh_client_init_special.py index d2abe79..0bed7d1 100644 --- a/test/test_ssh_client_init_special.py +++ b/test/test_ssh_client_init_special.py @@ -14,6 +14,7 @@ from __future__ import annotations +import typing from unittest import mock import paramiko @@ -79,7 +80,7 @@ def test_001_require_key( pkey = private_keys[0] - kwargs_no_key = { + kwargs_no_key: dict[str, typing.Any] = { "hostname": host, "pkey": None, "port": port, @@ -135,7 +136,7 @@ def test_002_use_next_key( ssh_auth_logger.debug.assert_called_once_with(f"Main key has been updated, public key is: \n{ssh.auth.public_key}") - kwargs_no_key = { + kwargs_no_key: dict[str, typing.Any] = { "hostname": host, "pkey": None, "port": port, diff --git a/test/test_sshauth.py b/test/test_sshauth.py index 7a7e76c..19a1194 100644 --- a/test/test_sshauth.py +++ b/test/test_sshauth.py @@ -44,7 +44,7 @@ def get_internal_keys( keys: typing.Iterable[paramiko.RSAKey] | None = None, **kwargs, ): - int_keys = [] + int_keys: list[paramiko.RSAKey | None] = [] if key is not None: int_keys.append(key) if keys is not None: