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

559 lines
23 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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`](../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`:
```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 `<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`:
```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 ``<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**
```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 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.
- [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.