diff --git a/backend/src/cyclone/secrets.py b/backend/src/cyclone/secrets.py index f09cefc..30d4379 100644 --- a/backend/src/cyclone/secrets.py +++ b/backend/src/cyclone/secrets.py @@ -15,18 +15,27 @@ 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: +SP25 + SP26: the lookup now resolves the secret in four tiers, in +this order: - 1. Plain env var named exactly ```` — highest priority. + 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. - 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 + 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. - 3. ``None`` — caller decides what to do (``SftpClient._connect`` + 4. ``None`` — caller decides what to do (``SftpClient._connect`` raises a precise ``RuntimeError`` on real-mode auth). """ @@ -34,6 +43,7 @@ from __future__ import annotations import logging import os +from pathlib import Path from typing import Optional log = logging.getLogger(__name__) @@ -51,29 +61,52 @@ 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 by name. Three-tier lookup: env var, Keychain, None. + """Fetch a secret by name. Four-tier lookup: _FILE, 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. + 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_NAME_FOR.get(name, name) + 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() 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"