Files
cyclone/docs/superpowers/plans/2026-06-24-cyclone-sftp-password-file-companion.md

23 KiB
Raw Permalink Blame History

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 — <env_name>_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


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 <env_name> + "_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:

"""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 `<env_name>_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 <env_name>_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:

"""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 ``<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 '<password>'

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. ``<env_name>_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/<name>`` 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 ``<env_name>`` — 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 = "<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 ``<env_name>_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
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:

      # 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:

  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
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:

| `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):

### 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:

    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:

    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:

    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
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):

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
git branch -d sp26-sftp-password-file-companion
git push origin --delete sp26-sftp-password-file-companion  # if remote exists

Self-Review Checklist

  • Spec coverage: §1 (scope) → Tasks 14; §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.
  • 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.
  • 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.
  • 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.