This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 8b115c0be65 Verify SSH host keys by default in SSH and SFTP hooks
(#73419)
8b115c0be65 is described below
commit 8b115c0be6549f1ab57d0e4eb52bd13969a092fc
Author: Jarek Potiuk <[email protected]>
AuthorDate: Wed Oct 7 21:04:09 2026 +0200
Verify SSH host keys by default in SSH and SFTP hooks (#73419)
* Verify SSH host keys by default in SSH and SFTP hooks
The SFTP filesystem backend has always verified host keys by default,
while SSHHook, SSHHookAsync and SFTPHookAsync did not: `no_host_key_check`
defaulted to true, so any connection that did not say otherwise got
paramiko's AutoAddPolicy, or `known_hosts=None` on the asyncssh paths.
This brings the hooks in line with the filesystem backend.
This is a breaking change: a connection to a host with no entry in the
known hosts file is now refused unless verification is disabled
explicitly.
Three further changes were needed to make that default workable:
* SSHHook gains a `no_host_key_check` constructor argument. The setting
could previously only come from a Connection extra, so code building
the hook directly had no way to opt out at all -- which also made the
documented migration path unusable for those callers.
* `ignore_hostkey_verification` is now honoured as a deprecated alias for
`no_host_key_check`. Nothing read it: connections setting it were
relying on the permissive default rather than on the extra, and would
otherwise have broken with no working replacement.
* store_directory_concurrently and retrieve_directory_concurrently built
their worker hooks with only `ssh_conn_id`, discarding the rest of the
parent hook's configuration. That was invisible while the default was
permissive; the workers now inherit the host key setting.
Generated-by: Claude Opus 5
* Point the Amazon SFTP transfer tests at a connection that skips host key
checks
`SFTPToS3Operator` and `S3ToSFTPOperator` build their own `SFTPHook` from
`sftp_conn_id` and take no hook argument, so these tests could not reuse the
`SSHHook` they already configure. They rely on the bundled `ssh_default`
connection, a bare `ssh://localhost` with no extras, which host key
verification now refuses.
Overrides that conn id for the duration of each class with the same host and
`no_host_key_check=true` -- the migration step any deployment relying on the
previous default has to take.
Generated-by: Claude Opus 5
* Apply host key verification settings consistently across SSH hooks
Connections migrating through the deprecated `ignore_hostkey_verification`
alias only kept working on the synchronous hook, so a deferrable task could
submit successfully and then fail once its trigger reconnected through
`SSHHookAsync` or `SFTPHookAsync`.
A connection carrying both `host_key` and `no_host_key_check` was rejected
before the constructor argument was applied, which left no way to resolve that
combination for a connection the caller cannot edit.
The SFTP provider now passes a constructor argument that SSH provider 6.0.x
does not accept, so its minimum SSH provider version has to move with the next
release.
Generated-by: Claude Opus 5
* Keep host key settings consistent on deferred and async SSH paths
A no_host_key_check set on the hook given to a deferrable SFTPOperator
was lost at deferral, so a transfer that worked synchronously failed in
the trigger once host keys became verified by default. The async hooks
also wrote their inline known_hosts entry for the connection's host
rather than the host actually connected to, and emitted an entry
asyncssh cannot use for a bare base64 host_key -- both undermining the
host_key migration path this change recommends.
Generated-by: Claude Opus 5
* Use stable parametrize ids for SFTP async host key test
Generated-by: Claude Opus 5
* Preserve async host verification without a known hosts file
An absent default known_hosts file should still produce an untrusted-host
result, rather than a filesystem error. Keep explicit known_hosts paths
unchanged and carry the effective host-key policy into concurrent SFTP workers.
Co-authored-by: Copilot <[email protected]>
---------
Co-authored-by: Copilot <[email protected]>
---
.../unit/amazon/aws/transfers/test_s3_to_sftp.py | 12 ++
.../unit/amazon/aws/transfers/test_sftp_to_s3.py | 12 ++
providers/sftp/docs/changelog.rst | 16 ++
providers/sftp/docs/connections/sftp.rst | 2 +-
providers/sftp/pyproject.toml | 2 +-
.../sftp/src/airflow/providers/sftp/hooks/sftp.py | 38 +++-
.../src/airflow/providers/sftp/operators/sftp.py | 1 +
.../src/airflow/providers/sftp/triggers/sftp.py | 22 ++-
providers/sftp/tests/unit/sftp/hooks/test_sftp.py | 205 +++++++++++++++++++--
.../sftp/tests/unit/sftp/operators/test_sftp.py | 31 +++-
providers/ssh/docs/changelog.rst | 15 ++
providers/ssh/docs/connections/ssh.rst | 2 +-
.../ssh/src/airflow/providers/ssh/hooks/ssh.py | 71 ++++++-
providers/ssh/tests/unit/ssh/hooks/test_ssh.py | 67 ++++++-
.../ssh/tests/unit/ssh/hooks/test_ssh_async.py | 64 +++++++
15 files changed, 518 insertions(+), 42 deletions(-)
diff --git
a/providers/amazon/tests/unit/amazon/aws/transfers/test_s3_to_sftp.py
b/providers/amazon/tests/unit/amazon/aws/transfers/test_s3_to_sftp.py
index 3ed273f7d96..e84d00ee8dc 100644
--- a/providers/amazon/tests/unit/amazon/aws/transfers/test_s3_to_sftp.py
+++ b/providers/amazon/tests/unit/amazon/aws/transfers/test_s3_to_sftp.py
@@ -51,6 +51,18 @@ DEFAULT_DATE = timezone.datetime(2018, 1, 1)
class TestS3ToSFTPOperator:
+ @pytest.fixture(autouse=True)
+ def _ssh_default_allows_unknown_host(self, monkeypatch):
+ """Let the bundled ``ssh_default`` connection reach the test SSH
server.
+
+ ``no_host_key_check`` now defaults to false, and ``ssh_default`` is a
bare
+ ``ssh://localhost`` with no extras, so the transfer operators -- which
build
+ their own ``SFTPHook`` from the conn id and cannot be handed a
pre-configured
+ hook -- would be refused by host key verification. Setting the extra
here is
+ exactly what a deployment relying on the old default has to do.
+ """
+ monkeypatch.setenv("AIRFLOW_CONN_SSH_DEFAULT",
"ssh://localhost/?no_host_key_check=true")
+
def setup_method(self):
hook = SSHHook(ssh_conn_id="ssh_default")
hook.no_host_key_check = True
diff --git
a/providers/amazon/tests/unit/amazon/aws/transfers/test_sftp_to_s3.py
b/providers/amazon/tests/unit/amazon/aws/transfers/test_sftp_to_s3.py
index c715c67fb99..5a5f5c466c1 100644
--- a/providers/amazon/tests/unit/amazon/aws/transfers/test_sftp_to_s3.py
+++ b/providers/amazon/tests/unit/amazon/aws/transfers/test_sftp_to_s3.py
@@ -50,6 +50,18 @@ DEFAULT_DATE = timezone.datetime(2018, 1, 1)
class TestSFTPToS3Operator:
+ @pytest.fixture(autouse=True)
+ def _ssh_default_allows_unknown_host(self, monkeypatch):
+ """Let the bundled ``ssh_default`` connection reach the test SSH
server.
+
+ ``no_host_key_check`` now defaults to false, and ``ssh_default`` is a
bare
+ ``ssh://localhost`` with no extras, so the transfer operators -- which
build
+ their own ``SFTPHook`` from the conn id and cannot be handed a
pre-configured
+ hook -- would be refused by host key verification. Setting the extra
here is
+ exactly what a deployment relying on the old default has to do.
+ """
+ monkeypatch.setenv("AIRFLOW_CONN_SSH_DEFAULT",
"ssh://localhost/?no_host_key_check=true")
+
def setup_method(self):
hook = SSHHook(ssh_conn_id="ssh_default")
diff --git a/providers/sftp/docs/changelog.rst
b/providers/sftp/docs/changelog.rst
index 8e937cac855..79712edd86e 100644
--- a/providers/sftp/docs/changelog.rst
+++ b/providers/sftp/docs/changelog.rst
@@ -27,6 +27,22 @@
Changelog
---------
+.. warning::
+ The ``no_host_key_check`` connection extra now defaults to ``false``. A
connection to a host that has
+ no entry in the known hosts file is refused unless host key verification is
disabled explicitly or a
+ ``host_key`` is supplied in the connection's extra field.
+
+ Deployments that relied on the previous default can keep the earlier
behaviour by adding the host key
+ to the known hosts file, supplying ``host_key`` on the connection, setting
the ``no_host_key_check``
+ connection extra to ``true``, or -- when building
``SSHHook``/``SFTPHook``/``SFTPHookAsync`` directly
+ rather than from a connection -- passing the new ``no_host_key_check=True``
constructor argument. The
+ constructor argument takes precedence over the connection extra, and a value
set on the hook passed to
+ a deferrable ``SFTPOperator`` is carried into its trigger.
+
+ The previously undocumented ``ignore_hostkey_verification`` extra is now
honoured as a deprecated alias
+ for ``no_host_key_check`` and emits a ``DeprecationWarning``. It had no
effect before: connections that
+ set it were relying on the old permissive default rather than on the setting
itself.
+
6.1.0
.....
diff --git a/providers/sftp/docs/connections/sftp.rst
b/providers/sftp/docs/connections/sftp.rst
index 6cc3cb47fbb..cb93aff6f54 100644
--- a/providers/sftp/docs/connections/sftp.rst
+++ b/providers/sftp/docs/connections/sftp.rst
@@ -65,7 +65,7 @@ Extra (optional)
* ``conn_timeout`` - An optional timeout (in seconds) for the TCP connect.
Default is ``10``.
* ``timeout`` - Deprecated - use conn_timeout instead.
* ``compress`` - ``true`` to ask the remote client/server to compress
traffic; ``false`` to refuse compression. Default is ``true``.
- * ``no_host_key_check`` - Set to ``false`` to restrict connecting to hosts
with no entries in ``~/.ssh/known_hosts`` (Hosts file). This provides maximum
protection against trojan horse attacks, but can be troublesome when the
``/etc/ssh/ssh_known_hosts`` file is poorly maintained or connections to new
hosts are frequently made. This option forces the user to manually add all new
hosts. Default is ``true``, ssh will automatically add new host keys to the
user known hosts files.
+ * ``no_host_key_check`` - Set to ``true`` to connect to hosts that have no
entry in ``~/.ssh/known_hosts`` (Hosts file), accepting unknown host keys
without requiring a known-hosts entry. Default is ``false``, which restricts
connecting to hosts already present in the known hosts file. The default
provides maximum protection against trojan horse attacks, but can be
troublesome when the ``/etc/ssh/ssh_known_hosts`` file is poorly maintained or
connections to new hosts are frequently m [...]
* ``allow_host_key_change`` - Set to ``true`` if you want to allow
connecting to hosts that has host key changed or when you get 'REMOTE HOST
IDENTIFICATION HAS CHANGED' error. This won't protect against
Man-In-The-Middle attacks. Other possible solution is to remove the host entry
from ``~/.ssh/known_hosts`` file. Default is ``false``.
* ``look_for_keys`` - Set to ``false`` if you want to disable searching
for discoverable private key files in ``~/.ssh/``
* ``host_key`` - The base64 encoded ssh-rsa public key of the host or
"ssh-<key type> <key data>" (as you would find in the ``known_hosts`` file).
Specifying this allows making the connection if and only if the public key of
the endpoint matches this value.
diff --git a/providers/sftp/pyproject.toml b/providers/sftp/pyproject.toml
index 7a6e5d68279..057c92a0571 100644
--- a/providers/sftp/pyproject.toml
+++ b/providers/sftp/pyproject.toml
@@ -59,7 +59,7 @@ requires-python = ">=3.11"
# After you modify the dependencies, and rebuild your Breeze CI image with
``breeze ci-image build``
dependencies = [
"apache-airflow>=2.11.0",
- "apache-airflow-providers-ssh>=6.0.0",
+ "apache-airflow-providers-ssh>=6.0.0", # use next version
"apache-airflow-providers-common-compat>=1.12.0",
"paramiko>=4.0.0",
"asyncssh>=2.12.0; python_version < '3.14'",
diff --git a/providers/sftp/src/airflow/providers/sftp/hooks/sftp.py
b/providers/sftp/src/airflow/providers/sftp/hooks/sftp.py
index 96df4feb730..48badace7d3 100644
--- a/providers/sftp/src/airflow/providers/sftp/hooks/sftp.py
+++ b/providers/sftp/src/airflow/providers/sftp/hooks/sftp.py
@@ -235,11 +235,11 @@ class SFTPHook(SSHHook):
auth_timeout=self.auth_timeout,
host_proxy_cmd=self.host_proxy_cmd,
conn_retry_attempts=self.conn_retry_attempts,
+ no_host_key_check=self.no_host_key_check,
)
# These have no constructor parameter and are only ever resolved from
the
# connection's `extra` field or left at their class default, so copy
the
# parent's already-resolved values across explicitly.
- worker_hook.no_host_key_check = self.no_host_key_check
worker_hook.allow_host_key_change = self.allow_host_key_change
worker_hook.host_key = self.host_key
worker_hook.look_for_keys = self.look_for_keys
@@ -879,6 +879,9 @@ class SFTPHookAsync(BaseHook):
:param known_hosts: path to the known_hosts file on the local file system.
Defaults to ``~/.ssh/known_hosts``.
:param key_file: path to the client key file used for authentication to
SFTP server
:param passphrase: passphrase used with the key_file for authentication to
SFTP server
+ :param no_host_key_check: Set to ``True`` to skip host key verification.
Overrides the
+ connection's ``no_host_key_check`` extra. Defaults to ``None``,
meaning the value is
+ taken from the connection (and host keys are verified when the
connection does not set it).
"""
conn_name_attr = "ssh_conn_id"
@@ -898,6 +901,7 @@ class SFTPHookAsync(BaseHook):
key_file: str = "",
passphrase: str = "",
private_key: str = "",
+ no_host_key_check: bool | None = None,
) -> None:
self.sftp_conn_id = sftp_conn_id
self.host = host
@@ -908,6 +912,7 @@ class SFTPHookAsync(BaseHook):
self.key_file = key_file
self.passphrase = passphrase
self.private_key = private_key
+ self.no_host_key_check = no_host_key_check
self.conn: asyncssh.SFTPClient | None = None
self._conn_count = 0
self._conn_lock = asyncio.Lock()
@@ -931,8 +936,20 @@ class SFTPHookAsync(BaseHook):
host_key = extra_options.get("host_key")
nhkc_raw = extra_options.get("no_host_key_check")
- no_host_key_check = True if nhkc_raw is None else
(str(nhkc_raw).lower() == "true")
+ if nhkc_raw is None and "ignore_hostkey_verification" in extra_options:
+ warnings.warn(
+ "The `ignore_hostkey_verification` connection extra is
deprecated; "
+ "use `no_host_key_check` instead.",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+ nhkc_raw = extra_options["ignore_hostkey_verification"]
+ no_host_key_check = False if nhkc_raw is None else
(str(nhkc_raw).lower() == "true")
+ if self.no_host_key_check is not None:
+ no_host_key_check = self.no_host_key_check
+ # Validated on the effective value, so the constructor argument can
resolve a connection
+ # that sets both `host_key` and `no_host_key_check` -- the same rule
as `SSHHook`.
if host_key is not None and no_host_key_check:
raise ValueError("Host key check was skipped, but `host_key` value
was given")
@@ -949,7 +966,16 @@ class SFTPHookAsync(BaseHook):
)
if len(host_key_parts) >= 2:
host_key = " ".join(host_key_parts[:2])
- self.known_hosts = f"{conn.host} {host_key}".encode()
+ else:
+ # A bare key is RSA, as on the sync hook; asyncssh needs the
type spelled out.
+ host_key = f"ssh-rsa {host_key}"
+ self.known_hosts = f"{self.host or conn.host} {host_key}".encode()
+
+ def _should_use_known_hosts(self) -> bool:
+ """Leave a missing default file unset so AsyncSSH reports an untrusted
host."""
+ if self.known_hosts == os.path.expanduser(self.default_known_hosts):
+ return os.path.isfile(self.known_hosts)
+ return True
async def _get_conn(self) -> asyncssh.SSHClientConnection:
"""
@@ -963,8 +989,8 @@ class SFTPHookAsync(BaseHook):
- passphrase
"""
conn = await get_async_connection(self.sftp_conn_id)
- if conn.extra is not None:
- self._parse_extras(conn) # type: ignore[arg-type]
+ # Parsed even without extras: a constructor `no_host_key_check` still
has to be applied.
+ self._parse_extras(conn) # type: ignore[arg-type]
def _get_value(self_val, conn_val, default=None):
"""Return the first non-None value among self, conn, default."""
@@ -985,7 +1011,7 @@ class SFTPHookAsync(BaseHook):
if self.known_hosts:
if self.known_hosts.lower() == "none":
conn_config.update(known_hosts=None)
- else:
+ elif self._should_use_known_hosts():
conn_config.update(known_hosts=self.known_hosts)
if self.private_key:
_private_key = asyncssh.import_private_key(self.private_key,
self.passphrase)
diff --git a/providers/sftp/src/airflow/providers/sftp/operators/sftp.py
b/providers/sftp/src/airflow/providers/sftp/operators/sftp.py
index 249628d7a30..aabef230ea3 100644
--- a/providers/sftp/src/airflow/providers/sftp/operators/sftp.py
+++ b/providers/sftp/src/airflow/providers/sftp/operators/sftp.py
@@ -189,6 +189,7 @@ class SFTPOperator(BaseOperator):
remote_host=self.remote_host,
concurrency=self.concurrency,
prefetch=self.prefetch,
+ no_host_key_check=self.sftp_hook.no_host_key_check,
),
method_name="execute_complete",
)
diff --git a/providers/sftp/src/airflow/providers/sftp/triggers/sftp.py
b/providers/sftp/src/airflow/providers/sftp/triggers/sftp.py
index 16c894dd3b1..e9d69d94801 100644
--- a/providers/sftp/src/airflow/providers/sftp/triggers/sftp.py
+++ b/providers/sftp/src/airflow/providers/sftp/triggers/sftp.py
@@ -33,13 +33,23 @@ from airflow.triggers.base import BaseTrigger, TriggerEvent
class BaseSFTPTrigger(BaseTrigger):
"""Base class for SFTP triggers, providing shared async hook
construction."""
- def __init__(self, sftp_conn_id: str = "sftp_default", remote_host: str |
None = None) -> None:
+ def __init__(
+ self,
+ sftp_conn_id: str = "sftp_default",
+ remote_host: str | None = None,
+ no_host_key_check: bool | None = None,
+ ) -> None:
super().__init__()
self.sftp_conn_id = sftp_conn_id
self.remote_host = remote_host
+ self.no_host_key_check = no_host_key_check
def _get_async_hook(self) -> SFTPHookAsync:
- return SFTPHookAsync(sftp_conn_id=self.sftp_conn_id,
host=self.remote_host)
+ return SFTPHookAsync(
+ sftp_conn_id=self.sftp_conn_id,
+ host=self.remote_host,
+ no_host_key_check=self.no_host_key_check,
+ )
class SFTPTrigger(BaseSFTPTrigger):
@@ -150,6 +160,8 @@ class SFTPTransferTrigger(BaseSFTPTrigger):
:param remote_host: Remote host to connect to (overrides connection).
:param concurrency: Number of threads for directory transfers.
:param prefetch: Whether to prefetch during file retrieval.
+ :param no_host_key_check: Host key verification setting of the operator's
hook, so that a
+ constructor override on that hook survives deferral. ``None`` defers
to the connection.
"""
def __init__(
@@ -163,8 +175,11 @@ class SFTPTransferTrigger(BaseSFTPTrigger):
remote_host: str | None = None,
concurrency: int = 1,
prefetch: bool = True,
+ no_host_key_check: bool | None = None,
) -> None:
- super().__init__(sftp_conn_id=sftp_conn_id, remote_host=remote_host)
+ super().__init__(
+ sftp_conn_id=sftp_conn_id, remote_host=remote_host,
no_host_key_check=no_host_key_check
+ )
self.local_filepath = local_filepath
self.remote_filepath = remote_filepath
self.operation = operation
@@ -187,6 +202,7 @@ class SFTPTransferTrigger(BaseSFTPTrigger):
"remote_host": self.remote_host,
"concurrency": self.concurrency,
"prefetch": self.prefetch,
+ "no_host_key_check": self.no_host_key_check,
},
)
diff --git a/providers/sftp/tests/unit/sftp/hooks/test_sftp.py
b/providers/sftp/tests/unit/sftp/hooks/test_sftp.py
index c6cdf214915..34a7dfdc75d 100644
--- a/providers/sftp/tests/unit/sftp/hooks/test_sftp.py
+++ b/providers/sftp/tests/unit/sftp/hooks/test_sftp.py
@@ -22,6 +22,7 @@ import json
import os
import shutil
import stat
+from contextlib import nullcontext
from io import BytesIO, StringIO
from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock, Mock, PropertyMock, call, patch
@@ -33,6 +34,7 @@ from asyncssh.sftp import SFTPName
from paramiko.client import SSHClient
from paramiko.sftp_client import SFTPClient
+from airflow.exceptions import AirflowProviderDeprecationWarning
from airflow.models import Connection
from airflow.providers.common.compat.sdk import AirflowException
from airflow.providers.sftp.hooks.sftp import CHUNK_SIZE, SFTPHook,
SFTPHookAsync, SFTPOperation
@@ -97,7 +99,7 @@ class TestSFTPHook:
"""Define default connection during tests and create directory
structure."""
temp_dir = tmp_path_factory.mktemp("sftp-temp")
self.old_login = self.update_connection(SFTP_CONNECTION_USER)
- self.hook = SFTPHook()
+ self.hook = SFTPHook(no_host_key_check=True)
os.makedirs(os.path.join(temp_dir, TMP_DIR_FOR_TESTS, SUB_DIR))
for file_name in [TMP_FILE_FOR_TESTS, ANOTHER_FILE_FOR_TESTS,
LOG_FILE_FOR_TESTS]:
@@ -357,7 +359,7 @@ class TestSFTPHook:
connection = Connection(login="login", host="host")
get_connection.return_value = connection
hook = SFTPHook()
- assert hook.no_host_key_check is True
+ assert hook.no_host_key_check is False
@patch("airflow.providers.sftp.hooks.sftp.SFTPHook.get_connection")
def test_no_host_key_check_enabled(self, get_connection):
@@ -393,10 +395,12 @@ class TestSFTPHook:
@patch("airflow.providers.sftp.hooks.sftp.SFTPHook.get_connection")
def test_no_host_key_check_ignore(self, get_connection):
+ """``ignore_hostkey_verification`` is a deprecated alias for
``no_host_key_check``."""
connection = Connection(login="login", host="host",
extra='{"ignore_hostkey_verification": true}')
get_connection.return_value = connection
- hook = SFTPHook()
+ with pytest.warns(AirflowProviderDeprecationWarning,
match="ignore_hostkey_verification"):
+ hook = SFTPHook()
assert hook.no_host_key_check is True
@patch("airflow.providers.sftp.hooks.sftp.SFTPHook.get_connection")
@@ -726,6 +730,41 @@ class TestSFTPHook:
)
assert mock_build.call_count == workers
+ def
test_concurrent_transfer_passes_effective_host_key_policy_to_worker(self,
tmp_path):
+ connection = Connection(
+ conn_id="sftp_default",
+ conn_type="sftp",
+ host="connection.example.com",
+ login="user",
+ extra=json.dumps({"host_key": f"ssh-rsa {TEST_HOST_KEY}",
"no_host_key_check": True}),
+ )
+ built_hooks = []
+ original_build = SFTPHook._build_worker_hook
+
+ def spy_build(hook_self):
+ worker_hook = original_build(hook_self)
+ built_hooks.append(worker_hook)
+ return worker_hook
+
+ with (
+ patch("airflow.providers.sftp.hooks.sftp.SFTPHook.get_connection",
return_value=connection),
+ patch.object(SFTPHook, "get_managed_conn",
return_value=nullcontext()),
+ patch.object(SFTPHook, "path_exists", return_value=False),
+ patch.object(SFTPHook, "create_directory"),
+ patch.object(SFTPHook, "get_conn",
return_value=MagicMock(spec=SFTPClient)),
+ patch.object(SFTPHook, "_build_worker_hook", autospec=True,
side_effect=spy_build) as mock_build,
+ ):
+ parent_hook = SFTPHook(ssh_conn_id="sftp_default",
no_host_key_check=False)
+ parent_hook.store_directory_concurrently(
+ remote_full_path="/remote/target",
+ local_full_path=str(tmp_path),
+ workers=1,
+ )
+
+ mock_build.assert_called_once()
+ assert parent_hook.no_host_key_check is False
+ assert built_hooks[0].no_host_key_check is False
+
def test_validate_within_directory_rejects_escape(self):
base = os.path.join(self.temp_dir, "download")
with pytest.raises(ValueError, match="outside the destination
directory"):
@@ -822,6 +861,9 @@ class MockSSHClient:
return MockSFTPClient()
+DEFAULT_KNOWN_HOSTS_PATH = os.path.expanduser("~/.ssh/known_hosts")
+
+
class MockAirflowConnection:
def __init__(self, known_hosts="~/.ssh/known_hosts"):
self.host = "localhost"
@@ -942,13 +984,16 @@ class TestSFTPHookAsync:
22,
"ssh-ed25519
AAAAC3NzaC1lZDI1NTE5AAAAIFe8P8lk5HFfL/rMlcCMHQhw1cg+uZtlK5rXQk2C4pOY user@host",
),
- (2222,
"AAAAC3NzaC1lZDI1NTE5AAAAIFe8P8lk5HFfL/rMlcCMHQhw1cg+uZtlK5rXQk2C4pOY"),
+ (2222, TEST_HOST_KEY),
(
2222,
"ecdsa-sha2-nistp256
AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBDDsXFe87LsBA1Hfi+mtw"
"/EoQkv8bXVtfOwdMP1ETpHVsYpm5QG/7tsLlKdE8h6EoV/OFw7XQtoibNZp/l5ABjE=",
),
],
+ # TEST_HOST_KEY is generated at import time, so explicit ids keep
collection
+ # identical across pytest-xdist workers.
+ ids=["ed25519", "ed25519-with-comment", "bare-rsa", "ecdsa"],
)
@patch("asyncssh.connect", new_callable=AsyncMock)
@patch("asyncssh.import_private_key")
@@ -974,9 +1019,77 @@ class TestSFTPHookAsync:
await hook._get_conn()
host_key_parts = mock_host_key.split()
- expected_host_key = " ".join(host_key_parts[:2]) if
len(host_key_parts) >= 2 else mock_host_key
+ # A bare key is RSA; asyncssh does not accept the two-field `host key`
form.
+ expected_host_key = (
+ " ".join(host_key_parts[:2]) if len(host_key_parts) >= 2 else
f"ssh-rsa {mock_host_key}"
+ )
assert hook.known_hosts == f"localhost {expected_host_key}".encode()
+ @patch("asyncssh.connect", new_callable=AsyncMock)
+ @patch("airflow.providers.sftp.hooks.sftp.get_async_connection")
+ @pytest.mark.asyncio
+ async def test_no_host_key_check_defaults_to_false(self,
mock_get_connection, mock_connect):
+ """A connection that omits ``no_host_key_check`` keeps host key
verification enabled."""
+
+ class MockAirflowConnectionWithoutHostKeyExtras:
+ host = "localhost"
+ port = 22
+ login = "username"
+ password = "password"
+ extra = "{}"
+ extra_dejson: dict = {}
+
+ mock_get_connection.return_value =
MockAirflowConnectionWithoutHostKeyExtras()
+
+ hook = SFTPHookAsync()
+ await hook._get_conn()
+
+ assert hook.known_hosts != "none"
+ assert str(hook.known_hosts).endswith(".ssh/known_hosts")
+
+ @patch("asyncssh.connect", new_callable=AsyncMock)
+ @patch("airflow.providers.sftp.hooks.sftp.get_async_connection")
+ @pytest.mark.asyncio
+ async def test_parse_extras_honours_deprecated_alias(self,
mock_get_connection, mock_connect):
+ """``ignore_hostkey_verification`` keeps working on the async path
too."""
+
+ class MockAirflowConnectionWithAlias:
+ host = "localhost"
+ port = 22
+ login = "username"
+ password = "password"
+ extra = '{"ignore_hostkey_verification": true}'
+ extra_dejson = {"ignore_hostkey_verification": True}
+
+ mock_get_connection.return_value = MockAirflowConnectionWithAlias()
+
+ hook = SFTPHookAsync()
+ with pytest.warns(AirflowProviderDeprecationWarning,
match="ignore_hostkey_verification"):
+ await hook._get_conn()
+
+ assert hook.known_hosts == "none"
+
+ @patch("asyncssh.connect", new_callable=AsyncMock)
+ @patch("airflow.providers.sftp.hooks.sftp.get_async_connection")
+ @pytest.mark.asyncio
+ async def test_parse_extras_canonical_key_wins_over_alias(self,
mock_get_connection, mock_connect):
+ """``no_host_key_check`` takes precedence over the deprecated alias."""
+
+ class MockAirflowConnectionWithBothKeys:
+ host = "localhost"
+ port = 22
+ login = "username"
+ password = "password"
+ extra = '{"no_host_key_check": false,
"ignore_hostkey_verification": true}'
+ extra_dejson = {"no_host_key_check": False,
"ignore_hostkey_verification": True}
+
+ mock_get_connection.return_value = MockAirflowConnectionWithBothKeys()
+
+ hook = SFTPHookAsync()
+ await hook._get_conn()
+
+ assert hook.known_hosts != "none"
+
@patch("asyncssh.connect", new_callable=AsyncMock)
@patch("airflow.providers.sftp.hooks.sftp.get_async_connection")
@pytest.mark.asyncio
@@ -1009,6 +1122,74 @@ class TestSFTPHookAsync:
with pytest.raises(ValueError, match="Host key check was skipped, but
`host_key` value was given"):
await hook._get_conn()
+ @patch("asyncssh.connect", new_callable=AsyncMock)
+ @patch("airflow.providers.sftp.hooks.sftp.get_async_connection")
+ @pytest.mark.asyncio
+ async def test_constructor_no_host_key_check_applies_without_extras(
+ self, mock_get_connection, mock_connect
+ ):
+ """The constructor opt-out applies even when the connection has no
extras at all."""
+ mock_get_connection.return_value = Connection(
+ conn_id="sftp_default", conn_type="sftp", host="localhost",
login="username"
+ )
+
+ hook = SFTPHookAsync(no_host_key_check=True)
+ await hook._get_conn()
+
+ assert mock_connect.call_args.kwargs["known_hosts"] is None
+
+ @patch("asyncssh.connect", new_callable=AsyncMock)
+ @patch("asyncssh.import_private_key")
+ @patch("airflow.providers.sftp.hooks.sftp.get_async_connection")
+ @pytest.mark.asyncio
+ async def
test_constructor_no_host_key_check_false_resolves_conflicting_extras(
+ self, mock_get_connection, mock_import_private_key, mock_connect
+ ):
+ """``no_host_key_check=False`` wins over the extra, so a connection's
`host_key` is used."""
+ mock_get_connection.return_value = MockAirflowConnectionWithHostKey(
+ host_key=TEST_HOST_KEY, no_host_key_check=True
+ )
+
+ hook = SFTPHookAsync(no_host_key_check=False)
+ await hook._get_conn()
+
+ assert hook.known_hosts == f"localhost ssh-rsa
{TEST_HOST_KEY}".encode()
+
+ @patch("asyncssh.connect", new_callable=AsyncMock)
+ @patch("airflow.providers.sftp.hooks.sftp.get_async_connection")
+ @pytest.mark.asyncio
+ async def test_constructor_no_host_key_check_true_with_host_key_raises(
+ self, mock_get_connection, mock_connect
+ ):
+ mock_get_connection.return_value = MockAirflowConnectionWithHostKey(
+ host_key=TEST_HOST_KEY, no_host_key_check=False
+ )
+
+ hook = SFTPHookAsync(no_host_key_check=True)
+ with pytest.raises(ValueError, match="Host key check was skipped, but
`host_key` value was given"):
+ await hook._get_conn()
+
+ @patch("asyncssh.connect", new_callable=AsyncMock)
+ @patch("asyncssh.import_private_key")
+ @patch("airflow.providers.sftp.hooks.sftp.get_async_connection")
+ @pytest.mark.asyncio
+ async def test_host_key_entry_uses_host_override(
+ self, mock_get_connection, mock_import_private_key, mock_connect
+ ):
+ """The known_hosts entry names the host actually connected to, not the
connection's host."""
+ mock_get_connection.return_value = MockAirflowConnectionWithHostKey(
+ host_key=f"ssh-rsa {TEST_HOST_KEY}", no_host_key_check=False
+ )
+
+ hook = SFTPHookAsync(host="override.example")
+ await hook._get_conn()
+
+ assert mock_connect.call_args.kwargs["host"] == "override.example"
+ assert (
+ mock_connect.call_args.kwargs["known_hosts"]
+ == f"override.example ssh-rsa {TEST_HOST_KEY}".encode()
+ )
+
@patch("paramiko.SSHClient.connect")
@patch("asyncssh.import_private_key")
@patch("asyncssh.connect", new_callable=AsyncMock)
@@ -1044,7 +1225,7 @@ class TestSFTPHookAsync:
"username": "username",
"password": "password",
"client_keys": "~/keys/my_key",
- "known_hosts": None,
+ "known_hosts": "~/.ssh/known_hosts",
"passphrase": "mypassphrase",
}
@@ -1073,16 +1254,16 @@ class TestSFTPHookAsync:
"username": "username",
"password": "password",
"client_keys": ["test"],
- "known_hosts": None,
"passphrase": "mypassphrase",
}
mock_connect.assert_called_with(**expected_connection_details)
@pytest.mark.asyncio
+ @patch("airflow.providers.sftp.hooks.sftp.os.path.isfile",
return_value=False)
@patch("asyncssh.connect", new_callable=AsyncMock)
@patch("airflow.providers.sftp.hooks.sftp.get_async_connection")
- async def test_connection_port_default_to_22(self, mock_get_connection,
mock_connect):
+ async def test_connection_port_default_to_22(self, mock_get_connection,
mock_connect, mock_isfile):
from unittest.mock import Mock, call
mock_get_connection.return_value = Mock(
@@ -1104,14 +1285,15 @@ class TestSFTPHookAsync:
port=22,
username="username",
password="password",
- known_hosts=None,
),
]
+ mock_isfile.assert_called_once_with(DEFAULT_KNOWN_HOSTS_PATH)
@pytest.mark.asyncio
+ @patch("airflow.providers.sftp.hooks.sftp.os.path.isfile",
return_value=True)
@patch("asyncssh.connect", new_callable=AsyncMock)
@patch("airflow.providers.sftp.hooks.sftp.get_async_connection")
- async def test_init_argument_not_ignored(self, mock_get_connection,
mock_connect):
+ async def test_init_argument_not_ignored(self, mock_get_connection,
mock_connect, mock_isfile):
from unittest.mock import Mock, call
mock_get_connection.return_value = Mock(
@@ -1136,9 +1318,10 @@ class TestSFTPHookAsync:
port=25,
username="username-from-init",
password="password-from-init",
- known_hosts=None,
+ known_hosts=DEFAULT_KNOWN_HOSTS_PATH,
),
]
+ mock_isfile.assert_called_once_with(DEFAULT_KNOWN_HOSTS_PATH)
@pytest.mark.asyncio
async def test_list_directory_path_does_not_exist(self, sftp_hook_mocked):
diff --git a/providers/sftp/tests/unit/sftp/operators/test_sftp.py
b/providers/sftp/tests/unit/sftp/operators/test_sftp.py
index 60bec348499..bb58b4b7f17 100644
--- a/providers/sftp/tests/unit/sftp/operators/test_sftp.py
+++ b/providers/sftp/tests/unit/sftp/operators/test_sftp.py
@@ -740,6 +740,31 @@ class TestSFTPOperatorDeferrable:
operator.execute(context={})
assert exc.value.trigger.sftp_conn_id == "my_prod_sftp"
+ @mock.patch("asyncssh.connect", new_callable=mock.AsyncMock)
+ @mock.patch("airflow.providers.sftp.hooks.sftp.get_async_connection")
+ @mock.patch.dict("os.environ", {"AIRFLOW_CONN_MY_PROD_SFTP":
"sftp://[email protected]"})
+ @pytest.mark.asyncio
+ async def test_sftp_operator_defer_keeps_hook_no_host_key_check(self,
mock_get_connection, mock_connect):
+ """A host key opt-out set on the supplied hook survives deferral into
the trigger's async hook."""
+ mock_get_connection.return_value = Connection(
+ conn_id="my_prod_sftp", conn_type="sftp", host="example.com",
login="user"
+ )
+ operator = SFTPOperator(
+ task_id="test_sftp_defer_no_host_key_check",
+ sftp_hook=SFTPHook(ssh_conn_id="my_prod_sftp",
no_host_key_check=True),
+ local_filepath="/tmp/test.txt",
+ remote_filepath="/remote/test.txt",
+ operation=SFTPOperation.PUT,
+ deferrable=True,
+ )
+ with pytest.raises(TaskDeferred) as exc:
+ operator.execute(context={})
+
+ trigger = exc.value.trigger
+ assert trigger.serialize()[1]["no_host_key_check"] is True
+ await trigger._get_async_hook()._get_conn()
+ assert mock_connect.call_args.kwargs["known_hosts"] is None
+
def test_sftp_operator_defer_without_any_conn_id_raises(self):
operator = SFTPOperator(
task_id="test_sftp_defer_no_conn_id",
@@ -819,7 +844,9 @@ class TestSFTPTransferTrigger:
remote_host="explicit-host.example.com",
)
trigger._get_async_hook()
- mock_hook_async.assert_called_once_with(sftp_conn_id="ssh_default",
host="explicit-host.example.com")
+ mock_hook_async.assert_called_once_with(
+ sftp_conn_id="ssh_default", host="explicit-host.example.com",
no_host_key_check=None
+ )
@mock.patch("airflow.providers.sftp.triggers.sftp.SFTPHookAsync",
autospec=True)
def test_get_async_hook_defaults_remote_host_to_none(self,
mock_hook_async):
@@ -831,7 +858,7 @@ class TestSFTPTransferTrigger:
operation="put",
)
trigger._get_async_hook()
- mock_hook_async.assert_called_once_with(sftp_conn_id="ssh_default",
host=None)
+ mock_hook_async.assert_called_once_with(sftp_conn_id="ssh_default",
host=None, no_host_key_check=None)
def test_run_success(self):
"""Test run() yields TriggerEvent with status success."""
diff --git a/providers/ssh/docs/changelog.rst b/providers/ssh/docs/changelog.rst
index fbba5ea1875..5efffb45d7d 100644
--- a/providers/ssh/docs/changelog.rst
+++ b/providers/ssh/docs/changelog.rst
@@ -27,6 +27,21 @@
Changelog
---------
+.. warning::
+ The ``no_host_key_check`` connection extra now defaults to ``false``. A
connection to a host that has
+ no entry in the known hosts file is refused unless host key verification is
disabled explicitly or a
+ ``host_key`` is supplied in the connection's extra field.
+
+ Deployments that relied on the previous default can keep the earlier
behaviour by adding the host key
+ to the known hosts file, supplying ``host_key`` on the connection, setting
the ``no_host_key_check``
+ connection extra to ``true``, or -- when building ``SSHHook``/``SFTPHook``
directly rather than from a
+ connection -- passing the new ``no_host_key_check=True`` constructor
argument. The constructor argument
+ takes precedence over the connection extra.
+
+ The previously undocumented ``ignore_hostkey_verification`` extra is now
honoured as a deprecated alias
+ for ``no_host_key_check`` and emits a ``DeprecationWarning``. It had no
effect before: connections that
+ set it were relying on the old permissive default rather than on the setting
itself.
+
6.1.0
.....
diff --git a/providers/ssh/docs/connections/ssh.rst
b/providers/ssh/docs/connections/ssh.rst
index 9ca7ce0dedf..9cfa3f88cdd 100644
--- a/providers/ssh/docs/connections/ssh.rst
+++ b/providers/ssh/docs/connections/ssh.rst
@@ -51,7 +51,7 @@ Extra (optional)
* ``timeout`` - Deprecated - use conn_timeout instead.
* ``cmd_timeout`` - Timeout (in seconds) for executing the command. The
default is 10 seconds. `null` value means no timeout.
* ``compress`` - ``true`` to ask the remote client/server to compress
traffic; ``false`` to refuse compression. Default is ``true``.
- * ``no_host_key_check`` - Set to ``false`` to restrict connecting to hosts
with no entries in ``~/.ssh/known_hosts`` (Hosts file). This provides maximum
protection against trojan horse attacks, but can be troublesome when the
``/etc/ssh/ssh_known_hosts`` file is poorly maintained or connections to new
hosts are frequently made. This option forces the user to manually add all new
hosts. Default is ``true``, ssh will automatically add new host keys to the
user known hosts files.
+ * ``no_host_key_check`` - Set to ``true`` to connect to hosts that have no
entry in ``~/.ssh/known_hosts`` (Hosts file), accepting unknown host keys
without requiring a known-hosts entry. Default is ``false``, which restricts
connecting to hosts already present in the known hosts file. The default
provides maximum protection against trojan horse attacks, but can be
troublesome when the ``/etc/ssh/ssh_known_hosts`` file is poorly maintained or
connections to new hosts are frequently m [...]
* ``allow_host_key_change`` - Set to ``true`` if you want to allow
connecting to hosts that has host key changed or when you get 'REMOTE HOST
IDENTIFICATION HAS CHANGED' error. This won't protect against
Man-In-The-Middle attacks. Other possible solution is to remove the host entry
from ``~/.ssh/known_hosts`` file. Default is ``false``.
* ``look_for_keys`` - Set to ``false`` if you want to disable searching
for discoverable private key files in ``~/.ssh/``
* ``host_key`` - The base64 encoded ssh-rsa public key of the host or
``"<key type> <key data>"`` (as you would find in the ``known_hosts`` file).
Specifying this allows making the connection if and only if the public key of
the endpoint matches this value. Supported key type strings are ``ssh-rsa``,
``ecdsa-sha2-nistp256``, ``ecdsa-sha2-nistp384``, ``ecdsa-sha2-nistp521``, and
``ssh-ed25519``; the legacy ``ssh-ecdsa`` form is also accepted for
compatibility. DSA/DSS (``ssh-dss``) ho [...]
diff --git a/providers/ssh/src/airflow/providers/ssh/hooks/ssh.py
b/providers/ssh/src/airflow/providers/ssh/hooks/ssh.py
index 38f68d43131..cbfad4a1ecc 100644
--- a/providers/ssh/src/airflow/providers/ssh/hooks/ssh.py
+++ b/providers/ssh/src/airflow/providers/ssh/hooks/ssh.py
@@ -21,6 +21,7 @@ from __future__ import annotations
import os
import selectors
+import warnings
from base64 import decodebytes
from collections.abc import Sequence
from functools import cached_property
@@ -31,6 +32,7 @@ import paramiko
from paramiko.config import SSH_PORT
from tenacity import Retrying, stop_after_attempt, wait_fixed, wait_random
+from airflow.exceptions import AirflowProviderDeprecationWarning
from airflow.providers.common.compat.connection import get_async_connection
from airflow.providers.common.compat.sdk import AirflowException, BaseHook
from airflow.providers.ssh.tunnel import AsyncSSHTunnel, SSHTunnel
@@ -92,6 +94,9 @@ class SSHHook(BaseHook):
lifetime of the transport
:param ciphers: list of ciphers to use in order of preference
:param auth_timeout: timeout (in seconds) for the attempt to authenticate
with the remote_host
+ :param no_host_key_check: Set to ``True`` to skip host key verification.
Overrides the
+ connection's ``no_host_key_check`` extra. Defaults to ``None``,
meaning the value is
+ taken from the connection, or ``False`` when the connection does not
set it.
:param conn_retry_attempts: number of times to attempt the initial SSH
connection before
giving up (default 3). Raising this helps when many tasks target the
same SSH server at
once and some connections are transiently refused (e.g. ``sshd``
``MaxStartups`` throttling).
@@ -146,6 +151,7 @@ class SSHHook(BaseHook):
auth_timeout: int | None = None,
host_proxy_cmd: str | None = None,
conn_retry_attempts: int = 3,
+ no_host_key_check: bool | None = None,
) -> None:
super().__init__()
self.ssh_conn_id = ssh_conn_id
@@ -167,11 +173,20 @@ class SSHHook(BaseHook):
# Default values, overridable from Connection
self.compress = True
- self.no_host_key_check = True
+ self.no_host_key_check = False
self.allow_host_key_change = False
self.host_key = None
self.look_for_keys = True
+ # Captured before parsing the connection extras, which rebind the
`no_host_key_check`
+ # name below. Without a separate binding the caller's value is
silently discarded
+ # whenever the connection has extras that do not mention host key
checking.
+ constructor_no_host_key_check = no_host_key_check
+
+ # Parsing a `host_key` extra forces `self.no_host_key_check` to False,
so what the
+ # connection actually asked for is kept here and re-applied once
parsing is done.
+ extra_no_host_key_check: bool | None = None
+
# Placeholder for future cached connection
self.client: paramiko.SSHClient | None = None
@@ -213,12 +228,18 @@ class SSHHook(BaseHook):
host_key = extra_options.get("host_key")
no_host_key_check = extra_options.get("no_host_key_check")
- if no_host_key_check is not None:
- no_host_key_check = str(no_host_key_check).lower() ==
"true"
- if host_key is not None and no_host_key_check:
- raise ValueError("Must check host key when provided")
+ if no_host_key_check is None and "ignore_hostkey_verification"
in extra_options:
+ warnings.warn(
+ "The `ignore_hostkey_verification` connection extra is
deprecated; "
+ "use `no_host_key_check` instead.",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+ no_host_key_check =
extra_options["ignore_hostkey_verification"]
- self.no_host_key_check = no_host_key_check
+ if no_host_key_check is not None:
+ extra_no_host_key_check = str(no_host_key_check).lower()
== "true"
+ self.no_host_key_check = extra_no_host_key_check
if (
"allow_host_key_change" in extra_options
@@ -262,6 +283,21 @@ class SSHHook(BaseHook):
self.host_key = key_constructor(data=decoded_host_key)
self.no_host_key_check = False
+ # An explicit constructor argument wins over the connection extra and
the default.
+ # Without this there is no way to opt out of host key verification
when the hook is
+ # built directly rather than from a Connection.
+ if constructor_no_host_key_check is not None:
+ self.no_host_key_check = constructor_no_host_key_check
+ elif extra_no_host_key_check is not None:
+ self.no_host_key_check = extra_no_host_key_check
+
+ # Validated on the effective value rather than on the extras alone, so
that an explicit
+ # constructor argument can resolve a connection that sets both
`host_key` and
+ # `no_host_key_check`, and so that skipping the check while a host key
is configured is
+ # rejected whichever source asked for it.
+ if self.host_key is not None and self.no_host_key_check:
+ raise ValueError("Must check host key when provided")
+
if self.cmd_timeout is NOTSET:
self.cmd_timeout = CMD_TIMEOUT
@@ -625,7 +661,15 @@ class SSHHookAsync(BaseHook):
host_key = extra_options.get("host_key")
nhkc_raw = extra_options.get("no_host_key_check")
- no_host_key_check = str(nhkc_raw).lower() == "true" if nhkc_raw is not
None else True
+ if nhkc_raw is None and "ignore_hostkey_verification" in extra_options:
+ warnings.warn(
+ "The `ignore_hostkey_verification` connection extra is
deprecated; "
+ "use `no_host_key_check` instead.",
+ AirflowProviderDeprecationWarning,
+ stacklevel=2,
+ )
+ nhkc_raw = extra_options["ignore_hostkey_verification"]
+ no_host_key_check = str(nhkc_raw).lower() == "true" if nhkc_raw is not
None else False
if host_key is not None and no_host_key_check:
raise ValueError("Host key check was skipped, but `host_key` value
was given")
@@ -643,7 +687,16 @@ class SSHHookAsync(BaseHook):
)
if len(host_key_parts) >= 2:
host_key = " ".join(host_key_parts[:2])
- self.known_hosts = f"{conn.host} {host_key}".encode()
+ else:
+ # A bare key is RSA, as on the sync hook; asyncssh needs the
type spelled out.
+ host_key = f"ssh-rsa {host_key}"
+ self.known_hosts = f"{self.host or conn.host} {host_key}".encode()
+
+ def _should_use_known_hosts(self) -> bool:
+ """Leave a missing default file unset so AsyncSSH reports an untrusted
host."""
+ if self.known_hosts == os.path.expanduser(self.default_known_hosts):
+ return os.path.isfile(self.known_hosts)
+ return True
async def _get_conn(self):
"""
@@ -675,7 +728,7 @@ class SSHHookAsync(BaseHook):
if self.known_hosts:
if isinstance(self.known_hosts, str) and self.known_hosts.lower()
== "none":
conn_config["known_hosts"] = None
- else:
+ elif self._should_use_known_hosts():
conn_config["known_hosts"] = self.known_hosts
if self.private_key:
_private_key = asyncssh.import_private_key(self.private_key,
self.passphrase)
diff --git a/providers/ssh/tests/unit/ssh/hooks/test_ssh.py
b/providers/ssh/tests/unit/ssh/hooks/test_ssh.py
index bc8fa2ae4eb..14c734daa25 100644
--- a/providers/ssh/tests/unit/ssh/hooks/test_ssh.py
+++ b/providers/ssh/tests/unit/ssh/hooks/test_ssh.py
@@ -567,25 +567,25 @@ class TestSSHHook:
)
def test_ssh_connection(self):
- hook = SSHHook(ssh_conn_id="ssh_default")
+ hook = SSHHook(ssh_conn_id="ssh_default", no_host_key_check=True)
with hook.get_conn() as client:
(_, stdout, _) = client.exec_command("ls")
assert stdout.read() is not None
def test_ssh_connection_no_connection_id(self):
- hook = SSHHook(remote_host="localhost")
+ hook = SSHHook(remote_host="localhost", no_host_key_check=True)
assert hook.ssh_conn_id is None
with hook.get_conn() as client:
(_, stdout, _) = client.exec_command("ls")
assert stdout.read() is not None
def test_ssh_connection_old_cm(self):
- with SSHHook(ssh_conn_id="ssh_default").get_conn() as client:
+ with SSHHook(ssh_conn_id="ssh_default",
no_host_key_check=True).get_conn() as client:
(_, stdout, _) = client.exec_command("ls")
assert stdout.read() is not None
def test_tunnel(self):
- hook = SSHHook(ssh_conn_id="ssh_default")
+ hook = SSHHook(ssh_conn_id="ssh_default", no_host_key_check=True)
import socket
import subprocess
@@ -798,6 +798,54 @@ class TestSSHHook:
assert ssh_client.return_value.connect.called is True
assert ssh_client.return_value.set_missing_host_key_policy.called
is True
+ def test_constructor_no_host_key_check_overrides_unrelated_extras(self):
+ """An explicit constructor value survives a connection whose extras
omit host key settings."""
+ hook = SSHHook(ssh_conn_id=self.CONN_SSH_WITH_PRIVATE_KEY_EXTRA,
no_host_key_check=True)
+ assert hook.no_host_key_check is True
+
+ @mock.patch("airflow.providers.ssh.hooks.ssh.SSHHook.get_connection")
+ def test_constructor_no_host_key_check_overrides_empty_extras(self,
get_connection):
+ """An empty ``extra`` must not discard the constructor value."""
+ get_connection.return_value = Connection(
+ conn_id="ssh_empty_extra", host="localhost", conn_type="ssh",
extra="{}"
+ )
+ hook = SSHHook(ssh_conn_id="ssh_empty_extra", no_host_key_check=True)
+ assert hook.no_host_key_check is True
+
+ def test_constructor_no_host_key_check_overrides_connection_extra(self):
+ """The constructor wins over a connection extra that sets the opposite
value."""
+ hook = SSHHook(ssh_conn_id=self.CONN_SSH_WITH_EXTRA,
no_host_key_check=False)
+ assert hook.no_host_key_check is False
+
+ def
test_constructor_no_host_key_check_false_resolves_conflicting_extras(self):
+ """An explicit ``False`` makes a connection setting both ``host_key``
and the skip flag usable."""
+ hook = SSHHook(
+
ssh_conn_id=self.CONN_SSH_WITH_HOST_KEY_AND_NO_HOST_KEY_CHECK_TRUE,
no_host_key_check=False
+ )
+ assert hook.no_host_key_check is False
+ assert hook.host_key is not None
+
+ def
test_constructor_no_host_key_check_true_rejects_connection_host_key(self):
+ """Skipping the check is rejected whichever source asks for it while a
host key is configured."""
+ with pytest.raises(ValueError, match="Must check host key when
provided"):
+ SSHHook(
+
ssh_conn_id=self.CONN_SSH_WITH_HOST_KEY_AND_NO_HOST_KEY_CHECK_FALSE,
no_host_key_check=True
+ )
+
+ @mock.patch("airflow.providers.ssh.hooks.ssh.paramiko.SSHClient")
+ def test_no_host_key_check_defaults_to_false(self, ssh_client):
+ """A connection that does not set ``no_host_key_check`` still verifies
the host key."""
+ hook = SSHHook(ssh_conn_id=self.CONN_SSH_WITH_NO_EXTRA)
+ assert hook.no_host_key_check is False
+ with hook.get_conn():
+ assert ssh_client.return_value.load_system_host_keys.called is True
+ installed = [
+ call.args[0]
+ for call in
ssh_client.return_value.set_missing_host_key_policy.call_args_list
+ if call.args
+ ]
+ assert not any(isinstance(policy, paramiko.AutoAddPolicy) for
policy in installed)
+
@mock.patch("airflow.providers.ssh.hooks.ssh.paramiko.SSHClient")
def test_conn_retry_attempts_defaults_to_three(self, ssh_client):
hook = SSHHook(ssh_conn_id="ssh_default")
@@ -984,6 +1032,7 @@ class TestSSHHook:
def test_exec_ssh_client_command(self):
hook = SSHHook(
ssh_conn_id="ssh_default",
+ no_host_key_check=True,
conn_timeout=30,
banner_timeout=100,
)
@@ -1000,6 +1049,7 @@ class TestSSHHook:
def test_command_timeout_success(self):
hook = SSHHook(
ssh_conn_id="ssh_default",
+ no_host_key_check=True,
conn_timeout=30,
cmd_timeout=2,
banner_timeout=100,
@@ -1072,6 +1122,7 @@ class TestSSHHook:
def test_command_timeout_not_set(self, monkeypatch):
hook = SSHHook(
ssh_conn_id="ssh_default",
+ no_host_key_check=True,
conn_timeout=30,
cmd_timeout=None,
banner_timeout=100,
@@ -1128,7 +1179,7 @@ class TestSSHHook:
assert ssh_mock.return_value.load_host_keys.called is False
def test_connection_success(self):
- hook = SSHHook(ssh_conn_id="ssh_default")
+ hook = SSHHook(ssh_conn_id="ssh_default", no_host_key_check=True)
status, msg = hook.test_connection()
assert status is True
assert msg == "Connection successfully tested"
@@ -1141,14 +1192,14 @@ class TestSSHHook:
assert msg == "Test failure case"
def test_ssh_connection_client_is_reused_if_open(self):
- hook = SSHHook(ssh_conn_id="ssh_default")
+ hook = SSHHook(ssh_conn_id="ssh_default", no_host_key_check=True)
client1 = hook.get_conn()
client2 = hook.get_conn()
assert client1 is client2
assert client2.get_transport().is_active()
def test_ssh_connection_client_is_recreated_if_closed(self):
- hook = SSHHook(ssh_conn_id="ssh_default")
+ hook = SSHHook(ssh_conn_id="ssh_default", no_host_key_check=True)
client1 = hook.get_conn()
client1.close()
client2 = hook.get_conn()
@@ -1156,7 +1207,7 @@ class TestSSHHook:
assert client2.get_transport().is_active()
def test_ssh_connection_client_is_recreated_if_transport_closed(self):
- hook = SSHHook(ssh_conn_id="ssh_default")
+ hook = SSHHook(ssh_conn_id="ssh_default", no_host_key_check=True)
client1 = hook.get_conn()
client1.get_transport().close()
client2 = hook.get_conn()
diff --git a/providers/ssh/tests/unit/ssh/hooks/test_ssh_async.py
b/providers/ssh/tests/unit/ssh/hooks/test_ssh_async.py
index dbda9f5e0ab..5598f4ee288 100644
--- a/providers/ssh/tests/unit/ssh/hooks/test_ssh_async.py
+++ b/providers/ssh/tests/unit/ssh/hooks/test_ssh_async.py
@@ -17,13 +17,21 @@
# under the License.
from __future__ import annotations
+import json
+import warnings
from unittest import mock
import pytest
+from airflow.exceptions import AirflowProviderDeprecationWarning
+from airflow.models import Connection
from airflow.providers.ssh.hooks.ssh import SSHHookAsync
+def _connection(extra: dict, host: str = "test.host") -> Connection:
+ return Connection(conn_id="test_conn", conn_type="ssh", host=host,
extra=json.dumps(extra))
+
+
class TestSSHHookAsync:
def test_init_with_conn_id(self):
"""Test initialization with connection ID."""
@@ -71,6 +79,44 @@ class TestSSHHookAsync:
hook._parse_extras(mock_conn)
assert hook.known_hosts == "none"
+ def test_parse_extras_no_host_key_check_defaults_to_false(self):
+ """A connection that omits ``no_host_key_check`` keeps host key
verification enabled."""
+ hook = SSHHookAsync(ssh_conn_id="test_conn")
+
+ hook._parse_extras(_connection({}))
+ assert hook.known_hosts != "none"
+
+ def test_parse_extras_honours_deprecated_alias(self):
+ """``ignore_hostkey_verification`` keeps working on the async path
too."""
+ hook = SSHHookAsync(ssh_conn_id="test_conn")
+
+ with pytest.warns(AirflowProviderDeprecationWarning,
match="ignore_hostkey_verification"):
+ hook._parse_extras(_connection({"ignore_hostkey_verification":
True}))
+ assert hook.known_hosts == "none"
+
+ def test_parse_extras_canonical_key_wins_over_alias(self):
+ """``no_host_key_check`` takes precedence and suppresses the
deprecation warning."""
+ hook = SSHHookAsync(ssh_conn_id="test_conn")
+
+ with warnings.catch_warnings():
+ warnings.simplefilter("error", AirflowProviderDeprecationWarning)
+ hook._parse_extras(_connection({"no_host_key_check": False,
"ignore_hostkey_verification": True}))
+ assert hook.known_hosts != "none"
+
+ def test_parse_extras_host_key_uses_host_override(self):
+ """The known_hosts entry names the host actually connected to, not the
connection's host."""
+ hook = SSHHookAsync(ssh_conn_id="test_conn", host="override.example")
+
+ hook._parse_extras(_connection({"host_key": "ssh-ed25519 AAAAC3..."},
host="connection.example"))
+ assert hook.known_hosts == b"override.example ssh-ed25519 AAAAC3..."
+
+ def test_parse_extras_bare_host_key_is_rsa(self):
+ """A bare base64 host key is RSA, as on the sync hook, and asyncssh
needs the type."""
+ hook = SSHHookAsync(ssh_conn_id="test_conn")
+
+ hook._parse_extras(_connection({"host_key": "AAAAB3NzaC1yc2E"}))
+ assert hook.known_hosts == b"test.host ssh-rsa AAAAB3NzaC1yc2E"
+
def test_parse_extras_host_key(self):
"""Test parsing host_key from connection extras."""
hook = SSHHookAsync(ssh_conn_id="test_conn")
@@ -151,6 +197,24 @@ class TestSSHHookAsync:
assert call_kwargs["username"] == "testuser"
assert result == mock_ssh_client
+ @mock.patch("airflow.providers.ssh.hooks.ssh.os.path.isfile",
return_value=False)
+ @mock.patch("asyncssh.connect", new_callable=mock.AsyncMock)
+ @mock.patch(
+ "airflow.providers.ssh.hooks.ssh.get_async_connection",
+ new_callable=mock.AsyncMock,
+ )
+ @pytest.mark.asyncio
+ async def test_get_conn_omits_missing_default_known_hosts(
+ self, mock_get_connection, mock_connect, mock_isfile
+ ):
+ mock_get_connection.return_value = Connection(host="test.host",
extra="{}")
+
+ hook = SSHHookAsync(ssh_conn_id="test_conn")
+ await hook._get_conn()
+
+ mock_isfile.assert_called_once_with(hook.known_hosts)
+ assert "known_hosts" not in mock_connect.call_args.kwargs
+
@pytest.mark.asyncio
async def test_run_command(self):
"""Test running a command."""
