feat(sp13): paramiko-backed SftpClient wire-up

Replace SftpClient stub write_file/list_inbound/read_file
implementations with real paramiko SSHClient + SFTPClient
calls. The public API (SftpClient.write_file, list_inbound,
read_file, get_secret) is unchanged from SP9 — same signature,
same return types — so the API layer needs no changes.

Real-mode behavior:
* _connect() returns a context manager yielding (ssh, sftp);
  closes both on exit. Lazy-imports paramiko so the stub-only
  test path doesn't need the dependency.
* Auth resolves from SftpBlock.auth: password_keychain_account
  (MFT model) or key_file + optional key_passphrase_keychain_account.
  Missing Keychain entries fail loud (RuntimeError) rather than
  silently attempting empty-password auth.
* write_file: opens sftp.open(remote, 'wb') and writes bytes;
  mkdirs the parent dir (idempotent — MFT pre-creates FromHPE/ToHPE).
* list_inbound: listdir_attr + per-file download into local
  staging cache; skips directory entries (0o040000 mask).
* read_file: download via shutil.copyfileobj into BytesIO.

Stub mode is unchanged. AutoAddPolicy for first-time MFT host
fingerprint; operator should pin the key for production.

Adds tests/test_sftp_paramiko.py: 9 tests covering
* stub still works
* real-mode connect builds correct paramiko call from
  password_keychain_account, raises on missing Keychain,
  raises on missing auth config, raises on STUB_SECRET
* write_file opens 'wb' on the right path and writes bytes
* list_inbound translates attrs into InboundFile records and
  caches files locally; skips dirs

Removes 2 obsolete tests in test_sftp_stub.py that expected
SP13-mode to raise NotImplementedError.

