feat(sp25): secrets.get_secret() env-var-first lookup

backend/src/cyclone/secrets.py

@@ -14,11 +14,26 @@ Setup (one-time, by the operator):
 
 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 ``<name>`` — 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__)
@@ -37,23 +52,44 @@ except ImportError:
 
 
 def get_secret(name: str) -> Optional[str]:
-    """Fetch a secret from macOS Keychain by name.
+    """Fetch a secret by name. Three-tier lookup: env var, Keychain, None.
 
     Args:
-        name: The Keychain account (e.g. "sftp.gainwell.password").
+        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_<UPPER_SNAKE_CASE>`` form). Operators who want
+            the env-var shortcut set the var named exactly ``<name>``.
+
+            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, or None if the entry is missing or keyring
+        The secret string (stripped), or ``None`` if no tier produced
-        is not installed.
+        a value.
     """
-    if not _HAS_KEYRING:
+    env_name = _ENV_NAME_FOR.get(name, name)
-        log.warning("keyring not installed; get_secret(%r) returning None", name)
+    raw = os.environ.get(env_name)
-        return None
+    if raw is not None:
-    try:
+        stripped = raw.strip()
-        return keyring.get_password(SERVICE_NAME, name)
+        if stripped:
-    except Exception as exc:  # noqa: BLE001 (Keychain can raise anything)
+            log.debug("Secret %r resolved from env var %r", name, env_name)
-        log.warning("Keychain get_secret(%r) failed: %s", name, exc)
+            return stripped
-        return None
+        # 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 +113,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",
+}

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"