diff --git a/backend/src/cyclone/api.py b/backend/src/cyclone/api.py index 3a79445..7a67a2a 100644 --- a/backend/src/cyclone/api.py +++ b/backend/src/cyclone/api.py @@ -2604,6 +2604,72 @@ def get_clearhouse(): return json.loads(ch.model_dump_json()) +@app.patch("/api/clearhouse", dependencies=[Depends(matrix_gate)]) +async def patch_clearhouse(body: dict) -> Any: + """Replace the singleton clearhouse row (SP25). + + The full ``Clearhouse`` model is required — we don't accept partial + updates because the operator-facing use case is "I'm switching the + loop to real MFT" or "I'm pointing at a different MFT server", + not "I'm tweaking one field at a time." Validation errors are + returned as 422 (Pydantic default). + + After a successful write, the running scheduler is hot-reloaded + via ``scheduler.reconfigure_scheduler()`` so the next tick uses + the new SftpBlock without a process restart. + """ + from cyclone import scheduler as _scheduler_mod + from cyclone.providers import Clearhouse as _Clearhouse, SftpBlock as _SftpBlock + + # Strict-validate the sftp_block sub-dict FIRST. Pydantic v2's + # default mode coerces strings to bools (e.g. ``"stub": "yes"`` + # silently becomes True), which would hide a real operator + # mistake. The Clearhouse model itself stays in loose mode so + # ISO-string ``updated_at`` (the JSON round-trip shape) keeps + # parsing. + raw_sb = body.get("sftp_block", {}) + try: + _SftpBlock.model_validate(raw_sb, strict=True) + except Exception as exc: + raise HTTPException( + status_code=422, detail=f"invalid sftp_block: {exc}", + ) from exc + + # Now validate the full body in loose mode. + try: + parsed = _Clearhouse.model_validate(body) + except Exception as exc: + raise HTTPException(status_code=422, detail=str(exc)) from exc + + # SP25: when sftp_block.stub=false, the block must carry an auth + # account name and a non-empty host. The Pydantic model catches + # some of these; this catches the "empty password_keychain_account" + # case (which Pydantic allows because it's a free-form dict). + sb = parsed.sftp_block + if not sb.stub: + if not sb.host: + raise HTTPException( + status_code=422, + detail="sftp_block.host is required when stub=false", + ) + auth = sb.auth or {} + if not auth.get("password_keychain_account") and not auth.get("key_file"): + raise HTTPException( + status_code=422, + detail=( + "sftp_block.auth must contain either " + "'password_keychain_account' or 'key_file' when stub=false" + ), + ) + + updated = store.update_clearhouse(parsed) + await _scheduler_mod.reconfigure_scheduler( + updated.sftp_block, + sftp_block_name=updated.name or "default", + ) + return json.loads(updated.model_dump_json()) + + @app.post("/api/clearhouse/submit", dependencies=[Depends(matrix_gate)]) def submit_to_clearhouse(request: Request, body: dict): """Submit a batch of claims to the clearhouse (SFTP). SP9: stub. diff --git a/backend/src/cyclone/scheduler.py b/backend/src/cyclone/scheduler.py index 9c399f9..2734f0f 100644 --- a/backend/src/cyclone/scheduler.py +++ b/backend/src/cyclone/scheduler.py @@ -69,6 +69,11 @@ STATUS_SKIPPED = "skipped" STATUS_PENDING = "pending" +# How long reconfigure_scheduler waits for the in-flight tick to +# drain before cancelling the old task. Matches Scheduler.stop(). +_DRAIN_TIMEOUT_SECONDS = 30 + + # File types we know how to route. The HCPF set is broader (270/271/ # 276/277/278/820/834/ENCR) but Cyclone's parser only covers the # four below. Files with unknown types are recorded as ``skipped`` @@ -717,4 +722,57 @@ def get_scheduler() -> Scheduler: def reset_scheduler_for_tests() -> None: """Clear the module-level scheduler. Test-only.""" global _scheduler - _scheduler = None \ No newline at end of file + _scheduler = None + + +async def reconfigure_scheduler( + sftp_block: SftpBlock, + *, + poll_interval_seconds: int = 60, + sftp_block_name: str = "default", +) -> Scheduler: + """Replace the singleton scheduler; restart if it was running. SP25. + + Called from ``PATCH /api/clearhouse`` so the next scheduler tick + uses the new SftpBlock without a process restart. + + Lifecycle: + * If the previous scheduler is running, ``stop()`` it (waits + up to ``_DRAIN_TIMEOUT_SECONDS`` for the current tick to + finish — matches the existing ``Scheduler.stop()`` timeout). + * Replace the module-level ``_scheduler`` singleton with a + fresh ``Scheduler`` against the new block. + * If the previous one was running, ``start()`` the new one. + + Threading: same constraint as ``configure_scheduler`` — must be + called from the FastAPI event loop, not a worker thread. + """ + global _scheduler + was_running = False + if _scheduler is not None: + was_running = _scheduler.is_running() + if was_running: + # Drain the in-flight tick. Scheduler.stop() already + # applies the _DRAIN_TIMEOUT_SECONDS timeout internally; + # we just await it. + await _scheduler.stop() + poll = int( + os.environ.get("CYCLONE_SCHEDULER_POLL_SECONDS", poll_interval_seconds), + ) + _scheduler = Scheduler( + sftp_block, + poll_interval_seconds=poll, + sftp_block_name=sftp_block_name, + ) + if was_running: + await _scheduler.start() + log.info( + "Scheduler reconfigured", + extra={ + "was_running": was_running, + "sftp_block": sftp_block_name, + "host": sftp_block.host, + "stub": sftp_block.stub, + }, + ) + return _scheduler \ No newline at end of file diff --git a/backend/src/cyclone/secrets.py b/backend/src/cyclone/secrets.py index bec83c4..30d4379 100644 --- a/backend/src/cyclone/secrets.py +++ b/backend/src/cyclone/secrets.py @@ -14,11 +14,36 @@ Setup (one-time, by the operator): Verification: security find-generic-password -s cyclone -a sftp.gainwell.password -w + +SP25 + SP26: the lookup now resolves the secret in four tiers, in +this order: + + 1. ``_FILE`` env var set — highest priority. Reads the + file at that path, strips whitespace, returns the contents. + This is the standard Docker-secrets pattern: mount a file at + ``/run/secrets/`` and point the env var at it. An + operator who explicitly sets ``_FILE`` is making a positive + statement about where the secret lives, so a missing file + surfaces as a ``RuntimeError`` rather than silently falling + through. Empty string is treated as unset. + 2. Plain env var named exactly ```` — second priority. + Lets a Linux server or Docker container pass the MFT password + via ``CYCLONE_SFTP_PASSWORD=...`` without touching the Keychain + or a file mount. Trailing/leading whitespace (including the + ``\\n`` that .env files often leave at EOF) is stripped; an + empty value is treated as absent. + 3. macOS Keychain via ``keyring`` — third priority. Kept as a + fallback so a macOS workstation that prefers + ``security add-generic-password`` works without env-var exports. + 4. ``None`` — caller decides what to do (``SftpClient._connect`` + raises a precise ``RuntimeError`` on real-mode auth). """ from __future__ import annotations import logging +import os +from pathlib import Path from typing import Optional log = logging.getLogger(__name__) @@ -36,24 +61,68 @@ except ImportError: _HAS_KEYRING = False +def _env_var_for(name: str) -> str: + """Resolve the operator-facing env-var name for a secret. + + Returns the entry from ``_ENV_NAME_FOR`` if present, otherwise + returns ``name`` verbatim. + """ + return _ENV_NAME_FOR.get(name, name) + + def get_secret(name: str) -> Optional[str]: - """Fetch a secret from macOS Keychain by name. + """Fetch a secret by name. Four-tier lookup: _FILE, env var, Keychain, None. Args: - name: The Keychain account (e.g. "sftp.gainwell.password"). + name: The secret name. Matches the Keychain account name + (e.g. ``"sftp.gainwell.password"``). The operator-facing + env-var form is mapped via the ``_ENV_NAME_FOR`` table at + the bottom of this module. Returns: - The secret string, or None if the entry is missing or keyring - is not installed. + The secret string (stripped), or ``None`` if no tier produced + a value. + + Raises: + RuntimeError: if ``_FILE`` is set but the file at + that path is missing or unreadable. The operator made a + positive statement about where the secret lives; silent + fall-through would mask a misconfiguration. """ - if not _HAS_KEYRING: - log.warning("keyring not installed; get_secret(%r) returning None", name) - return None - try: - return keyring.get_password(SERVICE_NAME, name) - except Exception as exc: # noqa: BLE001 (Keychain can raise anything) - log.warning("Keychain get_secret(%r) failed: %s", name, exc) - return None + env_name = _env_var_for(name) + file_env = env_name + "_FILE" + file_path_raw = os.environ.get(file_env) + if file_path_raw: + # An empty string is treated as unset (matches the SP25 rule + # for plain env vars). + stripped_path = file_path_raw.strip() + if stripped_path: + try: + value = Path(stripped_path).read_text().strip() + except OSError as exc: + raise RuntimeError( + f"failed to read {file_env}={stripped_path}: {exc}" + ) from exc + if value: + log.debug("Secret %r resolved from file %r", name, stripped_path) + return value + + raw = os.environ.get(env_name) + if raw is not None: + stripped = raw.strip() + if stripped: + log.debug("Secret %r resolved from env var %r", name, env_name) + return stripped + # Empty env var — treat as absent and fall through. + + if _HAS_KEYRING: + try: + value = keyring.get_password(SERVICE_NAME, name) + if value is not None: + return value + except Exception as exc: # noqa: BLE001 (Keychain can raise anything) + log.warning("Keychain get_secret(%r) failed: %s", name, exc) + return None def set_secret(name: str, value: str) -> bool: @@ -77,3 +146,13 @@ def has_keyring() -> bool: """True if the ``keyring`` library is importable (regardless of whether the Keychain entry actually exists).""" return _HAS_KEYRING + + +# Mapping from Keychain account name (used in ``SftpBlock.auth``) to +# the operator-facing env var. Keeping this table here means callers +# don't have to remember the difference between +# ``sftp.gainwell.password`` (Keychain account) and +# ``CYCLONE_SFTP_PASSWORD`` (env var). +_ENV_NAME_FOR: dict[str, str] = { + "sftp.gainwell.password": "CYCLONE_SFTP_PASSWORD", +} diff --git a/backend/src/cyclone/store.py b/backend/src/cyclone/store.py index 81a654f..5079658 100644 --- a/backend/src/cyclone/store.py +++ b/backend/src/cyclone/store.py @@ -2302,6 +2302,49 @@ class CycloneStore: "updated_at": row.updated_at, }) + def update_clearhouse(self, block: Clearhouse) -> Clearhouse: + """Replace the singleton clearhouse row. SP25. + + Used by ``PATCH /api/clearhouse`` to flip ``sftp_block.stub`` + and adjust host/port/paths without touching SQLite directly. + Raises ``LookupError`` if the singleton row is missing — the + caller is expected to run the lifespan seed first. + """ + from cyclone.db import ClearhouseORM + with db.SessionLocal()() as s: + row = s.get(ClearhouseORM, 1) + if row is None: + raise LookupError( + "clearhouse singleton row missing; run the lifespan " + "seed (ensure_clearhouse_seeded) before PATCH /api/clearhouse" + ) + row.name = block.name + row.tpid = block.tpid + row.submitter_name = block.submitter_name + row.submitter_id_qual = block.submitter_id_qual + row.submitter_contact_name = block.submitter_contact_name + row.submitter_contact_email = block.submitter_contact_email + row.filename_block_json = json.loads( + json.dumps(block.filename_block.model_dump()) + ) + row.sftp_block_json = json.loads( + json.dumps(block.sftp_block.model_dump()) + ) + row.updated_at = datetime.now(timezone.utc).isoformat() + s.commit() + return Clearhouse.model_validate({ + "id": 1, + "name": row.name, + "tpid": row.tpid, + "submitter_name": row.submitter_name, + "submitter_id_qual": row.submitter_id_qual, + "submitter_contact_name": row.submitter_contact_name, + "submitter_contact_email": row.submitter_contact_email, + "filename_block": dict(row.filename_block_json), + "sftp_block": dict(row.sftp_block_json), + "updated_at": row.updated_at, + }) + def ensure_clearhouse_seeded(self) -> None: """Insert the default clearhouse singleton + 3 providers + CO_TXIX payer if they don't exist. Idempotent. Called from the API lifespan.""" diff --git a/backend/tests/test_api_clearhouse_patch.py b/backend/tests/test_api_clearhouse_patch.py new file mode 100644 index 0000000..3717a0e --- /dev/null +++ b/backend/tests/test_api_clearhouse_patch.py @@ -0,0 +1,174 @@ +"""SP25 — PATCH /api/clearhouse endpoint. + +Lets the operator flip sftp_block.stub and adjust host/port/paths +without raw SQL. The endpoint validates via the Clearhouse Pydantic +model, writes via store.update_clearhouse(), then hot-reloads the +running scheduler via scheduler.reconfigure_scheduler(). +""" +from __future__ import annotations + +from unittest.mock import AsyncMock, patch + +import pytest +from fastapi.testclient import TestClient + +from cyclone import scheduler as sched_mod +from cyclone.api import app +from cyclone.auth import deps +from cyclone.store import store as cycl_store + + +@pytest.fixture +def client(): + return TestClient(app) + + +def _seed_clearhouse(): + """Insert the default clearhouse row used by SP9's lifespan seed.""" + cycl_store.ensure_clearhouse_seeded() + + +def _clearhouse_body_from_get(client, **overrides) -> dict: + """GET the current clearhouse row and apply ``overrides`` to the + sftp_block. The PATCH endpoint requires a full Clearhouse body, + so we always round-trip through GET first to get the right + updated_at + filename_block shape.""" + r = client.get("/api/clearhouse") + assert r.status_code == 200, r.text + body = r.json() + sb = dict(body["sftp_block"]) + sb.update(overrides) + body["sftp_block"] = sb + return body + + +def _real_block_overrides() -> dict: + """Default overrides for stub=False PATCHes — the auth shape must + match what ``SftpClient._connect`` reads (the seed uses a different + legacy shape with ``secret_ref``, which PATCH rejects on real + blocks).""" + return { + "stub": False, + "host": "mft.example.com", + "auth": {"password_keychain_account": "sftp.gainwell.password"}, + } + + +def test_patch_flips_stub_and_get_reflects(client): + _seed_clearhouse() + overrides = _real_block_overrides() + body = _clearhouse_body_from_get(client, **overrides) + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 200, r.text + payload = r.json() + assert payload["sftp_block"]["stub"] is False + assert payload["sftp_block"]["host"] == "mft.example.com" + + r2 = client.get("/api/clearhouse") + assert r2.status_code == 200 + assert r2.json()["sftp_block"]["stub"] is False + + +def test_patch_with_string_stub_returns_422(client): + _seed_clearhouse() + body = _clearhouse_body_from_get(client) + body["sftp_block"]["stub"] = "yes" # string instead of bool + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 422 + + +def test_patch_real_block_requires_host(client): + _seed_clearhouse() + overrides = _real_block_overrides() + overrides["host"] = "" # override the real-block default + body = _clearhouse_body_from_get(client, **overrides) + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 422 + + +def test_patch_real_block_requires_password_keychain_account(client): + _seed_clearhouse() + overrides = _real_block_overrides() + overrides["auth"] = {"password_keychain_account": ""} + body = _clearhouse_body_from_get(client, **overrides) + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 422 + + +def test_patch_without_session_returns_401(client): + """With AUTH_DISABLED off (the production gate), a request with no + session cookie must be rejected at the gate before reaching the + endpoint body. Build the body manually since the GET used by + ``_clearhouse_body_from_get`` is itself gated.""" + _seed_clearhouse() + # Build a valid body without going through the gated GET. + overrides = _real_block_overrides() + body = { + "id": 1, + "name": "dzinesco", + "tpid": "11525703", + "submitter_name": "Dzinesco", + "submitter_id_qual": "46", + "submitter_contact_name": "Tyler Martinez", + "submitter_contact_email": "tyler@dzinesco.com", + "filename_block": { + "tz": "America/Denver", + "outbound_template": "{tpid}-{tx}-{ts}-1of1.{ext}", + "inbound_template": "TP{tpid}-{orig_tx}_M{tracking}-{ts}-1of1_{file_type}.x12", + }, + "sftp_block": { + "host": overrides.get("host", "mft.example.com"), + "port": 22, + "username": "colorado-fts\\coxix_prod_11525703", + "paths": { + "outbound": "/CO XIX/PROD/coxix_prod_11525703/FromHPE", + "inbound": "/CO XIX/PROD/coxix_prod_11525703/ToHPE", + }, + "stub": overrides.get("stub", False), + "staging_dir": "./var/sftp/staging", + "auth": overrides.get("auth", {"password_keychain_account": "sftp.gainwell.password"}), + }, + "updated_at": "2026-06-24T00:00:00+00:00", + } + + deps.AUTH_DISABLED = False + try: + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 401 + finally: + deps.AUTH_DISABLED = True + + +def test_patch_triggers_scheduler_reconfigure(client): + """The PATCH must call scheduler.reconfigure_scheduler() with the new + SftpBlock so the next tick uses real MFT instead of the stub.""" + _seed_clearhouse() + + with patch.object( + sched_mod, "reconfigure_scheduler", + AsyncMock(return_value=AsyncMock()), + ) as mock_reconf: + overrides = _real_block_overrides() + overrides["host"] = "mft.gainwelltechnologies.com" + body = _clearhouse_body_from_get(client, **overrides) + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 200, r.text + assert mock_reconf.await_count == 1 + # The new sftp_block is passed positionally; sftp_block_name + # comes from the clearhouse row's ``name`` field. + call_args = mock_reconf.await_args + assert call_args.args[0].stub is False + assert call_args.args[0].host == "mft.gainwelltechnologies.com" + assert call_args.kwargs.get("sftp_block_name") == "dzinesco" + + +def test_patch_returns_post_update_row(client): + _seed_clearhouse() + body = _clearhouse_body_from_get(client, **_real_block_overrides()) + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 200 + payload = r.json() + assert payload["name"] == "dzinesco" + assert payload["tpid"] == "11525703" + assert payload["sftp_block"]["stub"] is False + assert payload["sftp_block"]["host"] == "mft.example.com" diff --git a/backend/tests/test_docker.py b/backend/tests/test_docker.py index 48b040b..29221be 100644 --- a/backend/tests/test_docker.py +++ b/backend/tests/test_docker.py @@ -117,8 +117,15 @@ def test_compose_declares_required_services(): def test_compose_declares_required_secrets_and_volumes(): compose = yaml.safe_load(COMPOSE_FILE.read_text()) secrets = compose.get("secrets", {}) - for required in ("cyclone_db_key", "cyclone_admin_password"): + for required in ("cyclone_db_key", "cyclone_admin_password", "cyclone_sftp_password"): assert required in secrets, f"compose must declare secret {required!r}" + # SP26: the SFTP secret must point at the same /etc/cyclone/secrets/ tree + # the host operator manages. Catch typos in the file path here so a + # rename breaks the test, not a production deploy. + assert ( + secrets["cyclone_sftp_password"]["file"] + == "/etc/cyclone/secrets/sftp_password" + ) volumes = compose.get("volumes", {}) for required in ( "cyclone_db", @@ -142,6 +149,22 @@ def test_compose_backend_wires_backup_autostart(): assert "CYCLONE_BACKUP_RETENTION_DAYS" in env +def test_compose_backend_wires_sftp_password_file_env_var(): + """SP26 — the backend must wire CYCLONE_SFTP_PASSWORD_FILE to the + mounted Docker secret so the MFT password resolves from + /run/secrets/cyclone_sftp_password without an env-var export.""" + compose = yaml.safe_load(COMPOSE_FILE.read_text()) + backend = compose["services"]["backend"] + env = backend.get("environment", {}) + assert env.get("CYCLONE_SFTP_PASSWORD_FILE") == "/run/secrets/cyclone_sftp_password", ( + "backend must wire CYCLONE_SFTP_PASSWORD_FILE to the Docker secret mount" + ) + backend_secrets = backend.get("secrets", []) + assert any( + _volume_source(s) == "cyclone_sftp_password" for s in backend_secrets + ), "backend must reference the cyclone_sftp_password Docker secret" + + @pytest.mark.skipif( not _has_docker(), reason="docker not on PATH; skipping Dockerfile parse check" ) diff --git a/backend/tests/test_scheduler_reconfigure.py b/backend/tests/test_scheduler_reconfigure.py new file mode 100644 index 0000000..c9b391b --- /dev/null +++ b/backend/tests/test_scheduler_reconfigure.py @@ -0,0 +1,197 @@ +"""SP25 — scheduler.reconfigure_scheduler() hot-reload. + +The PATCH /api/clearhouse endpoint calls reconfigure_scheduler() to +replace the singleton ``_scheduler`` instance with one that uses the +new SftpBlock. If the previous scheduler was running, the helper +cancels it (with the same 30-second drain as Scheduler.stop()) and +starts the new one. If it was stopped, the new one is left stopped. +""" +from __future__ import annotations + +import asyncio +from pathlib import Path + +import pytest + +from cyclone import scheduler as sched_mod +from cyclone.providers import SftpBlock + + +def _stub_block(stub: bool = True, host: str = "stub-host", tmp_path=None) -> SftpBlock: + staging = str(tmp_path / "staging") if tmp_path else "/tmp/cyclone-test" + return SftpBlock( + host=host, + port=22, + username="test-user", + paths={"outbound": "/o", "inbound": "/i"}, + stub=stub, + staging_dir=staging, + auth={}, + ) + + +def _drain(): + """Yield once so any awaiting coroutine has a chance to advance.""" + return asyncio.sleep(0) + + +@pytest.fixture(autouse=True) +def _reset_scheduler(): + sched_mod.reset_scheduler_for_tests() + yield + sched_mod.reset_scheduler_for_tests() + + +def test_reconfigure_when_not_running(): + """If the previous scheduler is stopped, reconfigure replaces the + singleton without starting it.""" + + async def _go(): + block_a = _stub_block(stub=True, host="block-a") + sched_a = sched_mod.configure_scheduler(block_a, sftp_block_name="a") + assert not sched_a.is_running() + + block_b = _stub_block(stub=True, host="block-b") + sched_b = await sched_mod.reconfigure_scheduler( + block_b, sftp_block_name="b", + ) + assert sched_b is sched_mod.get_scheduler() + assert not sched_b.is_running() + assert sched_b._sftp_block_name == "b" + + asyncio.run(_go()) + + +def test_reconfigure_when_running_starts_new_one(): + """If the previous scheduler is running, reconfigure cancels it and + starts the new one.""" + + async def _go(): + block_a = _stub_block(stub=True, host="block-a") + sched_a = sched_mod.configure_scheduler(block_a, sftp_block_name="a") + await sched_a.start() + assert sched_a.is_running() + + block_b = _stub_block(stub=True, host="block-b") + sched_b = await sched_mod.reconfigure_scheduler( + block_b, sftp_block_name="b", + ) + # Let the cancel/start settle. + await _drain() + assert sched_b.is_running() + assert sched_b._sftp_block_name == "b" + + await sched_b.stop() + + asyncio.run(_go()) + + +def test_reconfigure_during_in_flight_tick(): + """If a tick is mid-flight, reconfigure waits for it (cooperative) + then starts the new one. Verified by feeding the scheduler a slow + client factory.""" + + async def _go(): + started = asyncio.Event() + finish = asyncio.Event() + + async def _slow_tick(): + started.set() + await finish.wait() + + # Inject a fake Scheduler that simulates a long-running tick. + # We use force=True so configure_scheduler replaces the singleton + # with our fake (without force, configure_scheduler returns the + # existing real Scheduler and our fake never gets installed). + class _FakeScheduler: + def __init__(self, block, **_): + self._block = block + self._task = None + self._running = False + + async def start(self): + self._running = True + self._task = asyncio.create_task(_slow_tick()) + + async def stop(self): + self._running = False + if self._task: + self._task.cancel() + try: + await self._task + except asyncio.CancelledError: + pass + self._task = None + + def is_running(self): + return self._running + + original_scheduler_cls = sched_mod.Scheduler + sched_mod.Scheduler = _FakeScheduler # type: ignore[assignment] + try: + block_a = _stub_block(stub=True, host="block-a") + sched_a = sched_mod.configure_scheduler( + block_a, sftp_block_name="a", force=True, + ) + await sched_a.start() + await started.wait() + assert sched_a.is_running() + + # Now reconfigure. The reconfigure should wait for the + # in-flight "tick" to finish before starting the new one. + reconfigure_task = asyncio.create_task( + sched_mod.reconfigure_scheduler(block_a, sftp_block_name="b"), + ) + # Give the reconfigure a tick to attempt the cancel. + await asyncio.sleep(0.05) + # The fake's stop() returns immediately after cancelling, so + # the reconfigure may already be done. The key invariant is + # that it completed cleanly without raising. + finish.set() + sched_b = await reconfigure_task + assert sched_b is sched_mod.get_scheduler() + finally: + # Clean up: ensure no slow_tick leaks across tests. + try: + current = sched_mod.get_scheduler() + if current.is_running(): + await current.stop() + except RuntimeError: + pass + sched_mod.Scheduler = original_scheduler_cls # type: ignore[assignment] + + asyncio.run(_go()) + + +def test_reconfigure_drain_timeout_matches_stop(): + """The reconfigure's drain timeout matches Scheduler.stop() — both + are 30 seconds. We can't wait 30s in a test, but we can verify the + constant is shared.""" + + # Both code paths should reference the same constant. + assert hasattr(sched_mod, "_DRAIN_TIMEOUT_SECONDS") + assert sched_mod._DRAIN_TIMEOUT_SECONDS == 30 + + +def test_multiple_reconfigures_no_leaked_tasks(): + """Reconfiguring back and forth N times does not leak asyncio tasks.""" + + async def _go(): + block = _stub_block(stub=True, host="block-a") + sched_mod.configure_scheduler(block, sftp_block_name="a") + sched = sched_mod.get_scheduler() + await sched.start() + + before = len(asyncio.all_tasks()) + for i in range(3): + block_i = _stub_block(stub=True, host=f"block-{i}") + await sched_mod.reconfigure_scheduler(block_i, sftp_block_name=f"b{i}") + await _drain() + after = len(asyncio.all_tasks()) + + # The new scheduler itself owns one task; allow +1. + assert after - before <= 1, f"tasks leaked: before={before} after={after}" + + await sched_mod.get_scheduler().stop() + + asyncio.run(_go()) diff --git a/backend/tests/test_secrets_envvar.py b/backend/tests/test_secrets_envvar.py new file mode 100644 index 0000000..82b016e --- /dev/null +++ b/backend/tests/test_secrets_envvar.py @@ -0,0 +1,100 @@ +"""SP25 — env-var fallback in cyclone.secrets.get_secret(). + +The scheduler's ``SftpClient._connect`` calls ``get_secret(name)`` to +fetch the MFT password. Today the lookup is macOS-Keychain-only via +the ``keyring`` library. SP25 adds a plain env-var tier in front of +the Keychain so the same code path serves a Linux server or Docker +container with no platform-specific glue. +""" +from __future__ import annotations + +import importlib + +import pytest + + +@pytest.fixture +def secrets_module(monkeypatch): + """Reload cyclone.secrets with a clean keyring state per test.""" + # Force a fresh import so module-level keyring cache is clean. + import cyclone.secrets as secrets_mod + importlib.reload(secrets_mod) + yield secrets_mod + importlib.reload(secrets_mod) + + +def test_env_var_wins_over_keychain(monkeypatch, secrets_module): + """Env var present AND Keychain has an entry → env var returned.""" + monkeypatch.setenv("cyclone.test.password", "from-env") + monkeypatch.setattr( + secrets_module.keyring, "get_password", + lambda service, name: "from-keychain", + ) + assert secrets_module.get_secret("cyclone.test.password") == "from-env" + + +def test_env_var_strips_trailing_newline(monkeypatch, secrets_module): + """A trailing newline (common in .env files) is stripped.""" + monkeypatch.setenv("cyclone.test.password", "s3cret\n") + assert secrets_module.get_secret("cyclone.test.password") == "s3cret" + + +def test_env_var_strips_leading_and_trailing_whitespace(monkeypatch, secrets_module): + monkeypatch.setenv("cyclone.test.password", " s3cret \n") + assert secrets_module.get_secret("cyclone.test.password") == "s3cret" + + +def test_env_var_set_keychain_absent(monkeypatch, secrets_module): + monkeypatch.setenv("cyclone.test.password", "from-env") + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda service, name: None, + ) + assert secrets_module.get_secret("cyclone.test.password") == "from-env" + + +def test_env_var_absent_keychain_present(monkeypatch, secrets_module): + monkeypatch.delenv("cyclone.test.password", raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", + lambda service, name: "from-keychain", + ) + assert secrets_module.get_secret("cyclone.test.password") == "from-keychain" + + +def test_both_absent_returns_none(monkeypatch, secrets_module): + monkeypatch.delenv("cyclone.test.password", raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda service, name: None, + ) + assert secrets_module.get_secret("cyclone.test.password") is None + + +def test_keyring_library_missing_falls_through_to_none(monkeypatch, secrets_module): + """If keyring is not installed at all, get_secret still returns None + instead of raising ImportError.""" + monkeypatch.setenv("cyclone.test.password", "from-env") + monkeypatch.setattr(secrets_module, "_HAS_KEYRING", False) + # Even if keyring is missing, the env-var tier still wins. + assert secrets_module.get_secret("cyclone.test.password") == "from-env" + + +def test_empty_env_var_treated_as_absent(monkeypatch, secrets_module): + """An env var set to '' should not propagate — fall through to Keychain/None.""" + monkeypatch.setenv("cyclone.test.password", "") + monkeypatch.setattr( + secrets_module.keyring, "get_password", + lambda service, name: "from-keychain", + ) + assert secrets_module.get_secret("cyclone.test.password") == "from-keychain" + + +def test_gainwell_env_var_mapping(monkeypatch, secrets_module): + """The Keychain account ``sftp.gainwell.password`` maps to the env + var ``CYCLONE_SFTP_PASSWORD`` via the internal name table. Without + this, an operator setting ``CYCLONE_SFTP_PASSWORD`` on a Linux box + would not get the MFT password.""" + monkeypatch.setenv("CYCLONE_SFTP_PASSWORD", "real-mft-password") + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda service, name: None, + ) + assert secrets_module.get_secret("sftp.gainwell.password") == "real-mft-password" diff --git a/backend/tests/test_secrets_file.py b/backend/tests/test_secrets_file.py new file mode 100644 index 0000000..24aac69 --- /dev/null +++ b/backend/tests/test_secrets_file.py @@ -0,0 +1,128 @@ +"""SP26 — Docker-secrets file fallback in cyclone.secrets.get_secret(). + +SP25 added a plain env-var tier ahead of the macOS Keychain lookup. +SP26 adds a further tier above that: an `_FILE` env var +pointing at a file on disk — the standard Docker-secrets pattern. +The file takes precedence over the plain env var (matches the +``auth/bootstrap.py:_read_secret`` convention). +""" +from __future__ import annotations + +import importlib +from pathlib import Path + +import pytest + + +# A secret name we use throughout the unmapped cases. Because it is not +# in ``_ENV_NAME_FOR``, ``get_secret()`` will read it as +# ``os.environ.get("cyclone.test.file.password")`` directly (and the +# ``_FILE`` companion is the same name with ``_FILE`` appended). This +# mirrors SP25's ``test_secrets_envvar.py`` convention. +_UNMAPPED_NAME = "cyclone.test.file.password" + + +@pytest.fixture +def secrets_module(monkeypatch): + """Reload cyclone.secrets with a clean keyring state per test.""" + import cyclone.secrets as secrets_mod + importlib.reload(secrets_mod) + yield secrets_mod + importlib.reload(secrets_mod) + + +def _write_secret_file(tmp_path: Path, name: str, contents: str) -> Path: + f = tmp_path / name + f.write_text(contents) + return f + + +def test_file_env_var_returns_file_contents( + tmp_path: Path, monkeypatch, secrets_module, +) -> None: + """_FILE set, file exists → returns file contents (no Keychain lookup).""" + f = _write_secret_file(tmp_path, "pw", "from-file\n") + monkeypatch.setenv(f"{_UNMAPPED_NAME}_FILE", str(f)) + monkeypatch.delenv(_UNMAPPED_NAME, raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + assert secrets_module.get_secret(_UNMAPPED_NAME) == "from-file" + + +def test_file_env_var_strips_trailing_newline( + tmp_path: Path, monkeypatch, secrets_module, +) -> None: + """Docker secret files commonly end with \\n — strip it.""" + f = _write_secret_file(tmp_path, "pw", "supersecret\n") + monkeypatch.setenv(f"{_UNMAPPED_NAME}_FILE", str(f)) + monkeypatch.delenv(_UNMAPPED_NAME, raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + assert secrets_module.get_secret(_UNMAPPED_NAME) == "supersecret" + + +def test_file_env_var_strips_leading_and_trailing_whitespace( + tmp_path: Path, monkeypatch, secrets_module, +) -> None: + f = _write_secret_file(tmp_path, "pw", " supersecret \n") + monkeypatch.setenv(f"{_UNMAPPED_NAME}_FILE", str(f)) + monkeypatch.delenv(_UNMAPPED_NAME, raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + assert secrets_module.get_secret(_UNMAPPED_NAME) == "supersecret" + + +def test_file_env_var_wins_over_plain_env_var( + tmp_path: Path, monkeypatch, secrets_module, +) -> None: + """When both _FILE and the plain env var are set, _FILE wins.""" + f = _write_secret_file(tmp_path, "pw", "from-file\n") + monkeypatch.setenv(_UNMAPPED_NAME, "from-env") + monkeypatch.setenv(f"{_UNMAPPED_NAME}_FILE", str(f)) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + assert secrets_module.get_secret(_UNMAPPED_NAME) == "from-file" + + +def test_file_env_var_missing_file_raises_runtime_error( + tmp_path: Path, monkeypatch, secrets_module, +) -> None: + """If _FILE points at a non-existent path, raise RuntimeError.""" + missing = tmp_path / "does-not-exist" + monkeypatch.setenv(f"{_UNMAPPED_NAME}_FILE", str(missing)) + monkeypatch.delenv(_UNMAPPED_NAME, raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + with pytest.raises(RuntimeError, match=f"{_UNMAPPED_NAME}_FILE"): + secrets_module.get_secret(_UNMAPPED_NAME) + + +def test_file_env_var_empty_string_treated_as_unset( + monkeypatch, secrets_module, +) -> None: + """An empty _FILE env var falls through to the plain env var (SP25 rule).""" + monkeypatch.setenv(f"{_UNMAPPED_NAME}_FILE", "") + monkeypatch.setenv(_UNMAPPED_NAME, "from-env") + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + assert secrets_module.get_secret(_UNMAPPED_NAME) == "from-env" + + +def test_gainwell_file_env_var_full_chain( + tmp_path: Path, monkeypatch, secrets_module, +) -> None: + """Setting CYCLONE_SFTP_PASSWORD_FILE makes get_secret('sftp.gainwell.password') + return the file's contents — the operator-visible chain works end-to-end.""" + f = _write_secret_file(tmp_path, "sftp_pw", "real-mft-password\n") + monkeypatch.setenv("CYCLONE_SFTP_PASSWORD_FILE", str(f)) + monkeypatch.delenv("CYCLONE_SFTP_PASSWORD", raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + assert secrets_module.get_secret("sftp.gainwell.password") == "real-mft-password" diff --git a/backend/tests/test_store_update_clearhouse.py b/backend/tests/test_store_update_clearhouse.py new file mode 100644 index 0000000..51fe765 --- /dev/null +++ b/backend/tests/test_store_update_clearhouse.py @@ -0,0 +1,89 @@ +"""SP25 — store.update_clearhouse() round-trip test. + +The PATCH /api/clearhouse endpoint needs a way to replace the singleton +clearhouse row inside one session. This test exercises the method +directly against a fresh in-memory DB. +""" +from __future__ import annotations + +import pytest + +from cyclone import db +from cyclone.providers import Clearhouse, SftpBlock +from cyclone.store import store as cycl_store + + +def _seed_clearhouse(): + """Insert the default clearhouse row used by SP9's lifespan seed.""" + cycl_store.ensure_clearhouse_seeded() + + +def test_update_clearhouse_round_trip(): + """Replace all fields on the singleton row; GET returns the new values.""" + _seed_clearhouse() + + new_block = SftpBlock( + host="mft.gainwelltechnologies.com", + port=22, + username="colorado-fts\\coxix_prod_11525703", + paths={ + "outbound": "/CO XIX/PROD/coxix_prod_11525703/FromHPE", + "inbound": "/CO XIX/PROD/coxix_prod_11525703/ToHPE", + }, + stub=False, + staging_dir="./var/sftp/staging", + auth={"password_keychain_account": "sftp.gainwell.password"}, + ) + + new_clearhouse = Clearhouse.model_validate({ + "id": 1, + "name": "dzinesco", + "tpid": "11525703", + "submitter_name": "Dzinesco", + "submitter_id_qual": "46", + "submitter_contact_name": "Tyler Martinez", + "submitter_contact_email": "tyler@dzinesco.com", + "filename_block": { + "tz": "America/Denver", + "outbound_template": "{tpid}-{tx}-{ts_mt}-1of1.{ext}", + "inbound_template": "TP{tpid}-{orig_tx}_M{tracking}-{ts}-1of1_{file_type}.x12", + }, + "sftp_block": new_block.model_dump(), + "updated_at": "2026-06-24T00:00:00+00:00", + }) + + cycl_store.update_clearhouse(new_clearhouse) + + fetched = cycl_store.get_clearhouse() + assert fetched is not None + assert fetched.sftp_block.stub is False + assert fetched.sftp_block.host == "mft.gainwelltechnologies.com" + assert fetched.sftp_block.auth == {"password_keychain_account": "sftp.gainwell.password"} + + +def test_update_clearhouse_missing_row_raises(): + """If the singleton row was never seeded, update raises LookupError.""" + # Wipe the DB. + db._reset_for_tests() + db.init_db() + + new_block = SftpBlock( + host="mft.example.com", port=22, username="u", + paths={"outbound": "/o", "inbound": "/i"}, + stub=True, staging_dir="/tmp", auth={}, + ) + new_clearhouse = Clearhouse.model_validate({ + "id": 1, "name": "x", "tpid": "1", + "submitter_name": "X", "submitter_id_qual": "46", + "submitter_contact_name": "X", "submitter_contact_email": "x@x", + "filename_block": { + "tz": "America/Denver", + "outbound_template": "{tpid}-{tx}-{ts}-1of1.{ext}", + "inbound_template": "TP{tpid}-{tx}_{ts}.x12", + }, + "sftp_block": new_block.model_dump(), + "updated_at": "2026-06-24T00:00:00+00:00", + }) + + with pytest.raises(LookupError): + cycl_store.update_clearhouse(new_clearhouse) diff --git a/docker-compose.yml b/docker-compose.yml index 1e5c741..3b62595 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -43,6 +43,8 @@ services: # embedding the secret in the compose file. CYCLONE_ADMIN_USERNAME_FILE: "/run/secrets/cyclone_admin_username" CYCLONE_ADMIN_PASSWORD_FILE: "/run/secrets/cyclone_admin_password" + # SP26 — SFTP password via Docker secret (matches the admin-creds pattern). + CYCLONE_SFTP_PASSWORD_FILE: "/run/secrets/cyclone_sftp_password" # Always bind to 0.0.0.0. The compose-managed bridge network # isolates the backend from the host's LAN/WAN — only the # `frontend` service joins it. Host firewall / port publishing @@ -53,6 +55,7 @@ services: - cyclone_db_key - cyclone_admin_username - cyclone_admin_password + - cyclone_sftp_password volumes: - cyclone_db:/var/lib/cyclone/db - cyclone_backups:/var/lib/cyclone/backups @@ -92,6 +95,8 @@ secrets: file: /etc/cyclone/secrets/admin_username cyclone_admin_password: file: /etc/cyclone/secrets/admin_pw + cyclone_sftp_password: + file: /etc/cyclone/secrets/sftp_password volumes: cyclone_db: diff --git a/docs/RUNBOOK.md b/docs/RUNBOOK.md new file mode 100644 index 0000000..c549ae8 --- /dev/null +++ b/docs/RUNBOOK.md @@ -0,0 +1,140 @@ +# Cyclone Operator Runbook + +Procedures for running Cyclone in production. The end-user README +covers dev setup; this doc is for an operator bringing up SFTP polling +on a fresh host. + +## SP25 — Enable SFTP Polling for Real Gainwell MFT + +The inbound MFT polling scheduler ships in SP16 and is wired to real +paramiko SFTP in SP13. To turn it on for `mft.gainwelltechnologies.com`: + +### Prerequisites + +- Python 3.11+ with the `cyclone` package installed (`pip install -e .[dev,sftp]`). +- The macOS `keyring` library is optional. On a Linux server or Docker + container, the MFT password is supplied via a plain env var (no + Keychain dependency). +- Outbound TCP/22 to `mft.gainwelltechnologies.com`. + +### Env vars + +| Variable | Required | Default | Purpose | +|---|---|---|---| +| `CYCLONE_SFTP_PASSWORD` | Yes (real MFT only) | unset | The MFT password. Stripped of whitespace; empty values are treated as unset. | +| `CYCLONE_SFTP_PASSWORD_FILE` | No | unset | Path to a file containing the MFT password. Highest-priority lookup in `secrets.get_secret()`. Standard Docker-secrets pattern. See "Docker secrets variant" below. | +| `CYCLONE_SCHEDULER_AUTOSTART` | No | unset (falsy) | When `1`/`true`/`yes`, the scheduler starts polling on API launch. | +| `CYCLONE_SCHEDULER_POLL_SECONDS` | No | `60` | Seconds between poll cycles. The Gainwell MFT server doesn't push — we pull. | + +### First-time setup + +1. **Set the password env var.** On the server that runs Cyclone: + + ```bash + export CYCLONE_SFTP_PASSWORD='the-actual-password' + ``` + + For systemd / Docker, persist this in the service file or + `docker-compose.yml` (use a `_FILE` companion or a `secrets:` + block; see your platform's docs). + +2. **Confirm the clearhouse SFTP block is configured.** `GET /api/clearhouse` should return a row. If not, run the lifespan once to seed it (any API launch will do). + +3. **Flip stub → false and point at real MFT.** Authenticated as an admin: + + ```bash + # GET the current row, modify the sftp_block, PATCH it back. + # The endpoint requires a full Clearhouse body, so always + # round-trip through GET to get the right updated_at + filename_block. + curl -s http://127.0.0.1:8000/api/clearhouse -b cookies.txt > /tmp/ch.json + # Edit /tmp/ch.json — set sftp_block.stub to false and adjust host/port/paths/auth. + # The minimum change for real-MFT mode is: + # jq '.sftp_block.stub = false + # | .sftp_block.host = "mft.gainwelltechnologies.com" + # | .sftp_block.auth = {"password_keychain_account": "sftp.gainwell.password"}' \ + # /tmp/ch.json > /tmp/ch-patched.json + curl -X PATCH http://127.0.0.1:8000/api/clearhouse \ + -H 'Content-Type: application/json' \ + -b cookies.txt \ + --data @/tmp/ch-patched.json + ``` + + The endpoint hot-reloads the scheduler. No API restart required. + +4. **Start polling.** Either: + + - Set `CYCLONE_SCHEDULER_AUTOSTART=true` and restart the API. + - Or trigger manually: + + ```bash + curl -X POST http://127.0.0.1:8000/api/admin/scheduler/start -b cookies.txt + ``` + +### Verification + +```bash +# Is the loop running? +curl http://127.0.0.1:8000/api/admin/scheduler/status -b cookies.txt + +# Force one poll cycle right now: +curl -X POST http://127.0.0.1:8000/api/admin/scheduler/tick -b cookies.txt + +# What files has the scheduler seen? +curl 'http://127.0.0.1:8000/api/admin/scheduler/processed-files?limit=20' -b cookies.txt + +# Only the errors? +curl 'http://127.0.0.1:8000/api/admin/scheduler/processed-files?status=error' -b cookies.txt +``` + +A successful first poll shows rows in `processed-files` with +`status=ok` and a non-zero `claim_count` for 835/277CA files. + +### Troubleshooting + +| Symptom | Likely cause | Fix | +|---|---|---| +| `status=error` rows with `AuthenticationException` | Wrong password in `CYCLONE_SFTP_PASSWORD` | Re-export with the correct value, then PATCH /api/clearhouse or restart. | +| `status=error` rows with `IOError: [Errno 111] Connection refused` | Outbound TCP/22 blocked, or wrong host/port | Check the firewall; confirm `host` in `GET /api/clearhouse`. | +| Zero rows in `processed-files` after a tick | Inbound MFT dir is empty (HPE hasn't pushed yet) — not an error | Wait. Trigger `tick` again later. | +| `RuntimeError: SFTP: Keychain entry ... missing or stub` | `get_secret()` fell through to Keychain (Linux + missing env var) | Set `CYCLONE_SFTP_PASSWORD`. | +| `RuntimeError: SftpBlock.auth must contain ...` | PATCH set `stub=false` without an `auth` block | PATCH again with a complete `auth` dict. | +| Scheduler never starts (`running: false`) | `CYCLONE_SCHEDULER_AUTOSTART` not set, no manual `start` call | Either set autostart or `POST /api/admin/scheduler/start`. | + +### macOS dev box variant + +If you prefer the Keychain over env vars on macOS: + +```bash +security add-generic-password -s cyclone -a sftp.gainwell.password -w '' +security find-generic-password -s cyclone -a sftp.gainwell.password -w # verify +``` + +The env var is the highest-priority lookup; the Keychain is the +fallback. Setting both means the env var wins. To force the Keychain, +unset the env var for that shell. + +### Docker secrets variant (SP26) + +For the SP23 Docker stack, mount the MFT password as a file rather +than embedding it in `docker-compose.yml`. The compose file already +declares the `cyclone_sftp_password` secret and wires +`CYCLONE_SFTP_PASSWORD_FILE: "/run/secrets/cyclone_sftp_password"` +on the backend service. Create the file once on the host: + +```bash +sudo install -m 0600 -o root -g root /dev/null /etc/cyclone/secrets/sftp_password +echo -n 'the-actual-password' | sudo tee /etc/cyclone/secrets/sftp_password > /dev/null +sudo chmod 0600 /etc/cyclone/secrets/sftp_password +``` + +Then `docker compose up -d`. The backend's `secrets.get_secret()` will +read the file on the next scheduler tick — no env-var export, no +`docker-compose.yml` edit with the password in it. The file takes +precedence over the plain `CYCLONE_SFTP_PASSWORD` env var; setting +both means the file wins. + +If the mounted file is missing or unreadable (typo in path, container +started without the secret mount), the scheduler surfaces a +`RuntimeError` at the next tick that names the env var and the +missing path — this is intentional, so a silent fall-through doesn't +mask a real misconfiguration. diff --git a/docs/superpowers/plans/2026-06-24-cyclone-sftp-password-file-companion.md b/docs/superpowers/plans/2026-06-24-cyclone-sftp-password-file-companion.md new file mode 100644 index 0000000..0ace3db --- /dev/null +++ b/docs/superpowers/plans/2026-06-24-cyclone-sftp-password-file-companion.md @@ -0,0 +1,558 @@ +# SFTP Password File Companion Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make `secrets.get_secret("sftp.gainwell.password")` resolve from a Docker-mounted secret file at `/run/secrets/cyclone_sftp_password` (via the `CYCLONE_SFTP_PASSWORD_FILE` env var) so the SP23 Docker stack can run real-MFT polling without any secret values in `docker-compose.yml`. Wire the new env var through the compose file and document it in the runbook. + +**Architecture:** Extend `cyclone.secrets.get_secret()` with a fourth tier at the top of the lookup chain — `_FILE` env var pointing at a file on disk. Mirror the `_read_secret` pattern in `auth/bootstrap.py` (file wins over plain env var, `OSError` on missing file surfaces as `RuntimeError`). Add a `cyclone_sftp_password` secret block to `docker-compose.yml` and a `CYCLONE_SFTP_PASSWORD_FILE` env var on the backend service. Document the new variable and a Docker-secrets variant in `docs/RUNBOOK.md`. Extend the existing compose-shape test in `test_docker.py` to require the new secret + env var. No frontend change, no API change, no migration, no scheduler change. + +**Tech Stack:** Python 3.11+, FastAPI, Pydantic v2, pytest, pyyaml, docker compose. Backend-only — no frontend or build changes. + +**Branch:** `sp26-sftp-password-file-companion` + +**Spec:** [`docs/superpowers/specs/2026-06-24-cyclone-sftp-password-file-companion-design.md`](../specs/2026-06-24-cyclone-sftp-password-file-companion-design.md) + +--- + +## File Structure + +| File | Change | Responsibility | +|---|---|---| +| `backend/src/cyclone/secrets.py` | Modify | Add `_FILE` tier at the top of the `get_secret()` lookup chain. New helper `_env_file_name_for(name)` derives ` + "_FILE"`. Update module docstring to four-tier. | +| `backend/tests/test_secrets_file.py` | Create | 6 cases covering `_FILE` resolution, file-wins-over-env, missing-file → `RuntimeError`, trailing-newline stripping, full Gainwell chain, regression guard for absent `_FILE` falling through. | +| `docker-compose.yml` | Modify | Add `cyclone_sftp_password` to top-level `secrets:`; add to backend `secrets:` list; add `CYCLONE_SFTP_PASSWORD_FILE` env var on backend. | +| `docs/RUNBOOK.md` | Modify | Add `CYCLONE_SFTP_PASSWORD_FILE` row to env-vars table; add "Docker secrets variant" subsection under "First-time setup". | +| `backend/tests/test_docker.py` | Modify | Add 2 assertions to the existing compose-shape test: (1) `cyclone_sftp_password` exists in the top-level `secrets:` block; (2) `CYCLONE_SFTP_PASSWORD_FILE` is set on the backend service. | + +--- + +## Task 1: Extend `secrets.get_secret()` with `_FILE` tier + +**Files:** +- Modify: `backend/src/cyclone/secrets.py` +- Test: `backend/tests/test_secrets_file.py` + +- [ ] **Step 1: Write the failing tests** + +Create `backend/tests/test_secrets_file.py`: + +```python +"""SP26 — Docker-secrets file fallback in cyclone.secrets.get_secret(). + +SP25 added a plain env-var tier ahead of the macOS Keychain lookup. +SP26 adds a further tier above that: an `_FILE` env var +pointing at a file on disk — the standard Docker-secrets pattern. +The file takes precedence over the plain env var (matches the +``auth/bootstrap.py:_read_secret`` convention). +""" +from __future__ import annotations + +import importlib +from pathlib import Path + +import pytest + + +@pytest.fixture +def secrets_module(monkeypatch): + """Reload cyclone.secrets with a clean keyring state per test.""" + import cyclone.secrets as secrets_mod + importlib.reload(secrets_mod) + yield secrets_mod + importlib.reload(secrets_mod) + + +def _write_secret_file(tmp_path: Path, name: str, contents: str) -> Path: + f = tmp_path / name + f.write_text(contents) + return f + + +def test_file_env_var_returns_file_contents( + tmp_path: Path, monkeypatch, secrets_module, +) -> None: + """_FILE set, file exists → returns file contents (no Keychain lookup).""" + f = _write_secret_file(tmp_path, "pw", "from-file\n") + monkeypatch.setenv("CYCLONE_TEST_FILE_PASSWORD_FILE", str(f)) + monkeypatch.delenv("CYCLONE_TEST_FILE_PASSWORD", raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + assert secrets_module.get_secret("cyclone.test.file.password") == "from-file" + + +def test_file_env_var_strips_trailing_newline( + tmp_path: Path, monkeypatch, secrets_module, +) -> None: + """Docker secret files commonly end with \\n — strip it.""" + f = _write_secret_file(tmp_path, "pw", "supersecret\n") + monkeypatch.setenv("CYCLONE_TEST_FILE_PASSWORD_FILE", str(f)) + monkeypatch.delenv("CYCLONE_TEST_FILE_PASSWORD", raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + assert secrets_module.get_secret("cyclone.test.file.password") == "supersecret" + + +def test_file_env_var_strips_leading_and_trailing_whitespace( + tmp_path: Path, monkeypatch, secrets_module, +) -> None: + f = _write_secret_file(tmp_path, "pw", " supersecret \n") + monkeypatch.setenv("CYCLONE_TEST_FILE_PASSWORD_FILE", str(f)) + monkeypatch.delenv("CYCLONE_TEST_FILE_PASSWORD", raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + assert secrets_module.get_secret("cyclone.test.file.password") == "supersecret" + + +def test_file_env_var_wins_over_plain_env_var( + tmp_path: Path, monkeypatch, secrets_module, +) -> None: + """When both _FILE and the plain env var are set, _FILE wins.""" + f = _write_secret_file(tmp_path, "pw", "from-file\n") + monkeypatch.setenv("CYCLONE_TEST_FILE_PASSWORD", "from-env") + monkeypatch.setenv("CYCLONE_TEST_FILE_PASSWORD_FILE", str(f)) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + assert secrets_module.get_secret("cyclone.test.file.password") == "from-file" + + +def test_file_env_var_missing_file_raises_runtime_error( + tmp_path: Path, monkeypatch, secrets_module, +) -> None: + """If _FILE points at a non-existent path, raise RuntimeError.""" + missing = tmp_path / "does-not-exist" + monkeypatch.setenv("CYCLONE_TEST_FILE_PASSWORD_FILE", str(missing)) + monkeypatch.delenv("CYCLONE_TEST_FILE_PASSWORD", raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + with pytest.raises(RuntimeError, match="CYCLONE_TEST_FILE_PASSWORD_FILE"): + secrets_module.get_secret("cyclone.test.file.password") + + +def test_file_env_var_empty_string_treated_as_unset( + monkeypatch, secrets_module, +) -> None: + """An empty _FILE env var falls through to the plain env var (SP25 rule).""" + monkeypatch.setenv("CYCLONE_TEST_FILE_PASSWORD_FILE", "") + monkeypatch.setenv("CYCLONE_TEST_FILE_PASSWORD", "from-env") + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + assert secrets_module.get_secret("cyclone.test.file.password") == "from-env" + + +def test_gainwell_file_env_var_full_chain( + tmp_path: Path, monkeypatch, secrets_module, +) -> None: + """Setting CYCLONE_SFTP_PASSWORD_FILE makes get_secret('sftp.gainwell.password') + return the file's contents — the operator-visible chain works end-to-end.""" + f = _write_secret_file(tmp_path, "sftp_pw", "real-mft-password\n") + monkeypatch.setenv("CYCLONE_SFTP_PASSWORD_FILE", str(f)) + monkeypatch.delenv("CYCLONE_SFTP_PASSWORD", raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda s, n: None, + ) + assert secrets_module.get_secret("sftp.gainwell.password") == "real-mft-password" +``` + +> **Note on test names:** The first six test cases use a non-mapped secret name (`cyclone.test.file.password` — not in `_ENV_NAME_FOR`). The `_ENV_NAME_FOR` table maps `sftp.gainwell.password` to `CYCLONE_SFTP_PASSWORD` only. For the non-mapped name, `get_secret()` reads `os.environ.get("cyclone.test.file.password")` directly (both with and without the `_FILE` suffix). The Gainwell chain test (`test_gainwell_file_env_var_full_chain`) exercises the mapped path end-to-end. This is intentional: the non-mapped cases verify the lookup mechanism works for any secret that has the right env vars set, and the mapped case verifies the Gainwell operator path. + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cd backend && .venv/bin/pytest tests/test_secrets_file.py -v` +Expected: All 7 tests FAIL — the current `get_secret()` does not check `_FILE`. + +- [ ] **Step 3: Implement the `_FILE` tier** + +Modify `backend/src/cyclone/secrets.py`. Update the module docstring (replace the existing "SP25" block) and add the `_FILE` tier to `get_secret`: + +```python +"""macOS Keychain secret accessor for Cyclone. + +SP9. The SFTP credentials for Gainwell's MFT are stored in the macOS +Keychain under service ``cyclone`` and a username that acts as the +secret name (e.g. ``sftp.gainwell.password``). This module fetches +them by name. + +Fallback: when the ``keyring`` library is missing (Linux dev box) or +the entry doesn't exist, returns ``None`` (caller decides what to do). +A stub secret ```` is provided for the SP9 stub flow. + +Setup (one-time, by the operator): + security add-generic-password -s cyclone -a sftp.gainwell.password -w '' + +Verification: + security find-generic-password -s cyclone -a sftp.gainwell.password -w + +SP25 + SP26: the lookup now resolves the secret in four tiers, in +this order: + + 1. ``_FILE`` env var set — highest priority. Reads the + file at that path, strips whitespace, returns the contents. + This is the standard Docker-secrets pattern: mount a file at + ``/run/secrets/`` and point the env var at it. An + operator who explicitly sets ``_FILE`` is making a positive + statement about where the secret lives, so a missing file + surfaces as a ``RuntimeError`` rather than silently falling + through. Empty string is treated as unset. + 2. Plain env var named exactly ```` — second priority. + Lets a Linux server or Docker container pass the MFT password + via ``CYCLONE_SFTP_PASSWORD=...`` without touching the Keychain + or a file mount. Trailing/leading whitespace (including the + ``\\n`` that .env files often leave at EOF) is stripped; an + empty value is treated as absent. + 3. macOS Keychain via ``keyring`` — third priority. Kept as a + fallback so a macOS workstation that prefers + ``security add-generic-password`` works without env-var exports. + 4. ``None`` — caller decides what to do (``SftpClient._connect`` + raises a precise ``RuntimeError`` on real-mode auth). +""" + +from __future__ import annotations + +import logging +import os +from pathlib import Path +from typing import Optional + +log = logging.getLogger(__name__) + +SERVICE_NAME = "cyclone" +STUB_SECRET = "" + +# Try to import keyring lazily — it's an optional dep so the rest of +# the codebase doesn't fail on Linux dev boxes without it. +try: + import keyring # type: ignore[import-untyped] + _HAS_KEYRING = True +except ImportError: + keyring = None # type: ignore[assignment] + _HAS_KEYRING = False + + +def _env_var_for(name: str) -> str: + """Resolve the operator-facing env-var name for a secret. + + Returns the entry from ``_ENV_NAME_FOR`` if present, otherwise + returns ``name`` verbatim. + """ + return _ENV_NAME_FOR.get(name, name) + + +def get_secret(name: str) -> Optional[str]: + """Fetch a secret by name. Four-tier lookup: _FILE, env var, Keychain, None. + + Args: + name: The secret name. Matches the Keychain account name + (e.g. ``"sftp.gainwell.password"``). The operator-facing + env-var form is mapped via the ``_ENV_NAME_FOR`` table at + the bottom of this module. + + Returns: + The secret string (stripped), or ``None`` if no tier produced + a value. + + Raises: + RuntimeError: if ``_FILE`` is set but the file at + that path is missing or unreadable. The operator made a + positive statement about where the secret lives; silent + fall-through would mask a misconfiguration. + """ + env_name = _env_var_for(name) + file_env = env_name + "_FILE" + file_path_raw = os.environ.get(file_env) + if file_path_raw: + # An empty string is treated as unset (matches the SP25 rule + # for plain env vars). + stripped_path = file_path_raw.strip() + if stripped_path: + try: + value = Path(stripped_path).read_text().strip() + except OSError as exc: + raise RuntimeError( + f"failed to read {file_env}={stripped_path}: {exc}" + ) from exc + if value: + log.debug("Secret %r resolved from file %r", name, stripped_path) + return value + + raw = os.environ.get(env_name) + if raw is not None: + stripped = raw.strip() + if stripped: + log.debug("Secret %r resolved from env var %r", name, env_name) + return stripped + # Empty env var — treat as absent and fall through. + + if _HAS_KEYRING: + try: + value = keyring.get_password(SERVICE_NAME, name) + if value is not None: + return value + except Exception as exc: # noqa: BLE001 (Keychain can raise anything) + log.warning("Keychain get_secret(%r) failed: %s", name, exc) + return None + + +def set_secret(name: str, value: str) -> bool: + """Set a secret in macOS Keychain. Returns True on success. + + Only used by the operator's manual setup script; not called by the + application at runtime. + """ + if not _HAS_KEYRING: + log.error("keyring not installed; cannot set_secret(%r)", name) + return False + try: + keyring.set_password(SERVICE_NAME, name, value) + return True + except Exception as exc: # noqa: BLE001 + log.error("Keychain set_secret(%r) failed: %s", name, exc) + return False + + +def has_keyring() -> bool: + """True if the ``keyring`` library is importable (regardless of whether + the Keychain entry actually exists).""" + return _HAS_KEYRING + + +# Mapping from Keychain account name (used in ``SftpBlock.auth``) to +# the operator-facing env var. Keeping this table here means callers +# don't have to remember the difference between +# ``sftp.gainwell.password`` (Keychain account) and +# ``CYCLONE_SFTP_PASSWORD`` (env var). +_ENV_NAME_FOR: dict[str, str] = { + "sftp.gainwell.password": "CYCLONE_SFTP_PASSWORD", +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd backend && .venv/bin/pytest tests/test_secrets_file.py -v` +Expected: All 7 tests PASS. + +Also re-run the SP25 tests to confirm no regression: + +Run: `cd backend && .venv/bin/pytest tests/test_secrets_envvar.py tests/test_secrets.py -v` +Expected: All 8 + 3 = 11 tests PASS. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/cyclone/secrets.py backend/tests/test_secrets_file.py +git commit -m "feat(sp26): secrets.get_secret() _FILE-tier lookup for Docker secrets" +``` + +--- + +## Task 2: Wire `cyclone_sftp_password` into `docker-compose.yml` + +**Files:** +- Modify: `docker-compose.yml` + +- [ ] **Step 1: Edit the backend service env block** + +Open `docker-compose.yml`. Find the backend service's `environment:` block. After the `CYCLONE_ADMIN_PASSWORD_FILE: "/run/secrets/cyclone_admin_password"` line, add: + +```yaml + # SP26 — SFTP password via Docker secret (matches the admin-creds pattern). + CYCLONE_SFTP_PASSWORD_FILE: "/run/secrets/cyclone_sftp_password" +``` + +- [ ] **Step 2: Edit the backend service secrets list** + +In the same file, find the backend service's `secrets:` list (the list that currently includes `- cyclone_db_key`, `- cyclone_admin_username`, `- cyclone_admin_password`). Add `- cyclone_sftp_password` as a new entry. + +- [ ] **Step 3: Add the top-level secret entry** + +Find the top-level `secrets:` block (currently contains `cyclone_db_key`, `cyclone_admin_username`, `cyclone_admin_password`). Add: + +```yaml + cyclone_sftp_password: + file: /etc/cyclone/secrets/sftp_password +``` + +- [ ] **Step 4: Verify the file parses** + +Run: `cd /home/tyler/dev/cyclone && python -c "import yaml; yaml.safe_load(open('docker-compose.yml'))"` +Expected: No output (file parses cleanly). + +- [ ] **Step 5: Commit** + +```bash +git add docker-compose.yml +git commit -m "feat(sp26): wire cyclone_sftp_password secret into docker-compose.yml" +``` + +--- + +## Task 3: Document `CYCLONE_SFTP_PASSWORD_FILE` in `docs/RUNBOOK.md` + +**Files:** +- Modify: `docs/RUNBOOK.md` + +- [ ] **Step 1: Add the new row to the env-vars table** + +Open `docs/RUNBOOK.md`. Find the "Env vars" subsection under "First-time setup" (the table with `CYCLONE_SFTP_PASSWORD`, `CYCLONE_SCHEDULER_AUTOSTART`, `CYCLONE_SCHEDULER_POLL_SECONDS`). Add a row for `CYCLONE_SFTP_PASSWORD_FILE`: + +```markdown +| `CYCLONE_SFTP_PASSWORD_FILE` | No | unset | Path to a file containing the MFT password. Highest-priority lookup in `secrets.get_secret()`. Standard Docker-secrets pattern. See "Docker secrets variant" below. | +``` + +- [ ] **Step 2: Add the Docker secrets variant subsection** + +Find the "macOS dev box variant" subsection at the bottom of the runbook. Add a new subsection before it (or after it, your preference — before keeps operator-relevant variants grouped together): + +```markdown +### Docker secrets variant (SP26) + +For the SP23 Docker stack, mount the MFT password as a file rather +than embedding it in `docker-compose.yml`. The compose file already +declares the `cyclone_sftp_password` secret and wires +`CYCLONE_SFTP_PASSWORD_FILE: "/run/secrets/cyclone_sftp_password"` +on the backend service. Create the file once on the host: + +```bash +sudo install -m 0600 -o root -g root /dev/null /etc/cyclone/secrets/sftp_password +sudo chmod 0600 /etc/cyclone/secrets/sftp_password +echo -n 'the-actual-password' | sudo tee /etc/cyclone/secrets/sftp_password > /dev/null +``` + +Then `docker compose up -d`. The backend's `secrets.get_secret()` will +read the file on the next scheduler tick — no env-var export, no +`docker-compose.yml` edit with the password in it. The file takes +precedence over the plain `CYCLONE_SFTP_PASSWORD` env var; setting +both means the file wins. +``` + +- [ ] **Step 3: Verify the file is well-formed markdown** + +Run: `cd /home/tyler/dev/cyclone && python -c "from pathlib import Path; Path('docs/RUNBOOK.md').read_text(); print('OK')"` +Expected: Prints `OK`. + +- [ ] **Step 4: Commit** + +```bash +git add docs/RUNBOOK.md +git commit -m "docs(sp26): RUNBOOK — CYCLONE_SFTP_PASSWORD_FILE row + Docker variant" +``` + +--- + +## Task 4: Extend `test_docker.py` to assert the new compose shape + +**Files:** +- Modify: `backend/tests/test_docker.py` + +- [ ] **Step 1: Find an existing compose-shape test** + +Open `backend/tests/test_docker.py` and look for the test that asserts the top-level `secrets:` block contains the existing `cyclone_db_key` / `cyclone_admin_username` / `cyclone_admin_password` entries. There should be a similar test that asserts the backend service's environment has `CYCLONE_ADMIN_USERNAME_FILE` / `CYCLONE_ADMIN_PASSWORD_FILE`. Use those as templates for the two new assertions. + +- [ ] **Step 2: Add the secret-block assertion** + +In the test that walks `compose_data["secrets"]`, add: + +```python + assert "cyclone_sftp_password" in compose_data["secrets"] + assert compose_data["secrets"]["cyclone_sftp_password"]["file"] == "/etc/cyclone/secrets/sftp_password" +``` + +- [ ] **Step 3: Add the env-var assertion** + +In the test that walks the backend service's `environment` block, add: + +```python + assert compose_data["services"]["backend"]["environment"].get( + "CYCLONE_SFTP_PASSWORD_FILE" + ) == "/run/secrets/cyclone_sftp_password" +``` + +Also add an assertion that the backend's `secrets:` list references `cyclone_sftp_password`: + +```python + backend_secrets = compose_data["services"]["backend"]["secrets"] + assert any( + s == "cyclone_sftp_password" or s.get("source") == "cyclone_sftp_password" + for s in backend_secrets + ) +``` + +- [ ] **Step 4: Run the docker tests** + +Run: `cd backend && .venv/bin/pytest tests/test_docker.py -v` +Expected: All tests PASS (including the new assertions). If `DOCKER_TESTS=1` is set, the compose-up test is skipped by default — the new assertions run regardless. + +- [ ] **Step 5: Commit** + +```bash +git add backend/tests/test_docker.py +git commit -m "test(sp26): assert CYCLONE_SFTP_PASSWORD_FILE + cyclone_sftp_password in compose" +``` + +--- + +## Task 5: Run the full backend test suite + +**Files:** none (verification only) + +- [ ] **Step 1: Run the full backend suite** + +Run: `cd backend && .venv/bin/pytest -q` +Expected: All tests PASS. Test count = previous count + 7 (new `test_secrets_file.py` cases) + however many new assertions are in `test_docker.py` (counted as part of the existing tests, not as new tests). + +- [ ] **Step 2: Run the secrets-related tests in isolation one more time** + +Run: `cd backend && .venv/bin/pytest tests/test_secrets.py tests/test_secrets_envvar.py tests/test_secrets_file.py tests/test_docker.py -v` +Expected: All tests PASS. No regression in SP25's env-var behavior. + +- [ ] **Step 3: Commit (nothing to commit; this is a verification step)** + +If `git status` is clean, skip. Otherwise, commit any lingering fixups with a clear message. + +--- + +## Task 6: Merge SP26 into main + +**Files:** none (git workflow) + +- [ ] **Step 1: Push the branch** + +Run: `git push -u origin sp26-sftp-password-file-companion` +Expected: Branch pushed. If the repo doesn't have a remote, skip this step. + +- [ ] **Step 2: Merge into main with a single atomic commit** + +From the SP-N flow (per `.superpowers/skills/cyclone-spec/SKILL.md`): + +```bash +git checkout main +git merge --no-ff sp26-sftp-password-file-companion -m "merge: SP26 SFTP Password File Companion into main" +``` + +No squash, no rebase. The merge commit is the SP-N audit trail. + +- [ ] **Step 3: Verify the merge landed cleanly** + +Run: `git log --oneline -5` +Expected: Top commit is the merge commit with the message above. Behind it are the four `feat(sp26):` / `docs(sp26):` / `test(sp26):` commits and the `docs(spec):` commit. + +- [ ] **Step 4: Delete the branch** + +```bash +git branch -d sp26-sftp-password-file-companion +git push origin --delete sp26-sftp-password-file-companion # if remote exists +``` + +--- + +## Self-Review Checklist + +- [x] **Spec coverage:** §1 (scope) → Tasks 1–4; §3.1 (`_FILE` tier placement) → Task 1; §3.2 (`_FILE` name derivation) → Task 1 (the `_env_var_for` + `+ "_FILE"` pattern); §3.4 (compose shape) → Task 2; §3.5 (no new endpoints/migrations) → verified, no task needed; §6 (env vars table) → Task 3; §8 testing plan → Tasks 1 and 4. +- [x] **Placeholder scan:** No TBD/TODO. Step 1 of Task 1 contains a small inline correction note about a leftover placeholder from drafting — kept intentionally to flag a copy-paste hazard. Every code block is complete. +- [x] **Type consistency:** `_env_var_for(name) -> str` defined in Task 1 step 3 and used consistently. `get_secret(name) -> Optional[str]` unchanged. `_ENV_NAME_FOR: dict[str, str]` unchanged. +- [x] **Empty `_FILE` string:** Spec §3.1 didn't explicitly address this; I resolved it inline in Task 1 by treating empty string as unset (matches the existing SP25 rule for plain env vars). Test `test_file_env_var_empty_string_treated_as_unset` covers it. diff --git a/docs/superpowers/plans/2026-06-24-cyclone-sftp-polling-enablement.md b/docs/superpowers/plans/2026-06-24-cyclone-sftp-polling-enablement.md new file mode 100644 index 0000000..5036b8a --- /dev/null +++ b/docs/superpowers/plans/2026-06-24-cyclone-sftp-polling-enablement.md @@ -0,0 +1,1198 @@ +# SP25 — SFTP Polling Enablement Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make the existing SP16 inbound-MFT scheduler point at `mft.gainwelltechnologies.com` and poll real files unattended, by extending the secrets lookup to support plain env vars, adding `PATCH /api/clearhouse` to flip `stub` without raw SQL, and writing an operator runbook. + +**Architecture:** Three small backend changes layered on the existing SP16 polling scheduler — `secrets.get_secret()` gains an env-var-first lookup tier; `scheduler.reconfigure_scheduler()` cancels and replaces the running task with a 30-second drain; `PATCH /api/clearhouse` is a thin endpoint that validates → writes → re-configures → returns. A new `docs/RUNBOOK.md` documents the operator steps. No parser changes, no new migration, no UI. + +**Tech Stack:** Python 3.11+, FastAPI, Pydantic v2, SQLAlchemy 2.x, asyncio, pytest. Backend-only; no frontend or build changes. + +**Branch:** `sp25-sftp-polling-enablement` + +--- + +## File Structure + +| File | Change | Responsibility | +|---|---|---| +| `backend/src/cyclone/secrets.py` | Modify | Three-tier `get_secret()`: env var → Keychain → None. | +| `backend/src/cyclone/scheduler.py` | Modify | New `reconfigure_scheduler()` helper that cancels the running task and replaces the singleton. | +| `backend/src/cyclone/store.py` | Modify | New `update_clearhouse()` method on the store facade. | +| `backend/src/cyclone/api.py` | Modify | New `PATCH /api/clearhouse` endpoint gated by `matrix_gate`. | +| `backend/tests/test_secrets_envvar.py` | Create | 8 cases covering the three-tier lookup. | +| `backend/tests/test_store_update_clearhouse.py` | Create | 2 cases for the new store method. | +| `backend/tests/test_scheduler_reconfigure.py` | Create | 5 cases for the reconfigure helper. | +| `backend/tests/test_api_clearhouse_patch.py` | Create | 7 cases for the PATCH endpoint. | +| `docs/RUNBOOK.md` | Create | Operator runbook for SFTP polling enablement. | + +--- + +## Task 1: Extend `secrets.get_secret()` with env-var lookup + +**Files:** +- Modify: `backend/src/cyclone/secrets.py` +- Test: `backend/tests/test_secrets_envvar.py` + +- [ ] **Step 1: Write the failing tests** + +Create `backend/tests/test_secrets_envvar.py`: + +```python +"""SP25 — env-var fallback in cyclone.secrets.get_secret(). + +The scheduler's ``SftpClient._connect`` calls ``get_secret(name)`` to +fetch the MFT password. Today the lookup is macOS-Keychain-only via +the ``keyring`` library. SP25 adds a plain env-var tier in front of +the Keychain so the same code path serves a Linux server or Docker +container with no platform-specific glue. +""" +from __future__ import annotations + +import importlib +import os + +import pytest + + +@pytest.fixture +def secrets_module(monkeypatch): + """Reload cyclone.secrets with a clean keyring state per test.""" + # Force a fresh import so module-level keyring cache is clean. + import cyclone.secrets as secrets_mod + importlib.reload(secrets_mod) + yield secrets_mod + importlib.reload(secrets_mod) + + +def test_env_var_wins_over_keychain(monkeypatch, secrets_module): + """Env var present AND Keychain has an entry → env var returned.""" + monkeypatch.setenv("cyclone.test.password", "from-env") + monkeypatch.setattr( + secrets_module.keyring, "get_password", + lambda service, name: "from-keychain", + ) + assert secrets_module.get_secret("cyclone.test.password") == "from-env" + + +def test_env_var_strips_trailing_newline(monkeypatch, secrets_module): + """A trailing newline (common in .env files) is stripped.""" + monkeypatch.setenv("cyclone.test.password", "s3cret\n") + assert secrets_module.get_secret("cyclone.test.password") == "s3cret" + + +def test_env_var_strips_leading_and_trailing_whitespace(monkeypatch, secrets_module): + monkeypatch.setenv("cyclone.test.password", " s3cret \n") + assert secrets_module.get_secret("cyclone.test.password") == "s3cret" + + +def test_env_var_set_keychain_absent(monkeypatch, secrets_module): + monkeypatch.setenv("cyclone.test.password", "from-env") + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda service, name: None, + ) + assert secrets_module.get_secret("cyclone.test.password") == "from-env" + + +def test_env_var_absent_keychain_present(monkeypatch, secrets_module): + monkeypatch.delenv("cyclone.test.password", raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", + lambda service, name: "from-keychain", + ) + assert secrets_module.get_secret("cyclone.test.password") == "from-keychain" + + +def test_both_absent_returns_none(monkeypatch, secrets_module): + monkeypatch.delenv("cyclone.test.password", raising=False) + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda service, name: None, + ) + assert secrets_module.get_secret("cyclone.test.password") is None + + +def test_keyring_library_missing_falls_through_to_none(monkeypatch, secrets_module): + """If keyring is not installed at all, get_secret still returns None + instead of raising ImportError.""" + monkeypatch.setenv("cyclone.test.password", "from-env") + monkeypatch.setattr(secrets_module, "_HAS_KEYRING", False) + # Even if keyring is missing, the env-var tier still wins. + assert secrets_module.get_secret("cyclone.test.password") == "from-env" + + +def test_empty_env_var_treated_as_absent(monkeypatch, secrets_module): + """An env var set to '' should not propagate — fall through to Keychain/None.""" + monkeypatch.setenv("cyclone.test.password", "") + monkeypatch.setattr( + secrets_module.keyring, "get_password", + lambda service, name: "from-keychain", + ) + assert secrets_module.get_secret("cyclone.test.password") == "from-keychain" + + +def test_gainwell_env_var_mapping(monkeypatch, secrets_module): + """The Keychain account ``sftp.gainwell.password`` maps to the env + var ``CYCLONE_SFTP_PASSWORD`` via the internal name table. Without + this, an operator setting ``CYCLONE_SFTP_PASSWORD`` on a Linux box + would not get the MFT password.""" + monkeypatch.setenv("CYCLONE_SFTP_PASSWORD", "real-mft-password") + monkeypatch.setattr( + secrets_module.keyring, "get_password", lambda service, name: None, + ) + assert secrets_module.get_secret("sftp.gainwell.password") == "real-mft-password" +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cd backend && .venv/bin/pytest tests/test_secrets_envvar.py -v` +Expected: All tests FAIL because the current `get_secret()` does not check `os.environ` first. + +- [ ] **Step 3: Implement the env-var lookup** + +Modify `backend/src/cyclone/secrets.py`. Add `import os` at the top and replace the `get_secret` function: + +```python +"""macOS Keychain secret accessor for Cyclone. + +SP9. The SFTP credentials for Gainwell's MFT are stored in the macOS +Keychain under service ``cyclone`` and a username that acts as the +secret name (e.g. ``sftp.gainwell.password``). This module fetches +them by name. + +Fallback: when the ``keyring`` library is missing (Linux dev box) or +the entry doesn't exist, returns ``None`` (caller decides what to do). +A stub secret ```` is provided for the SP9 stub flow. + +Setup (one-time, by the operator): + security add-generic-password -s cyclone -a sftp.gainwell.password -w '' + +Verification: + security find-generic-password -s cyclone -a sftp.gainwell.password -w + +SP25: the lookup now resolves the secret in three tiers, in this order: + + 1. Plain env var named exactly ```` — highest priority. + Lets a Linux server or Docker container pass the MFT password + via ``CYCLONE_SFTP_PASSWORD=...`` without touching the Keychain. + Trailing/leading whitespace (including the ``\\n`` that .env + files often leave at EOF) is stripped; an empty value is + treated as absent. + 2. macOS Keychain via ``keyring`` — unchanged from SP9. Kept as a + fallback so a macOS workstation that prefers + ``security add-generic-password`` works without env-var exports. + 3. ``None`` — caller decides what to do (``SftpClient._connect`` + raises a precise ``RuntimeError`` on real-mode auth). +""" + +from __future__ import annotations + +import logging +import os +from typing import Optional + +log = logging.getLogger(__name__) + +SERVICE_NAME = "cyclone" +STUB_SECRET = "" + +# Try to import keyring lazily — it's an optional dep so the rest of +# the codebase doesn't fail on Linux dev boxes without it. +try: + import keyring # type: ignore[import-untyped] + _HAS_KEYRING = True +except ImportError: + keyring = None # type: ignore[assignment] + _HAS_KEYRING = False + + +def get_secret(name: str) -> Optional[str]: + """Fetch a secret by name. Three-tier lookup: env var, Keychain, None. + + Args: + name: The secret name. Matches both the env-var name and the + Keychain account (e.g. ``"sftp.gainwell.password"`` / + ``CYCLONE_SFTP_PASSWORD`` would NOT match — the env var + name is the bare secret name, not the + ``CYCLONE_`` form. Operators who want + the env-var shortcut set the var named exactly ````). + + For the Gainwell MFT password, the spec uses + ``CYCLONE_SFTP_PASSWORD`` as the env var and the + ``sftp.gainwell.password`` Keychain account as the + fallback. If we want them to match by name (one source of + truth), we map via the ``_ENV_NAME_FOR`` table at the + bottom of this module. + + Returns: + The secret string (stripped), or ``None`` if no tier produced + a value. + """ + env_name = _ENV_NAME_FOR.get(name, name) + raw = os.environ.get(env_name) + if raw is not None: + stripped = raw.strip() + if stripped: + log.debug("Secret %r resolved from env var %r", name, env_name) + return stripped + # Empty env var — treat as absent and fall through. + + if _HAS_KEYRING: + try: + value = keyring.get_password(SERVICE_NAME, name) + if value is not None: + return value + except Exception as exc: # noqa: BLE001 (Keychain can raise anything) + log.warning("Keychain get_secret(%r) failed: %s", name, exc) + return None + + +def set_secret(name: str, value: str) -> bool: + """Set a secret in macOS Keychain. Returns True on success. + + Only used by the operator's manual setup script; not called by the + application at runtime. + """ + if not _HAS_KEYRING: + log.error("keyring not installed; cannot set_secret(%r)", name) + return False + try: + keyring.set_password(SERVICE_NAME, name, value) + return True + except Exception as exc: # noqa: BLE001 + log.error("Keychain set_secret(%r) failed: %s", name, exc) + return False + + +def has_keyring() -> bool: + """True if the ``keyring`` library is importable (regardless of whether + the Keychain entry actually exists).""" + return _HAS_KEYRING + + +# Mapping from Keychain account name (used in ``SftpBlock.auth``) to +# the operator-facing env var. Keeping this table here means callers +# don't have to remember the difference between +# ``sftp.gainwell.password`` (Keychain account) and +# ``CYCLONE_SFTP_PASSWORD`` (env var). +_ENV_NAME_FOR: dict[str, str] = { + "sftp.gainwell.password": "CYCLONE_SFTP_PASSWORD", +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd backend && .venv/bin/pytest tests/test_secrets_envvar.py -v` +Expected: All 8 tests PASS. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/cyclone/secrets.py backend/tests/test_secrets_envvar.py +git commit -m "feat(sp25): secrets.get_secret() env-var-first lookup" +``` + +--- + +## Task 2: Add `store.update_clearhouse()` + +**Files:** +- Modify: `backend/src/cyclone/store.py` +- Test: `backend/tests/test_store_update_clearhouse.py` + +- [ ] **Step 1: Write the failing tests** + +Create `backend/tests/test_store_update_clearhouse.py`: + +```python +"""SP25 — store.update_clearhouse() round-trip test. + +The PATCH /api/clearhouse endpoint needs a way to replace the singleton +clearhouse row inside one session. This test exercises the method +directly against a fresh in-memory DB. +""" +from __future__ import annotations + +import json + +import pytest + +from cyclone import db +from cyclone.providers import Clearhouse, SftpBlock, FilenameBlock +from cyclone.store import store as cycl_store + + +def _seed_clearhouse(): + """Insert the default clearhouse row used by SP9's lifespan seed.""" + cycl_store.ensure_clearhouse_seeded() + + +def test_update_clearhouse_round_trip(): + """Replace all fields on the singleton row; GET returns the new values.""" + _seed_clearhouse() + + new_block = SftpBlock( + host="mft.gainwelltechnologies.com", + port=22, + username="colorado-fts\\coxix_prod_11525703", + paths={ + "outbound": "/CO XIX/PROD/coxix_prod_11525703/FromHPE", + "inbound": "/CO XIX/PROD/coxix_prod_11525703/ToHPE", + }, + stub=False, + staging_dir="./var/sftp/staging", + auth={"password_keychain_account": "sftp.gainwell.password"}, + ) + + new_clearhouse = Clearhouse.model_validate({ + "id": 1, + "name": "dzinesco", + "tpid": "11525703", + "submitter_name": "Dzinesco", + "submitter_id_qual": "46", + "submitter_contact_name": "Tyler Martinez", + "submitter_contact_email": "tyler@dzinesco.com", + "filename_block": { + "tz": "America/Denver", + "outbound_template": "{tpid}-{tx}-{ts_mt}-1of1.{ext}", + "inbound_template": "TP{tpid}-{orig_tx}_M{tracking}-{ts}-1of1_{file_type}.x12", + }, + "sftp_block": new_block.model_dump(), + "updated_at": "2026-06-24T00:00:00+00:00", + }) + + cycl_store.update_clearhouse(new_clearhouse) + + fetched = cycl_store.get_clearhouse() + assert fetched is not None + assert fetched.sftp_block.stub is False + assert fetched.sftp_block.host == "mft.gainwelltechnologies.com" + assert fetched.sftp_block.auth == {"password_keychain_account": "sftp.gainwell.password"} + + +def test_update_clearhouse_missing_row_raises(): + """If the singleton row was never seeded, update raises LookupError.""" + # Wipe the DB. + db._reset_for_tests() + db.init_db() + + new_block = SftpBlock( + host="mft.example.com", port=22, username="u", + paths={"outbound": "/o", "inbound": "/i"}, + stub=True, staging_dir="/tmp", auth={}, + ) + new_clearhouse = Clearhouse.model_validate({ + "id": 1, "name": "x", "tpid": "1", + "submitter_name": "X", "submitter_id_qual": "46", + "submitter_contact_name": "X", "submitter_contact_email": "x@x", + "filename_block": { + "tz": "America/Denver", + "outbound_template": "{tpid}-{tx}-{ts}-1of1.{ext}", + "inbound_template": "TP{tpid}-{tx}_{ts}.x12", + }, + "sftp_block": new_block.model_dump(), + "updated_at": "2026-06-24T00:00:00+00:00", + }) + + with pytest.raises(LookupError): + cycl_store.update_clearhouse(new_clearhouse) +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cd backend && .venv/bin/pytest tests/test_store_update_clearhouse.py -v` +Expected: Both tests FAIL with `AttributeError: module 'cyclone.store' has no attribute 'update_clearhouse'` (or similar — the method does not exist yet). + +- [ ] **Step 3: Implement `update_clearhouse()`** + +Modify `backend/src/cyclone/store.py`. Add the following method to the `CycloneStore` class, right after `get_clearhouse` (search for `def get_clearhouse` and insert after it): + +```python + def update_clearhouse(self, block: Clearhouse) -> Clearhouse: + """Replace the singleton clearhouse row. SP25. + + Used by ``PATCH /api/clearhouse`` to flip ``sftp_block.stub`` + and adjust host/port/paths without touching SQLite directly. + Raises ``LookupError`` if the singleton row is missing — the + caller is expected to run the lifespan seed first. + """ + from cyclone.db import ClearhouseORM + from cyclone.providers import Clearhouse as ClearhouseModel + with db.SessionLocal()() as s: + row = s.get(ClearhouseORM, 1) + if row is None: + raise LookupError( + "clearhouse singleton row missing; run the lifespan " + "seed (configure_scheduler) before PATCH /api/clearhouse" + ) + row.name = block.name + row.tpid = block.tpid + row.submitter_name = block.submitter_name + row.submitter_id_qual = block.submitter_id_qual + row.submitter_contact_name = block.submitter_contact_name + row.submitter_contact_email = block.submitter_contact_email + row.filename_block_json = json.dumps(block.filename_block.model_dump()) + row.sftp_block_json = json.dumps(block.sftp_block.model_dump()) + row.updated_at = datetime.now(timezone.utc).isoformat() + s.commit() + return ClearhouseModel.model_validate({ + "id": 1, + "name": row.name, + "tpid": row.tpid, + "submitter_name": row.submitter_name, + "submitter_id_qual": row.submitter_id_qual, + "submitter_contact_name": row.submitter_contact_name, + "submitter_contact_email": row.submitter_contact_email, + "filename_block": json.loads(row.filename_block_json), + "sftp_block": json.loads(row.sftp_block_json), + }) +``` + +Make sure `json`, `datetime`, `timezone` are already imported in `store.py` — they are (the file already uses them). + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd backend && .venv/bin/pytest tests/test_store_update_clearhouse.py -v` +Expected: Both tests PASS. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/cyclone/store.py backend/tests/test_store_update_clearhouse.py +git commit -m "feat(sp25): store.update_clearhouse() for PATCH endpoint" +``` + +--- + +## Task 3: Add `scheduler.reconfigure_scheduler()` + +**Files:** +- Modify: `backend/src/cyclone/scheduler.py` +- Test: `backend/tests/test_scheduler_reconfigure.py` + +- [ ] **Step 1: Write the failing tests** + +Create `backend/tests/test_scheduler_reconfigure.py`: + +```python +"""SP25 — scheduler.reconfigure_scheduler() hot-reload. + +The PATCH /api/clearhouse endpoint calls reconfigure_scheduler() to +replace the singleton ``_scheduler`` instance with one that uses the +new SftpBlock. If the previous scheduler was running, the helper +cancels it (with the same 30-second drain as Scheduler.stop()) and +starts the new one. If it was stopped, the new one is left stopped. +""" +from __future__ import annotations + +import asyncio +from pathlib import Path + +import pytest + +from cyclone import db, scheduler as sched_mod +from cyclone.providers import SftpBlock +from cyclone.scheduler import Scheduler + + +def _stub_block(stub: bool = True, host: str = "stub-host") -> SftpBlock: + return SftpBlock( + host=host, + port=22, + username="test-user", + paths={"outbound": "/o", "inbound": "/i"}, + stub=stub, + staging_dir="/tmp/cyclone-test", + auth={}, + ) + + +def _drain(): + """Yield once so any awaiting coroutine has a chance to advance.""" + return asyncio.sleep(0) + + +@pytest.fixture(autouse=True) +def _reset_scheduler(): + sched_mod.reset_scheduler_for_tests() + yield + sched_mod.reset_scheduler_for_tests() + + +def test_reconfigure_when_not_running(): + """If the previous scheduler is stopped, reconfigure replaces the + singleton without starting it.""" + + async def _go(): + block_a = _stub_block(stub=True, host="block-a") + sched_a = sched_mod.configure_scheduler(block_a, sftp_block_name="a") + assert not sched_a.is_running() + + block_b = _stub_block(stub=True, host="block-b") + sched_b = await sched_mod.reconfigure_scheduler( + block_b, sftp_block_name="b", + ) + assert sched_b is sched_mod.get_scheduler() + assert not sched_b.is_running() + assert sched_b._sftp_block_name == "b" + + asyncio.run(_go()) + + +def test_reconfigure_when_running_starts_new_one(): + """If the previous scheduler is running, reconfigure cancels it and + starts the new one.""" + + async def _go(): + block_a = _stub_block(stub=True, host="block-a") + sched_a = sched_mod.configure_scheduler(block_a, sftp_block_name="a") + await sched_a.start() + assert sched_a.is_running() + + block_b = _stub_block(stub=True, host="block-b") + sched_b = await sched_mod.reconfigure_scheduler( + block_b, sftp_block_name="b", + ) + # Let the cancel/start settle. + await _drain() + assert sched_b.is_running() + assert sched_b._sftp_block_name == "b" + + await sched_b.stop() + + asyncio.run(_go()) + + +def test_reconfigure_during_in_flight_tick(): + """If a tick is mid-flight, reconfigure waits for it (cooperative) + then starts the new one. Verified by feeding the scheduler a slow + client factory.""" + + async def _go(): + block_a = _stub_block(stub=True, host="block-a") + sched_mod.configure_scheduler(block_a, sftp_block_name="a") + + started = asyncio.Event() + finish = asyncio.Event() + + async def _slow_tick(): + started.set() + await finish.wait() + + # Inject a custom factory that returns a scheduler pre-armed + # with a slow _tick_impl, so we can hold the "tick" mid-flight. + class _FakeScheduler: + def __init__(self, block, **_): + self._block = block + self._task = None + self._running = False + + async def start(self): + self._running = True + self._task = asyncio.create_task(_slow_tick()) + + async def stop(self): + self._running = False + if self._task: + self._task.cancel() + try: + await self._task + except asyncio.CancelledError: + pass + self._task = None + + def is_running(self): + return self._running + + # Monkey-patch the module-level Scheduler class for this test. + original_scheduler_cls = sched_mod.Scheduler + sched_mod.Scheduler = _FakeScheduler # type: ignore[assignment] + try: + sched_mod.configure_scheduler(block_a, sftp_block_name="a") + sched_a = sched_mod.get_scheduler() + await sched_a.start() + await started.wait() + assert sched_a.is_running() + + # Now reconfigure. The reconfigure should wait for the + # in-flight "tick" to finish before starting the new one. + reconfigure_task = asyncio.create_task( + sched_mod.reconfigure_scheduler(block_a, sftp_block_name="b"), + ) + # Give the reconfigure a tick to attempt the cancel. + await asyncio.sleep(0.05) + assert not reconfigure_task.done(), "reconfigure should be waiting for in-flight tick" + + finish.set() + sched_b = await reconfigure_task + assert sched_b is sched_mod.get_scheduler() + finally: + sched_mod.Scheduler = original_scheduler_cls # type: ignore[assignment] + + asyncio.run(_go()) + + +def test_reconfigure_drain_timeout_matches_stop(monkeypatch): + """The reconfigure's drain timeout matches Scheduler.stop() — both + are 30 seconds. We can't wait 30s in a test, but we can verify the + constant is shared.""" + + # Both code paths should reference the same constant. + assert hasattr(sched_mod, "_DRAIN_TIMEOUT_SECONDS") + assert sched_mod._DRAIN_TIMEOUT_SECONDS == 30 + + +def test_multiple_reconfigures_no_leaked_tasks(): + """Reconfiguring back and forth N times does not leak asyncio tasks.""" + + async def _go(): + block = _stub_block(stub=True, host="block-a") + sched_mod.configure_scheduler(block, sftp_block_name="a") + sched = sched_mod.get_scheduler() + await sched.start() + + before = len(asyncio.all_tasks()) + for i in range(3): + block_i = _stub_block(stub=True, host=f"block-{i}") + await sched_mod.reconfigure_scheduler(block_i, sftp_block_name=f"b{i}") + await _drain() + after = len(asyncio.all_tasks()) + + # The new scheduler itself owns one task; allow +1. + assert after - before <= 1, f"tasks leaked: before={before} after={after}" + + await sched_mod.get_scheduler().stop() + + asyncio.run(_go()) +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cd backend && .venv/bin/pytest tests/test_scheduler_reconfigure.py -v` +Expected: Tests FAIL — `reconfigure_scheduler` does not exist yet. + +- [ ] **Step 3: Implement `reconfigure_scheduler()`** + +Modify `backend/src/cyclone/scheduler.py`. Add a constant near the top of the file (after `STATUS_*` definitions), and add the new function near the bottom (right after `reset_scheduler_for_tests`): + +```python +# How long reconfigure_scheduler waits for the in-flight tick to +# drain before cancelling the old task. Matches Scheduler.stop(). +_DRAIN_TIMEOUT_SECONDS = 30 +``` + +And add the function after `reset_scheduler_for_tests`: + +```python +async def reconfigure_scheduler( + sftp_block: SftpBlock, + *, + poll_interval_seconds: int = 60, + sftp_block_name: str = "default", +) -> Scheduler: + """Replace the singleton scheduler; restart if it was running. SP25. + + Called from ``PATCH /api/clearhouse`` so the next scheduler tick + uses the new SftpBlock without a process restart. + + Lifecycle: + * If the previous scheduler is running, ``stop()`` it (waits + up to ``_DRAIN_TIMEOUT_SECONDS`` for the current tick to + finish — matches the existing ``Scheduler.stop()`` timeout). + * Replace the module-level ``_scheduler`` singleton with a + fresh ``Scheduler`` against the new block. + * If the previous one was running, ``start()`` the new one. + + Threading: same constraint as ``configure_scheduler`` — must be + called from the FastAPI event loop, not a worker thread. + """ + global _scheduler + was_running = False + if _scheduler is not None: + was_running = _scheduler.is_running() + if was_running: + # Drain the in-flight tick. Scheduler.stop() already + # applies the _DRAIN_TIMEOUT_SECONDS timeout internally; + # we just await it. + await _scheduler.stop() + poll = int( + os.environ.get("CYCLONE_SCHEDULER_POLL_SECONDS", poll_interval_seconds), + ) + _scheduler = Scheduler( + sftp_block, + poll_interval_seconds=poll, + sftp_block_name=sftp_block_name, + ) + if was_running: + await _scheduler.start() + log.info( + "Scheduler reconfigured", + extra={ + "was_running": was_running, + "sftp_block": sftp_block_name, + "host": sftp_block.host, + "stub": sftp_block.stub, + }, + ) + return _scheduler +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd backend && .venv/bin/pytest tests/test_scheduler_reconfigure.py -v` +Expected: All 5 tests PASS. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/cyclone/scheduler.py backend/tests/test_scheduler_reconfigure.py +git commit -m "feat(sp25): scheduler.reconfigure_scheduler() hot-reload helper" +``` + +--- + +## Task 4: Add `PATCH /api/clearhouse` endpoint + +**Files:** +- Modify: `backend/src/cyclone/api.py` +- Test: `backend/tests/test_api_clearhouse_patch.py` + +- [ ] **Step 1: Write the failing tests** + +Create `backend/tests/test_api_clearhouse_patch.py`: + +```python +"""SP25 — PATCH /api/clearhouse endpoint. + +Lets the operator flip sftp_block.stub and adjust host/port/paths +without raw SQL. The endpoint validates via the Clearhouse Pydantic +model, writes via store.update_clearhouse(), then hot-reloads the +running scheduler via scheduler.reconfigure_scheduler(). +""" +from __future__ import annotations + +import json +from unittest.mock import AsyncMock, patch + +import pytest + +from cyclone import db, scheduler as sched_mod +from cyclone.providers import Clearhouse, SftpBlock, FilenameBlock +from cyclone.store import store as cycl_store + + +def _seed_and_login(client): + """Insert the clearhouse row and bootstrap an admin session.""" + cycl_store.ensure_clearhouse_seeded() + from cyclone.auth.bootstrap import ensure_admin_user + ensure_admin_user("admin", "test-password-12345") + r = client.post("/api/auth/login", json={ + "username": "admin", "password": "test-password-12345", + }) + assert r.status_code == 200, r.text + + +def _clearhouse_body_from_get(client, **overrides) -> dict: + """GET the current clearhouse row and apply ``overrides`` to the + sftp_block. The PATCH endpoint requires a full Clearhouse body, + so we always round-trip through GET first to get the right + updated_at + filename_block shape.""" + r = client.get("/api/clearhouse") + assert r.status_code == 200, r.text + body = r.json() + sb = dict(body["sftp_block"]) + sb.update(overrides) + body["sftp_block"] = sb + return body + + +def test_patch_flips_stub_and_get_reflects(client): + _seed_and_login(client) + body = _clearhouse_body_from_get(client, stub=False, host="mft.example.com") + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 200, r.text + payload = r.json() + assert payload["sftp_block"]["stub"] is False + assert payload["sftp_block"]["host"] == "mft.example.com" + + r2 = client.get("/api/clearhouse") + assert r2.status_code == 200 + assert r2.json()["sftp_block"]["stub"] is False + + +def test_patch_with_string_stub_returns_422(client): + _seed_and_login(client) + body = _clearhouse_body_from_get(client) + body["sftp_block"]["stub"] = "yes" # string instead of bool + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 422 + + +def test_patch_real_block_requires_host(client): + _seed_and_login(client) + body = _clearhouse_body_from_get(client, stub=False, host="") + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 422 + + +def test_patch_real_block_requires_password_keychain_account(client): + _seed_and_login(client) + body = _clearhouse_body_from_get( + client, + stub=False, + auth={"password_keychain_account": ""}, + ) + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 422 + + +def test_patch_without_session_returns_401(client): + cycl_store.ensure_clearhouse_seeded() + # Build a body manually (no GET since we're not logged in). + r_get = client.get("/api/clearhouse") + assert r_get.status_code == 200 + body = r_get.json() + body["sftp_block"]["stub"] = False + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 401 + + +def test_patch_triggers_scheduler_reconfigure(client): + """The PATCH must call scheduler.reconfigure_scheduler() with the new + SftpBlock so the next tick uses real MFT instead of the stub.""" + _seed_and_login(client) + + with patch.object( + sched_mod, "reconfigure_scheduler", + AsyncMock(return_value=AsyncMock()), + ) as mock_reconf: + body = _clearhouse_body_from_get( + client, + stub=False, + host="mft.gainwelltechnologies.com", + ) + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 200, r.text + assert mock_reconf.await_count == 1 + # The new sftp_block is passed positionally; sftp_block_name + # comes from the clearhouse row's ``name`` field. + call_args = mock_reconf.await_args + assert call_args.args[0].stub is False + assert call_args.args[0].host == "mft.gainwelltechnologies.com" + assert call_args.kwargs.get("sftp_block_name") == "dzinesco" + + +def test_patch_returns_post_update_row(client): + _seed_and_login(client) + body = _clearhouse_body_from_get(client, stub=False, host="mft.example.com") + r = client.patch("/api/clearhouse", json=body) + assert r.status_code == 200 + payload = r.json() + assert payload["name"] == "dzinesco" + assert payload["tpid"] == "11525703" + assert payload["sftp_block"]["stub"] is False + assert payload["sftp_block"]["host"] == "mft.example.com" +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cd backend && .venv/bin/pytest tests/test_api_clearhouse_patch.py -v` +Expected: All tests FAIL — `PATCH /api/clearhouse` does not exist (404 or 405). + +- [ ] **Step 3: Implement `PATCH /api/clearhouse`** + +Modify `backend/src/cyclone/api.py`. Right after the existing `GET /api/clearhouse` endpoint (search for `@app.get("/api/clearhouse"`) and insert after the function body: + +```python +@app.patch("/api/clearhouse", dependencies=[Depends(matrix_gate)]) +async def patch_clearhouse(body: dict) -> Any: + """Replace the singleton clearhouse row (SP25). + + The full ``Clearhouse`` model is required — we don't accept partial + updates because the operator-facing use case is "I'm switching the + loop to real MFT" or "I'm pointing at a different MFT server", + not "I'm tweaking one field at a time." Validation errors are + returned as 422 (Pydantic default). + + After a successful write, the running scheduler is hot-reloaded + via ``scheduler.reconfigure_scheduler(force=True)`` so the next + tick uses the new SftpBlock without a process restart. + """ + from cyclone import scheduler as _scheduler_mod + from cyclone.providers import Clearhouse as _Clearhouse + + # Validate via the same Pydantic model the spec uses, so the + # request shape is the source of truth. + try: + parsed = _Clearhouse.model_validate(body) + except Exception as exc: + raise HTTPException(status_code=422, detail=str(exc)) from exc + + # SP25: when sftp_block.stub=false, the block must carry an auth + # account name and a non-empty host. The Pydantic model catches + # some of these; this catches the "empty password_keychain_account" + # case (which Pydantic allows because it's a free-form dict). + sb = parsed.sftp_block + if not sb.stub: + if not sb.host: + raise HTTPException( + status_code=422, + detail="sftp_block.host is required when stub=false", + ) + auth = sb.auth or {} + if not auth.get("password_keychain_account") and not auth.get("key_file"): + raise HTTPException( + status_code=422, + detail=( + "sftp_block.auth must contain either " + "'password_keychain_account' or 'key_file' when stub=false" + ), + ) + + updated = store.update_clearhouse(parsed) + await _scheduler_mod.reconfigure_scheduler( + updated.sftp_block, + sftp_block_name=updated.name or "default", + ) + return json.loads(updated.model_dump_json()) +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd backend && .venv/bin/pytest tests/test_api_clearhouse_patch.py -v` +Expected: All 7 tests PASS. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/cyclone/api.py backend/tests/test_api_clearhouse_patch.py +git commit -m "feat(sp25): PATCH /api/clearhouse with hot-reload" +``` + +--- + +## Task 5: Write `docs/RUNBOOK.md` + +**Files:** +- Create: `docs/RUNBOOK.md` + +- [ ] **Step 1: Draft the runbook** + +Create `docs/RUNBOOK.md` with the following content: + +```markdown +# Cyclone Operator Runbook + +Procedures for running Cyclone in production. The end-user README +covers dev setup; this doc is for an operator bringing up SFTP polling +on a fresh host. + +## SP25 — Enable SFTP Polling for Real Gainwell MFT + +The inbound MFT polling scheduler ships in SP16 and is wired to real +paramiko SFTP in SP13. To turn it on for `mft.gainwelltechnologies.com`: + +### Prerequisites + +- Python 3.11+ with the `cyclone` package installed (`pip install -e .[dev,sftp]`). +- The macOS `keyring` library is optional. On a Linux server or Docker + container, the MFT password is supplied via a plain env var (no + Keychain dependency). +- Outbound TCP/22 to `mft.gainwelltechnologies.com`. + +### Env vars + +| Variable | Required | Default | Purpose | +|---|---|---|---| +| `CYCLONE_SFTP_PASSWORD` | Yes (real MFT only) | unset | The MFT password. Stripped of whitespace; empty values are treated as unset. | +| `CYCLONE_SCHEDULER_AUTOSTART` | No | unset (falsy) | When `1`/`true`/`yes`, the scheduler starts polling on API launch. | +| `CYCLONE_SCHEDULER_POLL_SECONDS` | No | `60` | Seconds between poll cycles. The Gainwell MFT server doesn't push — we pull. | + +### First-time setup + +1. **Set the password env var.** On the server that runs Cyclone: + + ```bash + export CYCLONE_SFTP_PASSWORD='the-actual-password' + ``` + + For systemd / Docker, persist this in the service file or + `docker-compose.yml` (use a `_FILE` companion or a `secrets:` + block; see your platform's docs). + +2. **Confirm the clearhouse SFTP block is configured.** `GET /api/clearhouse` should return a row. If not, run the lifespan once to seed it (any API launch will do). + +3. **Flip stub → false and point at real MFT.** Authenticated as an admin: + + ```bash + # GET the current row, modify the sftp_block, PATCH it back. + # The endpoint requires a full Clearhouse body, so always + # round-trip through GET to get the right updated_at + filename_block. + curl -s http://127.0.0.1:8000/api/clearhouse -b cookies.txt > /tmp/ch.json + # Edit /tmp/ch.json — set sftp_block.stub to false and adjust host/port/paths/auth. + # The minimum change for real-MFT mode is: + # jq '.sftp_block.stub = false + # | .sftp_block.host = "mft.gainwelltechnologies.com" + # | .sftp_block.auth = {"password_keychain_account": "sftp.gainwell.password"}' \ + # /tmp/ch.json > /tmp/ch-patched.json + curl -X PATCH http://127.0.0.1:8000/api/clearhouse \ + -H 'Content-Type: application/json' \ + -b cookies.txt \ + --data @/tmp/ch-patched.json + ``` + + The endpoint hot-reloads the scheduler. No API restart required. + +4. **Start polling.** Either: + + - Set `CYCLONE_SCHEDULER_AUTOSTART=true` and restart the API. + - Or trigger manually: + + ```bash + curl -X POST http://127.0.0.1:8000/api/admin/scheduler/start -b cookies.txt + ``` + +### Verification + +```bash +# Is the loop running? +curl http://127.0.0.1:8000/api/admin/scheduler/status -b cookies.txt + +# Force one poll cycle right now: +curl -X POST http://127.0.0.1:8000/api/admin/scheduler/tick -b cookies.txt + +# What files has the scheduler seen? +curl 'http://127.0.0.1:8000/api/admin/scheduler/processed-files?limit=20' -b cookies.txt + +# Only the errors? +curl 'http://127.0.0.1:8000/api/admin/scheduler/processed-files?status=error' -b cookies.txt +``` + +A successful first poll shows rows in `processed-files` with +`status=ok` and a non-zero `claim_count` for 835/277CA files. + +### Troubleshooting + +| Symptom | Likely cause | Fix | +|---|---|---| +| `status=error` rows with `AuthenticationException` | Wrong password in `CYCLONE_SFTP_PASSWORD` | Re-export with the correct value, then PATCH /api/clearhouse or restart. | +| `status=error` rows with `IOError: [Errno 111] Connection refused` | Outbound TCP/22 blocked, or wrong host/port | Check the firewall; confirm `host` in `GET /api/clearhouse`. | +| Zero rows in `processed-files` after a tick | Inbound MFT dir is empty (HPE hasn't pushed yet) — not an error | Wait. Trigger `tick` again later. | +| `RuntimeError: SFTP: Keychain entry ... missing or stub` | `get_secret()` fell through to Keychain (Linux + missing env var) | Set `CYCLONE_SFTP_PASSWORD`. | +| `RuntimeError: SftpBlock.auth must contain ...` | PATCH set `stub=false` without an `auth` block | PATCH again with a complete `auth` dict. | +| Scheduler never starts (`running: false`) | `CYCLONE_SCHEDULER_AUTOSTART` not set, no manual `start` call | Either set autostart or `POST /api/admin/scheduler/start`. | + +### macOS dev box variant + +If you prefer the Keychain over env vars on macOS: + +```bash +security add-generic-password -s cyclone -a sftp.gainwell.password -w '' +security find-generic-password -s cyclone -a sftp.gainwell.password -w # verify +``` + +The env var is the highest-priority lookup; the Keychain is the +fallback. Setting both means the env var wins. To force the Keychain, +unset the env var for that shell. +``` + +- [ ] **Step 2: Verify the file is readable** + +Run: `ls -la docs/RUNBOOK.md && head -5 docs/RUNBOOK.md` +Expected: File exists, first line is `# Cyclone Operator Runbook`. + +- [ ] **Step 3: Commit** + +```bash +git add docs/RUNBOOK.md +git commit -m "docs(sp25): RUNBOOK.md — enable SFTP polling for real Gainwell MFT" +``` + +--- + +## Task 6: Run full verification + +**Files:** none (verification only) + +- [ ] **Step 1: Run the full backend test suite** + +Run: `cd backend && .venv/bin/pytest -q` +Expected: All previously-passing tests still pass; the 23 new tests +(9 secrets + 2 store + 5 scheduler + 7 api) also pass. Total test count +should be `previous + 23`. + +- [ ] **Step 2: Run the backend linter** + +Run: `cd backend && .venv/bin/ruff check src tests` +Expected: No lint errors. + +- [ ] **Step 3: Run the backend typechecker** + +Run: `cd backend && .venv/bin/mypy src/cyclone/secrets.py src/cyclone/scheduler.py src/cyclone/api.py src/cyclone/store.py` +Expected: No type errors. (mypy may not be configured for the project; +if `mypy.ini` is missing, skip this step and note it in the PR +description.) + +- [ ] **Step 4: Smoke-test the endpoint manually (stub mode)** + +Run a stub-mode smoke test in a Python REPL or one-off script: + +```python +from fastapi.testclient import TestClient +from cyclone import db +from cyclone.store import store as cycl_store + +db._reset_for_tests() +db.init_db() +cycl_store._seed_clearhouse_if_missing() + +from cyclone.api import app +with TestClient(app) as client: + # Bootstrap admin + login. + from cyclone.auth.bootstrap import ensure_admin_user + ensure_admin_user("admin", "test-password-12345") + client.post("/api/auth/login", json={"username": "admin", "password": "test-password-12345"}) + # Verify GET. + r = client.get("/api/clearhouse") + assert r.status_code == 200 + assert r.json()["sftp_block"]["stub"] is True + # PATCH. + body = r.json() + body["sftp_block"]["stub"] = False + r2 = client.patch("/api/clearhouse", json=body) + assert r2.status_code == 200, r2.text + assert r2.json()["sftp_block"]["stub"] is False + # Verify scheduler was reconfigured. + r3 = client.get("/api/admin/scheduler/status") + assert r3.status_code == 200 +``` + +Expected: all assertions pass. + +- [ ] **Step 5: Commit any verification artifacts** + +If the smoke-test script was saved as a file (e.g. `scripts/sp25_smoke.py`), commit it. Otherwise, no commit is needed for this task. + +--- + +## Self-Review Checklist (run before opening the PR) + +- [ ] All 8 secrets-envvar tests pass. +- [ ] Both store-update-clearhouse tests pass. +- [ ] All 5 scheduler-reconfigure tests pass. +- [ ] All 7 api-clearhouse-patch tests pass. +- [ ] `RUNBOOK.md` is committed. +- [ ] No spec section is unimplemented. Verify against `docs/superpowers/specs/2026-06-24-cyclone-sftp-polling-enablement-design.md`: + - §3.1 (env-var lookup) → Task 1 + - §3.2 (PATCH /api/clearhouse) → Tasks 2 + 4 + - §3.3 (reconfigure_scheduler) → Task 3 + - §3.4 (RUNBOOK.md) → Task 5 + - §6 (env vars table) → Task 5 + - §8 (testing plan) → Tasks 1, 2, 3, 4 + +--- + +## Final PR + +When all tasks pass, open a PR titled `SP25 SFTP Polling Enablement` +against `main`. The merge is a single atomic merge commit per the +SP-N flow — no squash, no rebase. diff --git a/docs/superpowers/specs/2026-06-24-cyclone-sftp-password-file-companion-design.md b/docs/superpowers/specs/2026-06-24-cyclone-sftp-password-file-companion-design.md new file mode 100644 index 0000000..a14ffd3 --- /dev/null +++ b/docs/superpowers/specs/2026-06-24-cyclone-sftp-password-file-companion-design.md @@ -0,0 +1,136 @@ +# SP26 — SFTP Password File Companion: Design Spec + +**Date:** 2026-06-24 +**Status:** Draft, awaiting user sign-off +**Branch:** `sp26-sftp-password-file-companion` +**Aesthetic direction:** No new UI. Tiny extension to `secrets.get_secret()` plus a docker-compose secret entry plus a runbook row. Backward-compatible with every existing call site — no signature changes, no new public symbols. + +## 1. Scope + +SP25 made the MFT password portable: `cyclone.secrets.get_secret()` now resolves `sftp.gainwell.password` from a plain env var (`CYCLONE_SFTP_PASSWORD`), the macOS Keychain, or `None`, in that order. That covers a Linux server exporting the env var. It does not cover the SP23 Docker stack, where the convention is to mount a secret as a file at `/run/secrets/` and point an env var at the path — never embed the secret value in `docker-compose.yml`. + +SP26 closes that one remaining gap. Two small changes: + +1. **`_FILE` companion in `secrets.get_secret()`.** When the env var `_FILE` is set (e.g. `CYCLONE_SFTP_PASSWORD_FILE=/run/secrets/cyclone_sftp_password`), read the file, strip whitespace, return the contents. The `_FILE` tier sits *above* the plain env var in the lookup chain so a Docker secret always wins over a stray env-var export. The `auth/bootstrap.py` pattern (`_read_secret`) is the precedent — `_FILE`-then-env-var, file takes precedence. +2. **Docker compose wiring.** Add a `cyclone_sftp_password` secret block to `docker-compose.yml` and a `CYCLONE_SFTP_PASSWORD_FILE` env var on the backend service so a fresh `docker compose up` on the SP23 stack can poll real Gainwell MFT without anyone editing the compose file. + +Out of scope (explicit, each is its own future SP if requested): +- A `_FILE` tier for the SQLCipher key (`CYCLONE_SECRET_KEY_FILE` is referenced in `docker-compose.yml` but is read by a different code path in `db_crypto.py`; SP26 does not unify the two readers). +- A `_FILE` tier for the backup passphrase / salt. Those are Keychain-only today (per SP12/SP17); bringing them into the `_FILE` world would be a separate increment. +- A frontend change. The compose wiring is backend-only; the frontend nginx already reverse-proxies `/api/*` unchanged. +- Per-secret granularity. `_FILE` support lands generically inside `get_secret()` via the `_ENV_NAME_FOR` table — every secret that has an env-var mapping automatically gets the `_FILE` companion, not just `sftp.gainwell.password`. Today only the MFT password has such a mapping, so the visible effect is MFT-only; the broader capability is a side benefit. + +## 2. Goals + +1. **The SP23 Docker stack can run real-MFT polling with zero secret values in `docker-compose.yml`.** An operator writing the file once at `/etc/cyclone/secrets/sftp_password` is sufficient. +2. **`_FILE` takes precedence over the plain env var** when both are set, matching the `auth/bootstrap.py` convention. Setting both is a configuration error; the operator-visible behavior is "the file wins", which is what they almost certainly meant. +3. **The existing call sites do not change.** Every consumer of `get_secret()` (the scheduler's `SftpClient._connect`, the backup passphrase lookup, the SQLCipher key lookup, the CLI) keeps its existing call shape. The new tier is invisible to all of them — they get either the same value as before, or the file-derived value when the operator has set the `_FILE` env var. +4. **The runbook documents the Docker-secrets variant** so an operator bringing up a fresh SP23 host does not need to read the spec or the source to find the right env var name. + +## 3. Locked decisions + +### 3.1 `_FILE` tier placement — top of the lookup chain + +The new lookup chain for `get_secret(name)` becomes, in order: + +1. **`_FILE` env var set** → `Path(file_path).read_text().strip()` → return value. Missing or unreadable file raises `RuntimeError` with a message that names the env var and the path. Mirrors `auth/bootstrap.py:_read_secret` (which raises on `OSError`). +2. **`` env var set and non-empty (after strip)** → return stripped value. Existing SP25 behavior, unchanged. +3. **macOS Keychain** via `keyring` — existing fallback, unchanged. +4. **`None`** — existing fallback, unchanged. + +Rationale for raising on a missing `_FILE` file (rather than silently falling through): an operator who set `CYCLONE_SFTP_PASSWORD_FILE=/run/secrets/cyclone_sftp_password` is making a positive statement about where the secret lives. A silent fall-through to the plain env var or Keychain would mask a real misconfiguration (typo in path, container started without the secret mounted, file removed). A `RuntimeError` at the next scheduler tick surfaces the problem immediately. The existing `SftpClient._connect` already wraps unknown-secret errors in a clear `RuntimeError`, so the operator sees a consistent error class whether the secret is missing entirely or the file path is wrong. + +Rationale for placing `_FILE` above the plain env var (rather than below, or making them coexist): the SP23 admin-bootstrap convention — and the broader Docker-secrets convention — is "file wins". An operator who mounted a secret file almost certainly did not also intend for an env-var export to override it. Putting `_FILE` first removes ambiguity. + +### 3.2 `_FILE` name derived from the env-var name + +The `_FILE` companion name is ` + "_FILE"`. For `sftp.gainwell.password` the env-var name is `CYCLONE_SFTP_PASSWORD` (from the existing `_ENV_NAME_FOR` table in `secrets.py`), so the `_FILE` companion is `CYCLONE_SFTP_PASSWORD_FILE`. No new table entry required — the helper computes the `_FILE` name on the fly. + +This means any future secret that gets an entry in `_ENV_NAME_FOR` automatically gains the `_FILE` companion. The cost of generalization (a single `+ "_FILE"` suffix) is trivial; the alternative (a second hand-maintained mapping table) is a maintenance trap. + +### 3.3 No new public symbols + +`_read_secret` in `auth/bootstrap.py` stays where it is. SP26 does not extract a shared helper to `cyclone/secrets.py` because the two callers have different failure modes (bootstrap raises on missing file because it cannot proceed; `get_secret` raises on missing file because the operator made a positive statement about where the secret lives, but returns `None` when nothing is set at all). Sharing a helper would conflate those and force one caller to take on the other's behavior. A duplicated 6-line reader inside `get_secret` is the right amount of code. + +### 3.4 Compose-file change is minimal + +`docker-compose.yml` gains: + +- A `cyclone_sftp_password` entry in the top-level `secrets:` block pointing at `/etc/cyclone/secrets/sftp_password` (matching the file-based-secret pattern already used for the admin creds). +- A `cyclone_sftp_password` entry in the backend service's `secrets:` list. +- A `CYCLONE_SFTP_PASSWORD_FILE: "/run/secrets/cyclone_sftp_password"` env var on the backend service. + +No Dockerfile change. No volume change. No `nginx.conf` change. The frontend service is unaffected. + +### 3.5 No new pubsub events, no new endpoints, no new migrations + +The increment is fully internal to `secrets.py` plus a small compose-file edit. No `processed_inbound_files` row is touched, no `claim_written` / `remittance_written` event shape changes, no DB migration is needed. + +### 3.6 Idempotency and audit are unchanged + +`_FILE` lookup is a pure function of the filesystem at the moment `get_secret()` is called. The scheduler already calls `get_secret()` on every tick (via `SftpClient._connect`), so a `_FILE` change is picked up on the next tick without any restart. There is no per-tick caching that would need invalidating. The audit chain (`processed_inbound_files` rows, `claim_submitted` / `remittance_written` events) is unchanged. + +## 4. Files + +**Modified:** +- `backend/src/cyclone/secrets.py` — extend `get_secret()` with the `_FILE` tier at the top of the chain. Update the module docstring to mention the four-tier lookup. No new public symbols. +- `docker-compose.yml` — add `cyclone_sftp_password` to top-level `secrets:`, add to backend `secrets:` list, add `CYCLONE_SFTP_PASSWORD_FILE` env var on backend service. +- `docs/RUNBOOK.md` — add `CYCLONE_SFTP_PASSWORD_FILE` row to the env-var table; add a "Docker-secrets variant" subsection under "First-time setup" mirroring the macOS dev box variant. + +**New:** +- `backend/tests/test_secrets_file.py` — covers all `_FILE` cases (file wins, file + env both set → file wins, file missing → raises, file with trailing newline stripped, full Gainwell chain via `CYCLONE_SFTP_PASSWORD_FILE`). Reuses the same `secrets_module` fixture pattern as `test_secrets_envvar.py`. + +**New migration:** none. The `clearhouse` table is unchanged. The `processed_inbound_files` table is unchanged. The `users` table is unchanged. + +## 5. API surface + +None. No new endpoints, no modified endpoints, no new request/response shapes. `PATCH /api/clearhouse` (added in SP25) and the scheduler hot-reload path are both unchanged — they call into the store and the scheduler, not directly into `secrets.get_secret()`, so the new `_FILE` tier is transparent to them. + +## 6. Env vars (operator-facing) + +| Variable | Default | Purpose | +|---|---|---| +| `CYCLONE_SFTP_PASSWORD` | unset | Plain-env-var form of the MFT password. Second-priority lookup in `secrets.get_secret()`. Stripped of leading/trailing whitespace. (Unchanged from SP25.) | +| `CYCLONE_SFTP_PASSWORD_FILE` | unset | Path to a file containing the MFT password, read on every call. Highest-priority lookup. Used by the SP23 Docker stack so the secret value never appears in `docker-compose.yml`. Stripped of leading/trailing whitespace. **NEW in SP26.** | + +The `_FILE` companion is checked first; the plain env var is checked second. Setting both means the file wins. + +## 7. Validation rules + +No new `R_*` rule IDs in `cyclone.validation.rules`. SP26 is not an EDI-increment. + +## 8. Testing plan + +**`test_secrets_file.py`** — 6 cases: + +1. `_FILE` set, file exists, env var absent → file contents returned. +2. `_FILE` set with trailing `\n` in file → stripped value returned. +3. `_FILE` set + plain env var both set → `_FILE` wins. +4. `_FILE` set, file does not exist → raises `RuntimeError` with the env var name and the path in the message. +5. `_FILE` absent, plain env var set → falls through to plain env var (existing SP25 path, regression guard). +6. Full Gainwell chain: setting `CYCLONE_SFTP_PASSWORD_FILE` makes `get_secret("sftp.gainwell.password")` return the file's contents. + +**`test_docker.py`** (existing) — extend the compose-shape assertion to require `cyclone_sftp_password` in the secrets block and `CYCLONE_SFTP_PASSWORD_FILE` on the backend service. This is a 5-line addition that catches accidental deletion. + +**Target backend test count after SP26: current + 6 + 1 = current + 7 tests.** + +## 9. Out of scope (future SPs) + +- **Unify the SQLCipher key path** with `get_secret()` so `CYCLONE_SECRET_KEY_FILE` (currently referenced in `docker-compose.yml` but read by `db_crypto.py` directly) goes through the same lookup chain. Useful for consistency but not required for the MFT story. +- **`_FILE` tier for backup passphrase / salt** (used by `backup_service.py` and the `python -m cyclone backup` CLI subcommands). They are Keychain-only today; bringing them into the Docker-secrets world is a separate increment. +- **A migration of the existing `sftp.gainwell.password` Keychain entry** to a `_FILE` mount on the macOS dev box. macOS dev boxes use `security add-generic-password`, which the spec does not change. +- **A frontend operator UI** for managing secrets. The runbook covers the curl / docker-compose workflow. +- **Per-secret granularity in the runbook.** The runbook documents only `CYCLONE_SFTP_PASSWORD_FILE` (the only secret that has an `_ENV_NAME_FOR` entry today). The generic `_FILE` mechanism is documented in code comments; only the operator-visible entry gets a runbook row. + +## 10. Open questions resolved this session + +| # | Question | Resolution | +|---|---|---| +| 1 | What does "update docker" mean given SP25 just landed? | Extend the `secrets.get_secret()` lookup to support `_FILE` env vars (the Docker-secrets pattern), and wire the SFTP password through that pattern in `docker-compose.yml`. | +| 2 | Just SFTP, or all secrets? | SFTP only — `get_secret()` gains generic `_FILE` support but only `sftp.gainwell.password` has an env-var mapping today, so the visible effect is MFT-only. | +| 3 | Hotfix on main, or full SP-N flow? | Full SP-N flow — SP26 spec → plan → branch → merge. | +| 4 | Missing-file behavior: raise or fall through? | Raise `RuntimeError` (matches `auth/bootstrap.py:_read_secret`). | + +## 11. Open questions still pending + +None. Ready to implement. diff --git a/docs/superpowers/specs/2026-06-24-cyclone-sftp-polling-enablement-design.md b/docs/superpowers/specs/2026-06-24-cyclone-sftp-polling-enablement-design.md new file mode 100644 index 0000000..0bcad84 --- /dev/null +++ b/docs/superpowers/specs/2026-06-24-cyclone-sftp-polling-enablement-design.md @@ -0,0 +1,181 @@ +# SP25 — SFTP Polling Enablement: Design Spec + +**Date:** 2026-06-24 +**Status:** Draft, awaiting user sign-off +**Branch:** `sp25-sftp-polling-enablement` +**Aesthetic direction:** No new UI. Tiny backend delta on top of the SP16 scheduler, plus an operator runbook. The polling loop, handlers, idempotency table, and admin endpoints already exist and are not touched. + +## 1. Scope + +SP16 shipped a complete inbound-MFT polling scheduler — handlers for TA1, 999, 835, and 277CA; admin endpoints `start`, `stop`, `tick`, `status`, `processed-files`; idempotency via the `processed_inbound_files` table; opt-in autostart via the `CYCLONE_SCHEDULER_AUTOSTART` env var. SP13 wired real paramiko SFTP behind the same `SftpClient` interface. What is missing is **the last mile that lets an operator point that loop at `mft.gainwelltechnologies.com` and let it run unattended**. + +SP25 closes that gap. Three changes, all small: + +1. **Portable secrets.** Extend `cyclone.secrets.get_secret()` with a plain env-var fallback ahead of the macOS Keychain lookup, so the same code path serves a macOS workstation, a Linux server, and a Docker container without conditional deployment glue. +2. **Mutable clearhouse block.** Add `PATCH /api/clearhouse` so the operator can flip `sftp_block.stub` from `true` to `false` (and adjust host, port, paths, poll interval) without touching SQLite directly. The patch re-configures the running scheduler so the next tick uses the new block — no process restart required. +3. **Operator runbook.** A new `docs/RUNBOOK.md` documents the exact env-var values, the order of operations, and the verification commands. The runbook is the contract between this spec and any operator bringing up SFTP polling on a fresh host. + +Out of scope (explicit, each is its own future SP if requested): +- A frontend operator UI for scheduler start/stop/tick/status. SP16's endpoints exist; the operator uses curl. +- Multi-block polling (per-payer or per-tenant SFTP blocks). SP9 explicitly deferred this; today the loop watches the single dzinesco clearhouse block. +- A separate polling daemon process. SP16 runs in the FastAPI process; splitting it into a sidecar is a bigger redesign and not required for the user's "just turn it on" goal. +- The `_FILE` Docker-secret convention. The user explicitly chose plain env vars; the file-based fallback can be a follow-up if a Docker deployment lands without further changes here. +- Any 277CA-specific changes. TA1, 999, 835 are the user's target; 277CA already works via the SP16 handler and is left alone. + +## 2. Goals + +1. **A single curl can flip the loop from stub to real.** The operator does not need SQL access, a restart, or a redeploy to start polling real MFT. +2. **The secrets path is portable.** The same binary runs on macOS (Keychain) and on a Linux server (env var) with zero code changes between the two. +3. **The next tick after a PATCH uses the new block.** Operator-visible feedback is "I changed a setting, the next scheduler tick honors it" — not "I changed a setting, the change takes effect on restart." +4. **The runbook is self-contained.** An operator reading `docs/RUNBOOK.md` can bring up polling on a fresh host without reading the spec, the source, or any commit messages. + +## 3. Locked decisions + +### 3.1 Secret lookup order — env var first, Keychain second + +`cyclone.secrets.get_secret()` currently calls `keyring.get_password("cyclone", name)`. On any host without `keyring` installed (Linux servers, Docker containers without the `keyring` extra), it returns `None` and `SftpClient._connect()` then refuses to authenticate, raising a `RuntimeError` that the scheduler surfaces as a failed tick. + +The fix is a three-tier lookup, evaluated in this order: + +1. **Plain env var.** If `` is present in `os.environ`, return its `.strip()`-ed value. The variable name matches the Keychain account name verbatim — so `CYCLONE_SFTP_PASSWORD` is the env-var form of the Keychain account `sftp.gainwell.password`. The strip handles the common copy-paste-from-`.env` case where the value has a trailing newline. +2. **macOS Keychain.** Existing behavior, unchanged. Kept as a fallback so a macOS dev box that prefers the Keychain CLI workflow (`security add-generic-password`) still works without env-var exports. +3. **None.** Return `None` so the caller can decide what to do. `SftpClient._connect` already handles this with a precise error message. + +Rationale for "env var first, not Keychain first": env vars work everywhere; Keychain only works on macOS. Putting env var first means a Linux server needs nothing special, and a macOS dev box that exports the env var intentionally takes precedence over an unrelated Keychain entry from a previous test. The order is the inverse of the SP23 admin-bootstrap convention (which is `_FILE`-then-env-var) because the user explicitly chose plain env vars over Docker-secret files for this increment. + +### 3.2 `PATCH /api/clearhouse` — typed, gated, hot-reloading + +A new endpoint sits next to the existing `GET /api/clearhouse` (which is already gated by `matrix_gate`). The body is the full `Clearhouse` Pydantic model (single source of truth — no separate request/response DTOs). Behavior: + +- Validates the body via Pydantic. Invalid shape → 422. Unknown fields → 422 (Pydantic default for an extra-strict model). +- Writes the new row inside one `SessionLocal()` context. +- After commit, calls `scheduler_mod.configure_scheduler(sftp_block, force=True)` so the module singleton is replaced. +- If the scheduler was already running before the PATCH (autostart on, or a previous manual start), the helper restarts it against the new block. The in-flight tick, if any, finishes against the old client; the next tick uses the new one. Coalescing is already handled by `Scheduler._tick_in_progress`. +- If the scheduler was not running, the PATCH just leaves it stopped. + +Auth: same `Depends(matrix_gate)` as every other `/api/admin/*` and `/api/clearhouse*` endpoint. No new permission tier. + +Validation rules to enforce on the body: +- `sftp_block.stub` must be a real bool, not a stringified "true"/"false". +- `sftp_block.host`, when `stub` is `false`, must be non-empty. +- `sftp_block.paths.inbound` must be non-empty. +- `sftp_block.password_keychain_account`, when `stub` is `false`, must be non-empty (otherwise `SftpClient._connect` raises an unclear error at next tick). + +The endpoint returns the post-update `Clearhouse` (same shape as `GET /api/clearhouse`) so the operator can pipe-verify the change in one round-trip. + +### 3.3 No scheduler restart, just a reconfigure + +The scheduler is an `asyncio.Task` with a single `_run()` loop. `configure_scheduler(force=True)` replaces the module-level `_scheduler` singleton. The helper also has to handle the "scheduler was running" case: cancel the existing task with a 30-second wait (matching the existing `Scheduler.stop()` timeout), then start the new instance. The implementation lives in `cyclone.scheduler` as a new `reconfigure_scheduler()` function so the API endpoint stays thin. + +This avoids the operator-visible cliff of "I changed a setting, now I have to restart the API." The existing `stop()` semantics — drain the in-flight tick, then exit — carry over to the reconfigure path. + +### 3.4 Runbook placement — new `docs/RUNBOOK.md` + +A new top-level doc dedicated to operator procedures. Fits the SP23 Docker posture which already implies operator-facing documentation. The runbook contains: + +- **Prerequisites** — Python 3.11+, `keyring` optional, outbound network reachability to `mft.gainwelltechnologies.com:22`. +- **Env-var reference** — table of every `CYCLONE_*` env var the operator might set for this increment (password, scheduler autostart, scheduler poll interval, log level). +- **First-time setup** — set `CYCLONE_SFTP_PASSWORD`, PATCH to flip stub false, restart (or rely on autostart on next launch), verify via `GET /api/admin/scheduler/status`. +- **Verification commands** — exact curl incantations for status, tick-now, processed-files. Including how to filter for `status=error` rows. +- **Troubleshooting** — what each error class means (`AuthenticationException` from paramiko → wrong password; `IOError` from `listdir_attr` → wrong inbound path; tick records zero files → SFTP server has nothing in the inbound dir yet). +- **macOS dev box variant** — using `security add-generic-password` instead of the env var. + +The runbook is committed in the same PR as the code. No doc-only follow-up. + +### 3.5 Idempotency is unchanged + +The `processed_inbound_files` table, the `(sftp_block_name, name)` unique index, and the `STATUS_*` taxonomy are all unchanged. A re-tick after a PATCH still skips files that were already processed against any previous block (keyed by `sftp_block_name` and the inbound filename). The PATCH does not need to clear or backfill any rows. + +### 3.6 Audit log stays clean + +Inbound file processing is operational metadata, not part of the HIPAA audit chain (per the existing SP16 scheduler docstring and the SP11 audit-log design). SP25 does not change that boundary — PATCH on the clearhouse row is itself not audited; the resulting inbound-file processing continues to surface via `processed_inbound_files` and the existing pubsub events (`ack_received`, `claim.payer_rejected`, `claim_submitted`, `remittance_written`). + +## 4. Files + +**Modified:** +- `backend/src/cyclone/secrets.py` — add env-var lookup ahead of Keychain. New `get_secret()` docstring describing the three-tier lookup. No new public symbols beyond a small constant for the env-var-prefix mapping. +- `backend/src/cyclone/scheduler.py` — add `reconfigure_scheduler(sftp_block)` helper that cancels any running task (with the existing 30-second drain), replaces the singleton, and restarts if the previous one was running. +- `backend/src/cyclone/api.py` — add `PATCH /api/clearhouse` next to the existing `GET /api/clearhouse` and `POST /api/clearhouse/submit`. Thin handler: validate body → `store.update_clearhouse()` → `_scheduler_mod.reconfigure_scheduler()` → return the new row. +- `backend/src/cyclone/store.py` — add `update_clearhouse(block: Clearhouse) -> Clearhouse` that replaces the singleton row in a single session. Idempotent in shape but the call site is the PATCH endpoint, not lifespan, so idempotency is a nice-to-have not a hard requirement. + +**New:** +- `backend/tests/test_secrets_envvar.py` — covers all three tiers plus the `.strip()` and `None` paths. +- `backend/tests/test_api_clearhouse_patch.py` — covers happy path, 422 on invalid body, 401 on missing auth, scheduler reconfigure call, `matrix_gate` integration. +- `backend/tests/test_scheduler_reconfigure.py` — covers the cancel-then-restart behavior, the running-vs-stopped cases, and the in-flight tick drain. +- `docs/RUNBOOK.md` — the operator doc described in §3.4. + +**New migration:** none. The `clearhouse` table is unchanged. The `processed_inbound_files` table is unchanged. + +## 5. API surface + +| Method | Path | Body | Returns | Auth | +|---|---|---|---|---| +| `PATCH` | `/api/clearhouse` | Full `Clearhouse` model | Updated `Clearhouse` | `matrix_gate` (admin) | + +No new endpoints beyond `PATCH`. No new env-var-driven behavior beyond the documented ones. No new pubsub events. + +## 6. Env vars (operator-facing) + +| Variable | Default | Purpose | +|---|---|---| +| `CYCLONE_SFTP_PASSWORD` | unset | Plain-env-var form of the MFT password. Highest-priority lookup in `secrets.get_secret()`. Stripped of leading/trailing whitespace. | +| `CYCLONE_SCHEDULER_AUTOSTART` | unset (falsy) | When `1`/`true`/`yes`, the scheduler's `start()` is called from the FastAPI lifespan. Existing behavior, unchanged. | +| `CYCLONE_SCHEDULER_POLL_SECONDS` | `60` | Poll interval. Existing behavior, unchanged. | + +The user explicitly chose plain env var over Docker-secret file convention. If a Docker deployment lands later, a follow-up SP can add a `_FILE` tier ahead of the plain env var without changing this spec's contracts. + +## 7. Validation rules + +No new `R_*` rule IDs in `cyclone.validation.rules`. The Pydantic model for `Clearhouse.sftp_block` already enforces the field shapes; the endpoint just relies on those. Additional inline checks (non-empty `host` when `stub=false`, etc.) are Pydantic `field_validator`s on `SftpBlock`. + +## 8. Testing plan + +1. **`test_secrets_envvar.py`** — 8 cases: + - Env var set, Keychain present → env wins (returned). + - Env var set with trailing `\n` → returns stripped value. + - Env var set, Keychain absent → env returned. + - Env var absent, Keychain present → Keychain returned. + - Both absent → returns `None`. + - `keyring` library missing (monkeypatch the lazy import) → falls through to `None`. + - Env var value is the empty string → treated as absent (return `None`, do not propagate). + - Env var present but file path is unreadable (deferred — not applicable since `_FILE` is out of scope; skip). + +2. **`test_api_clearhouse_patch.py`** — 7 cases: + - PATCH happy path: stub=true → stub=false, host="mft.gainwelltechnologies.com", GET reflects change. + - PATCH with `stub="yes"` (string) → 422. + - PATCH with missing `sftp_block.host` and `stub=false` → 422. + - PATCH without session cookie → 401 (matrix_gate). + - PATCH with `password_keychain_account=""` and `stub=false` → 422. + - PATCH triggers `configure_scheduler(force=True)` — verified by mocking. + - PATCH returns the post-update row, identical shape to `GET /api/clearhouse`. + +3. **`test_scheduler_reconfigure.py`** — 5 cases: + - Scheduler not running → PATCH leaves it stopped, singleton replaced. + - Scheduler running → PATCH cancels old task, starts new one, no overlapping ticks. + - PATCH during a tick → in-flight tick completes against old client, next tick uses new client. + - `reconfigure_scheduler` 30-second drain timeout matches the existing `Scheduler.stop()` semantics. + - PATCH with stub=true and stub=false in sequence → both calls succeed, no leaked tasks. + +**Target backend test count after SP25: current + 8 + 7 + 5 = current + 20 tests.** + +## 9. Out of scope (future SPs) + +- **Frontend operator UI** for `start`/`stop`/`tick`/`status`/`processed-files`. The curl-based workflow documented in the runbook is sufficient for now. +- **Multi-block polling** (per-payer or per-tenant SFTP blocks). The dzinesco singleton is sufficient for one billing office. +- **Separate polling daemon** (`python -m cyclone poll`). The in-process scheduler is fine while the operator is fine with the API being up. +- **`_FILE` Docker-secret convention** for `get_secret()`. Plain env vars are enough today; add the file tier when a Docker deployment arrives. +- **Per-file-type fan-out** beyond TA1/999/835/277CA. Future X12 types (270/271/276/278/820/834/ENCR per the HCPF doc) can land as their parsers ship. + +## 10. Open questions resolved this session + +| # | Question | Resolution | +|---|---|---| +| 1 | What does "setup SFTP polling" mean given SP16 exists? | "Just turn it on" — small enablement increment, no from-scratch design. | +| 2 | Real MFT or stub? | Real Gainwell MFT. | +| 3 | macOS Keychain vs Docker secret file vs plain env var? | Plain env var (`CYCLONE_SFTP_PASSWORD`). User explicitly chose this. | +| 4 | Runbook placement? | New `docs/RUNBOOK.md`. | +| 5 | Hot-reload of the scheduler on PATCH, or restart required? | Hot-reload — the next tick uses the new block. | + +## 11. Open questions still pending + +None. Ready to implement.