docs(spec): SP26 SFTP Password File Companion — initial design
This commit is contained in:
Nora
@@ -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/<name>` 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 `<NAME>_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. **`<env_name>_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_name>` 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 `<env_name> + "_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 `<NAME>_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.
|
||||
Reference in New Issue
Block a user