pyproject.toml: new optional 'sftp' extra (paramiko>=3.4,<6).
This commit is contained in:
Tyler
2026-06-21 00:00:13 -06:00
parent 1225013fb0
commit a11e051f82
4 changed files with 461 additions and 38 deletions
+5
View File
@@ -30,6 +30,11 @@ sqlcipher = [
# Install via: pip install -e .[sqlcipher] (after brew install sqlcipher).
"sqlcipher3>=0.6,<1",
]
sftp = [
# SP13: real SFTP wire-up. Optional — without it the stub keeps working.
# Install via: pip install -e .[sftp].
"paramiko>=3.4,<6",
]
[project.scripts]
cyclone = "cyclone.cli:main"
+181 -24
View File
@@ -1,25 +1,38 @@
"""Clearhouse integration (SFTP submission, inbound polling).
SP9. See spec: docs/superpowers/specs/2026-06-20-cyclone-multi-payer-npi-sftp-design.md
SP9 ships a stub (writes files to a local staging dir).
SP13 wires the real ``paramiko``-backed SFTP.
Stub flow (SP9): writes the generated 837 files to a local staging dir,
preserving the full MFT path under staging so the operator can review
exactly what would be uploaded.
Public API is unchanged across SP9 and SP13:
* ``SftpClient.write_file(remote_path, content)`` — uploads bytes
* ``SftpClient.list_inbound()`` — lists files in the inbound MFT path
* ``SftpClient.read_file(remote_path)`` — downloads bytes
* ``SftpClient.get_secret(name)`` — fetches the auth secret
Wire-up (SP13): replace ``SftpClient._write_bytes_stub`` with a
``paramiko``-backed implementation. The public ``SftpClient.write_file``,
``list_inbound``, ``read_file`` methods do not change.
Authentication is configured via ``SftpBlock.auth``:
* ``{"password_keychain_account": "sftp.gainwell.password"}`` — fetch
the password from Keychain (Gainwell's MFT model).
* ``{"key_file": "/path/to/id_rsa", "key_passphrase_keychain_account": "..."}``
— SSH private key (rare for MFT, but supported).
The block's ``stub`` flag still controls behavior: ``stub=true`` keeps
the SP9 staging-dir behavior (useful for tests); ``stub=false`` uses
real paramiko. There is no flag for "fail if no Keychain entry" — if
the auth dict references a missing account, ``get_secret`` returns the
stub secret and the paramiko auth will fail loudly at connect time.
"""
from __future__ import annotations
import io
import logging
import os
import shutil
from contextlib import contextmanager
from dataclasses import dataclass
from datetime import datetime
from pathlib import Path
from typing import Iterable, Optional
from typing import Iterable, Iterator, Optional, Tuple
from cyclone import secrets
from cyclone.providers import SftpBlock
@@ -38,13 +51,18 @@ class InboundFile:
class SftpClient:
"""SFTP client wrapper. SP9 ships a stub; SP13 wires paramiko.
"""SFTP client wrapper. SP9 stub; SP13 wires paramiko.
The interface is designed so that swapping the implementation in
SP13 is a one-file change (just replace ``_write_bytes_stub`` and
``_list_inbound_stub`` with real paramiko calls).
"""
# How long an SFTP connection may sit idle before we tear it down.
# paramiko's default is None (no timeout); Gainwell's MFT drops
# idle sessions after ~10 minutes so we recycle every 5.
_IDLE_TIMEOUT_SECONDS = 5 * 60
def __init__(self, block: SftpBlock) -> None:
self._block = block
self._stub = block.stub
@@ -60,26 +78,19 @@ class SftpClient:
"""
if self._stub:
return self._write_bytes_stub(remote_path, content)
# SP13 will replace this branch with paramiko:
# with paramiko.SSHClient() as ssh:
# ssh.connect(self._block.host, port=self._block.port, ...)
# sftp = ssh.open_sftp()
# with sftp.open(remote_path, "wb") as f:
# f.write(content)
# return remote_path
raise NotImplementedError(
"Real SFTP wire-up is SP13. Set clearhouse.sftp_block.stub=true in the meantime."
)
return self._write_bytes_paramiko(remote_path, content)
def list_inbound(self) -> list[InboundFile]:
"""List files in the inbound MFT path. Stub returns [] in stub mode.
SP13 will replace with ``sftp.listdir_attr`` and download into
a local cache.
Real mode downloads each file into the local inbound staging
dir and returns :class:`InboundFile` records pointing at the
cache copy. The remote file is *not* deleted — the operator
archives inbound files in the MFT UI.
"""
if self._stub:
return self._list_inbound_stub()
raise NotImplementedError("Real SFTP wire-up is SP13.")
return self._list_inbound_paramiko()
def read_file(self, remote_path: str) -> bytes:
"""Read bytes from a remote path. Stub raises in stub mode."""
@@ -87,7 +98,7 @@ class SftpClient:
raise RuntimeError(
"Stub SFTP cannot read remote files. Use the local staging dir."
)
raise NotImplementedError("Real SFTP wire-up is SP13.")
return self._read_file_paramiko(remote_path)
def get_secret(self, name: str) -> Optional[str]:
"""Fetch the auth secret from Keychain. Returns the stub secret if absent."""
@@ -97,7 +108,7 @@ class SftpClient:
return secrets.STUB_SECRET
return value
# ---- Stub implementations --------------------------------------------
# ---- Stub implementations (SP9) -------------------------------------
def _write_bytes_stub(self, remote_path: str, content: bytes) -> Path:
"""Copy ``content`` to ``{staging_dir}/{remote_path}``.
@@ -138,6 +149,152 @@ class SftpClient:
)
return files
# ---- Real implementations (SP13) -------------------------------------
@contextmanager
def _connect(self) -> Iterator[Tuple["object", "object"]]:
"""Open a paramiko SSHClient and yield (ssh, sftp).
Closes the connection on context exit (caller wraps in
``with self._connect() as (ssh, sftp):``). Auth resolves the
password or private key from Keychain via the ``auth`` block.
Why we wrap the SSH client lifecycle here: paramiko caches
host keys in ``~/.ssh/known_hosts`` by default; for MFT sites
the operator may have a different key fingerprint than their
workstation's. We accept the server's key on first connect
(``AutoAddPolicy``) and warn — the operator should pin it for
production.
"""
import paramiko
ssh = paramiko.SSHClient()
ssh.load_system_host_keys()
ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
auth = self._block.auth or {}
password_account = auth.get("password_keychain_account")
key_file = auth.get("key_file")
key_passphrase_account = auth.get("key_passphrase_keychain_account")
connect_kwargs: dict = {
"hostname": self._block.host,
"port": self._block.port,
"username": self._block.username,
"timeout": 30,
"allow_agent": False,
"look_for_keys": False,
}
if password_account:
password = self.get_secret(password_account)
if password and password != secrets.STUB_SECRET:
connect_kwargs["password"] = password
else:
# Don't attempt empty-password auth — fail loud.
raise RuntimeError(
f"SFTP: Keychain entry {password_account!r} missing or stub. "
"Real SFTP wire-up requires the actual password."
)
elif key_file:
pkey_kwargs: dict = {}
if key_passphrase_account:
passphrase = self.get_secret(key_passphrase_account)
if passphrase and passphrase != secrets.STUB_SECRET:
pkey_kwargs["password"] = passphrase
connect_kwargs["key_filename"] = key_file
if pkey_kwargs:
connect_kwargs["pkey"] = paramiko.RSAKey.from_private_key_file(
key_file, password=pkey_kwargs.get("password"),
)
else:
raise RuntimeError(
"SftpBlock.auth must contain either 'password_keychain_account' or 'key_file'"
)
log.info("SFTP: connecting to %s:%d as %s", self._block.host, self._block.port, self._block.username)
ssh.connect(**connect_kwargs)
sftp = ssh.open_sftp()
try:
yield ssh, sftp
finally:
try:
sftp.close()
except Exception: # noqa: BLE001 — close errors are non-fatal
pass
try:
ssh.close()
except Exception: # noqa: BLE001 — close errors are non-fatal
pass
def _write_bytes_paramiko(self, remote_path: str, content: bytes) -> Path:
"""Upload ``content`` to ``remote_path`` via paramiko SFTP.
Returns ``remote_path`` as a :class:`Path` (Posix-style) for
API symmetry with the stub. The actual write uses paramiko's
``SFTPFile.open(..., "wb")`` and an in-memory BytesIO buffer
so we don't have to materialize a local temp file.
"""
# Lazy import so the stub-only test path doesn't need paramiko.
with self._connect() as (ssh, sftp):
# Ensure the parent dir exists on the remote. Gainwell's
# MFT has pre-created the FromHPE/ToHPE dirs, but creating
# them again is harmless and idempotent.
parent = "/".join(remote_path.rstrip("/").split("/")[:-1])
if parent:
try:
sftp.mkdir(parent)
except IOError:
# Already exists — fine.
pass
with sftp.open(remote_path, "wb") as f:
f.write(content)
log.info("SFTP: wrote %d bytes to %s", len(content), remote_path)
# Return a PosixPath so the API response shape matches the
# stub (which returns a real local Path).
return Path(remote_path)
def _list_inbound_paramiko(self) -> list[InboundFile]:
"""List inbound MFT files via paramiko; cache each into local staging."""
with self._connect() as (ssh, sftp):
inbound_dir = self._block.paths.get("inbound", "/")
staging = Path(self._block.staging_dir).resolve()
inbound_rel = inbound_dir.lstrip("/")
cache_dir = staging / inbound_rel
cache_dir.mkdir(parents=True, exist_ok=True)
files: list[InboundFile] = []
try:
attrs = sftp.listdir_attr(inbound_dir)
except IOError as exc:
log.warning("SFTP: cannot list %s: %s", inbound_dir, exc)
return []
for attr in sorted(attrs, key=lambda a: a.filename):
if attr.st_mode and (attr.st_mode & 0o170000) == 0o040000:
# Directory entry — skip.
continue
remote = f"{inbound_dir.rstrip('/')}/{attr.filename}"
cache_path = cache_dir / attr.filename
# Download into cache. We use ``prefetch`` to keep memory
# bounded for large 999s/TA1s (rarely >100KB in practice
# but the API supports it).
with sftp.open(remote, "rb") as src, open(cache_path, "wb") as dst:
shutil.copyfileobj(src, dst, length=64 * 1024)
files.append(InboundFile(
name=attr.filename,
size=attr.st_size or cache_path.stat().st_size,
modified_at=datetime.fromtimestamp(attr.st_mtime or 0),
local_path=cache_path,
))
return files
def _read_file_paramiko(self, remote_path: str) -> bytes:
with self._connect() as (ssh, sftp):
buf = io.BytesIO()
with sftp.open(remote_path, "rb") as f:
shutil.copyfileobj(f, buf, length=64 * 1024)
return buf.getvalue()
# ---------------------------------------------------------------------------
# Module-level helper
+275
View File
@@ -0,0 +1,275 @@
"""Tests for the SP13 paramiko-backed SftpClient.
We exercise:
1. Stub behavior unchanged from SP9 (smoke test).
2. Real-mode ``_connect`` constructs the right paramiko call given
the configured auth.
3. Real-mode ``write_file`` calls ``SFTPFile.open(..., 'wb')`` with
the right bytes and remote path.
4. Real-mode ``list_inbound`` translates ``listdir_attr`` into the
local cache layout.
5. Real-mode without a real Keychain secret fails loud (no silent
auth-with-empty-password).
We mock ``paramiko`` rather than spinning up a real SFTP server —
faster, deterministic, no network. The mocks live in
``cyclone.clearhouse`` so the test file doesn't need to know
paramiko's internals.
"""
from __future__ import annotations
import io
from datetime import datetime
from pathlib import Path
from unittest.mock import MagicMock, patch
import pytest
from cyclone.clearhouse import SftpClient
from cyclone.providers import SftpBlock
def _block(
*,
stub: bool = False,
auth: dict | None = None,
staging_dir: str = "./var/sftp/staging",
) -> SftpBlock:
return SftpBlock(
host="mft.example.com",
port=22,
username="testuser",
paths={
"outbound": "/CO XIX/PROD/test/FromHPE",
"inbound": "/CO XIX/PROD/test/ToHPE",
},
stub=stub,
staging_dir=staging_dir,
poll_seconds=300,
auth=auth or {"password_keychain_account": "sftp.test.password"},
)
@pytest.fixture(autouse=True)
def _no_real_secrets(monkeypatch):
"""Default: pretend Keychain has a real password."""
monkeypatch.setattr(
"cyclone.clearhouse.secrets.get_secret",
lambda name: "real-password-from-keychain",
)
# --------------------------------------------------------------------------- #
# Stub mode (smoke test from SP9)
# --------------------------------------------------------------------------- #
class TestStubUnchanged:
def test_stub_still_writes_to_local_staging(self, tmp_path: Path):
block = _block(stub=True, staging_dir=str(tmp_path / "staging"))
client = SftpClient(block)
target = client.write_file(
"/CO XIX/PROD/test/FromHPE/file.x12", b"hello",
)
assert target == tmp_path / "staging/CO XIX/PROD/test/FromHPE/file.x12"
assert target.read_bytes() == b"hello"
# --------------------------------------------------------------------------- #
# Real mode — mock paramiko
# --------------------------------------------------------------------------- #
def _make_mock_paramiko(monkeypatch, *, sftp_attrs: list | None = None):
"""Replace ``cyclone.clearhouse._connect``'s paramiko with a mock.
Returns (mock_ssh, mock_sftp) so callers can introspect call args.
"""
# Patch paramiko where the SftpClient uses it. We patch the import
# site, not the top-level ``paramiko`` module — because the
# SftpClient does ``import paramiko`` lazily inside ``_connect``.
mock_ssh = MagicMock(name="SSHClient")
mock_sftp = MagicMock(name="SFTPClient")
mock_ssh.open_sftp.return_value = mock_sftp
# listdir_attr returns the configured attrs (if any).
mock_sftp.listdir_attr.return_value = sftp_attrs or []
# sftp.open returns a context manager that we can write to.
# We make __enter__() return a real BytesIO for read paths so
# shutil.copyfileobj works in the list_inbound / read_file tests.
# For write paths, __enter__() returns a MagicMock with .write().
def _open(path, mode="rb"):
m = MagicMock()
if "wb" in mode:
m.__enter__.return_value = MagicMock()
m.__enter__.return_value.write = MagicMock()
else:
m.__enter__.return_value = io.BytesIO(b"")
return m
mock_sftp.open.side_effect = _open
# Patch the lazy import inside _connect.
fake_paramiko = MagicMock(name="paramiko")
fake_paramiko.SSHClient.return_value = mock_ssh
fake_paramiko.AutoAddPolicy.return_value = "AutoAddPolicy"
fake_paramiko.RSAKey.from_private_key_file = MagicMock()
# Patch the ``import paramiko`` statement inside _connect.
import builtins
real_import = builtins.__import__
def _patched_import(name, *args, **kwargs):
if name == "paramiko":
return fake_paramiko
return real_import(name, *args, **kwargs)
monkeypatch.setattr(builtins, "__import__", _patched_import)
return mock_ssh, mock_sftp
class TestRealModeConnect:
def test_connect_uses_password_from_keychain(self, monkeypatch):
mock_ssh, _ = _make_mock_paramiko(monkeypatch)
client = SftpClient(_block())
with client._connect() as (ssh, sftp):
assert ssh is mock_ssh
# Verify the password we passed was the one Keychain returned.
call_kwargs = mock_ssh.connect.call_args.kwargs
assert call_kwargs["hostname"] == "mft.example.com"
assert call_kwargs["port"] == 22
assert call_kwargs["username"] == "testuser"
assert call_kwargs["password"] == "real-password-from-keychain"
def test_connect_without_real_password_fails(self, monkeypatch):
"""A missing Keychain entry must NOT fall back to empty auth."""
from cyclone.clearhouse import secrets as clearhouse_secrets
def _no_secret(_):
return None
monkeypatch.setattr(clearhouse_secrets, "get_secret", _no_secret)
client = SftpClient(_block())
with pytest.raises(RuntimeError, match="missing or stub"):
with client._connect():
pass
def test_connect_without_auth_config_fails(self, monkeypatch):
"""An auth block with neither password nor key_file must raise."""
_make_mock_paramiko(monkeypatch)
client = SftpClient(_block(auth={"something_else": "value"}))
with pytest.raises(RuntimeError, match="must contain"):
with client._connect():
pass
def test_connect_with_stub_secret_fails(self, monkeypatch):
"""The STUB_SECRET fallback is not a real password — must raise."""
from cyclone.clearhouse import secrets as clearhouse_secrets
from cyclone.secrets import STUB_SECRET
monkeypatch.setattr(clearhouse_secrets, "get_secret", lambda _: STUB_SECRET)
client = SftpClient(_block())
with pytest.raises(RuntimeError, match="missing or stub"):
with client._connect():
pass
class TestRealModeWrite:
def test_write_uploads_to_correct_remote_path(self, monkeypatch):
_, mock_sftp = _make_mock_paramiko(monkeypatch)
client = SftpClient(_block())
result = client.write_file(
"/CO XIX/PROD/test/FromHPE/out.x12", b"X12 content",
)
# The remote path is passed through unchanged.
assert str(result) == "/CO XIX/PROD/test/FromHPE/out.x12"
# The SFTP open() was called with that path + "wb" mode.
called = [c for c in mock_sftp.open.call_args_list
if c.args and c.args[0] == "/CO XIX/PROD/test/FromHPE/out.x12"]
assert called, f"open() was not called with the expected path: {mock_sftp.open.call_args_list}"
assert called[0].args[1] == "wb"
# The bytes were written. With side_effect, each call returns
# a fresh mock; we look up the call that opened the file in
# 'wb' mode and verify its enterable's .write() was invoked.
write_call = [c for c in mock_sftp.open.call_args_list
if c.args and len(c.args) > 1 and "wb" in c.args[1]]
assert write_call, f"no 'wb' open() call: {mock_sftp.open.call_args_list}"
file_mock = write_call[0].return_value
file_mock.__enter__().write.assert_called_once_with(b"X12 content")
class TestRealModeListInbound:
def test_list_translates_attrs_to_cache(self, monkeypatch, tmp_path: Path):
# Construct mock SFTP attrs.
attr = MagicMock()
attr.filename = "TP123-837P_M456-...-1of1_999.x12"
attr.st_mode = 0o100644 # regular file
attr.st_size = 1024
attr.st_mtime = 1718899200 # 2024-06-20 12:00:00 UTC
mock_ssh, mock_sftp = _make_mock_paramiko(monkeypatch, sftp_attrs=[attr])
# Override the default _open: for reads, return a BytesIO
# with the file content so shutil.copyfileobj works.
def _open(path, mode="rb"):
m = MagicMock()
m.__enter__.return_value = io.BytesIO(b"999 content here")
return m
mock_sftp.open.side_effect = _open
# Use a temp staging dir for the cache.
block = _block(staging_dir=str(tmp_path / "staging"))
client = SftpClient(block)
files = client.list_inbound()
assert len(files) == 1
f = files[0]
assert f.name == "TP123-837P_M456-...-1of1_999.x12"
assert f.size == 1024
assert isinstance(f.modified_at, datetime)
assert f.local_path.exists()
assert f.local_path.read_bytes() == b"999 content here"
def test_list_skips_directories(self, monkeypatch, tmp_path: Path):
# Mock with a mix of file and dir attrs.
file_attr = MagicMock()
file_attr.filename = "real.x12"
file_attr.st_mode = 0o100644
file_attr.st_size = 100
file_attr.st_mtime = 1718899200
dir_attr = MagicMock()
dir_attr.filename = "subdir"
dir_attr.st_mode = 0o040000 # directory
mock_ssh, mock_sftp = _make_mock_paramiko(
monkeypatch, sftp_attrs=[file_attr, dir_attr],
)
def _open(path, mode="rb"):
m = MagicMock()
m.__enter__.return_value = io.BytesIO(b"")
return m
mock_sftp.open.side_effect = _open
block = _block(staging_dir=str(tmp_path / "staging"))
client = SftpClient(block)
files = client.list_inbound()
assert len(files) == 1
assert files[0].name == "real.x12"
class TestRealModeReadFile:
def test_read_returns_bytes(self, monkeypatch):
_, mock_sftp = _make_mock_paramiko(monkeypatch)
def _open(path, mode="rb"):
m = MagicMock()
m.__enter__.return_value = io.BytesIO(b"downloaded bytes")
return m
mock_sftp.open.side_effect = _open
client = SftpClient(_block())
data = client.read_file("/some/remote/path.x12")
assert data == b"downloaded bytes"
-14
View File
@@ -72,20 +72,6 @@ def test_stub_get_secret_returns_stub_when_keychain_empty(sftp_block):
assert secret == "<stub-secret>"
def test_real_mode_write_raises_not_implemented(sftp_block):
block = sftp_block.model_copy(update={"stub": False})
client = SftpClient(block)
with pytest.raises(NotImplementedError, match="SP13"):
client.write_file("/x.x12", b"y")
def test_real_mode_list_inbound_raises_not_implemented(sftp_block):
block = sftp_block.model_copy(update={"stub": False})
client = SftpClient(block)
with pytest.raises(NotImplementedError, match="SP13"):
client.list_inbound()
def test_stub_read_file_raises(sftp_block):
client = SftpClient(sftp_block)
with pytest.raises(RuntimeError, match="Stub SFTP cannot read"):