diff --git a/CHANGELOG.md b/CHANGELOG.md index c72edd7..b0bac1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ ### Enhancements +#### Docker-secret support via `_FILE` env vars + +Any `PARSEDMARC_{SECTION}_{KEY}` environment variable can now also be supplied via a file by appending `_FILE` to its name (e.g. `PARSEDMARC_IMAP_PASSWORD_FILE=/run/secrets/imap_password`). The file's contents (with trailing CR/LF stripped) are used as the value. This is the same convention used by the official Postgres, MariaDB, and Redis container images, so credentials no longer have to appear in plain `environment:` blocks where `docker inspect`, container logs, and `/proc//environ` would expose them. + +When both the direct var and its `_FILE` companion are set, the file wins. A missing or unreadable file raises `ConfigurationError` rather than silently falling back to an empty value. The four pre-existing `*_file` config keys (`[general] log_file`, `[msgraph] token_file`, `[gmail_api] credentials_file`, `[gmail_api] token_file`) keep their direct-path semantics; wrap them in a Docker secret by doubling the suffix (`PARSEDMARC_GMAIL_API_CREDENTIALS_FILE_FILE`). + #### Support for RFC 9989 / RFC 9990 / RFC 9991 reports Adds parsing support for the final DMARC specification (RFC 9989), the new aggregate-report schema (RFC 9990), and the new failure-report format (RFC 9991), while preserving full RFC 7489 / RFC 6591 backward compatibility. diff --git a/docs/source/usage.md b/docs/source/usage.md index 576d389..fdceac9 100644 --- a/docs/source/usage.md +++ b/docs/source/usage.md @@ -613,6 +613,51 @@ services: PARSEDMARC_GENERAL_SAVE_FORENSIC: "true" ``` +### Docker secrets (`_FILE` suffix) + +Any `PARSEDMARC_{SECTION}_{KEY}` environment variable can also be supplied +via a file by appending `_FILE` to its name. The file's contents (with any +trailing CR/LF characters stripped) are used as the value. This is the +same convention used by the official Postgres, MariaDB, and Redis container +images, and is designed to plug straight into Docker / Docker Compose / +Kubernetes secrets so credentials never appear in plain `environment:` +blocks (where they would be readable via `docker inspect`, container logs, +and `/proc//environ`). + +The bare `DEBUG` / `PARSEDMARC_DEBUG` aliases and `PARSEDMARC_CONFIG_FILE` +do not have a `_FILE` form; only `PARSEDMARC_{SECTION}_{KEY}` vars resolved +to a known config section are eligible. + +If both the direct env var and the `_FILE` variant are set, the `_FILE` +variant wins. If the file does not exist or is unreadable, parsedmarc +exits with a configuration error rather than silently falling back to an +empty value. + +```yaml +secrets: + imap_password: + file: ./secrets/imap_password.txt + +services: + parsedmarc: + image: parsedmarc:latest + secrets: + - imap_password + environment: + PARSEDMARC_IMAP_HOST: imap.example.com + PARSEDMARC_IMAP_USER: dmarc@example.com + PARSEDMARC_IMAP_PASSWORD_FILE: /run/secrets/imap_password +``` + +Note that a small set of config keys whose own names already end in +`_file` (`[general] log_file`, `[msgraph] token_file`, +`[gmail_api] credentials_file`, `[gmail_api] token_file`) keep their +pre-existing meaning when set via `PARSEDMARC_..._FILE` — that env var is +the path itself, not a wrapper around a file containing the path. To pass +*those* paths via a Docker secret, double up the suffix +(`PARSEDMARC_GMAIL_API_CREDENTIALS_FILE_FILE`); the inner contents are +then read and stored as the `credentials_file` value. + ### Section name mapping For sections with underscores in the name, the full section name is used: diff --git a/parsedmarc/cli.py b/parsedmarc/cli.py index 3ba7551..9be3fd0 100644 --- a/parsedmarc/cli.py +++ b/parsedmarc/cli.py @@ -74,6 +74,12 @@ handler.setFormatter(formatter) logger.addHandler(handler) +class ConfigurationError(Exception): + """Raised when a configuration file has missing or invalid settings.""" + + pass + + def _str_to_list(s): """Converts a comma separated string to a list""" _list = s.split(",") @@ -108,6 +114,26 @@ _KNOWN_SECTIONS = frozenset( ) +# Short aliases that don't follow the PARSEDMARC_{SECTION}_{KEY} pattern. +_ENV_ALIASES: dict[str, tuple[str, str]] = { + "DEBUG": ("general", "debug"), + "PARSEDMARC_DEBUG": ("general", "debug"), +} + +# Real config keys whose own names end in ``_file``. For these the +# ``PARSEDMARC_..._FILE`` env var is the direct value (a path string), +# not a Docker-secret file reference. Keep in sync with ``_parse_config`` +# whenever a new ``*_file`` config key is added. +_DIRECT_FILE_KEYS = frozenset( + [ + "GENERAL_LOG_FILE", + "MSGRAPH_TOKEN_FILE", + "GMAIL_API_CREDENTIALS_FILE", + "GMAIL_API_TOKEN_FILE", + ] +) + + def _resolve_section_key(suffix: str) -> tuple: """Resolve an env var suffix like ``IMAP_PASSWORD`` to ``('imap', 'password')``. @@ -131,38 +157,76 @@ def _resolve_section_key(suffix: str) -> tuple: return best_section, best_key +def _read_secret_file(env_key: str, raw_path: str) -> str: + """Read a Docker-secret file referenced by a ``PARSEDMARC_..._FILE`` env var. + + Strips any trailing CR/LF from the file contents. Raises + ``ConfigurationError`` (not a silent fallback) when the file is missing, + unreadable, or not valid UTF-8. + """ + path = _expand_path(raw_path) + try: + with open(path, encoding="utf-8") as f: + return f.read().rstrip("\r\n") + except (OSError, UnicodeDecodeError) as exc: + raise ConfigurationError( + "Cannot read secret file for {0}: {1} ({2})".format( + env_key, path, exc.__class__.__name__ + ) + ) from exc + + def _apply_env_overrides(config: ConfigParser) -> None: """Inject ``PARSEDMARC_*`` environment variables into *config*. Environment variables matching ``PARSEDMARC_{SECTION}_{KEY}`` override - (or create) the corresponding config-file values. Sections are created + (or create) the corresponding config-file values. Sections are created automatically when they do not yet exist. + + A ``PARSEDMARC_{SECTION}_{KEY}_FILE`` variant reads the value from the + referenced file (Docker / Kubernetes secret convention). When both the + direct variable and its ``_FILE`` companion are set, the file wins. The + handful of real config keys whose own names end in ``_file`` (see + ``_DIRECT_FILE_KEYS``) keep their pre-existing direct-value semantics + and are not eligible for the secret-file wrap. """ prefix = "PARSEDMARC_" + file_suffix = "_FILE" - # Short aliases that don't follow the PARSEDMARC_{SECTION}_{KEY} pattern. - _ENV_ALIASES = { - "DEBUG": ("general", "debug"), - "PARSEDMARC_DEBUG": ("general", "debug"), - } + direct: dict[tuple[str, str], str] = {} + secrets: dict[tuple[str, str], str] = {} - for env_key, env_value in os.environ.items(): - if env_key in _ENV_ALIASES: - section, key = _ENV_ALIASES[env_key] - elif env_key.startswith(prefix) and env_key != "PARSEDMARC_CONFIG_FILE": - suffix = env_key[len(prefix) :] - section, key = _resolve_section_key(suffix) - else: + for env_key, value in os.environ.items(): + if env_key == "PARSEDMARC_CONFIG_FILE": continue + if env_key in _ENV_ALIASES: + direct[_ENV_ALIASES[env_key]] = value + continue + if not env_key.startswith(prefix): + continue + + key_body = env_key[len(prefix) :] + is_secret = key_body.endswith(file_suffix) and key_body not in _DIRECT_FILE_KEYS + + if is_secret: + section, key = _resolve_section_key(key_body[: -len(file_suffix)]) + else: + section, key = _resolve_section_key(key_body) if section is None: logger.debug("Ignoring unrecognized env var: %s", env_key) continue + if is_secret: + value = _read_secret_file(env_key, value) + secrets[(section, key)] = value + else: + direct[(section, key)] = value + # _FILE entries win over direct ones: dict-unpack lets later mappings overwrite. + for (section, key), value in {**direct, **secrets}.items(): if not config.has_section(section): config.add_section(section) - - config.set(section, key, env_value) + config.set(section, key, value) logger.debug("Config override from env: [%s] %s", section, key) @@ -266,12 +330,6 @@ def cli_parse( conn.close() -class ConfigurationError(Exception): - """Raised when a configuration file has missing or invalid settings.""" - - pass - - def _load_config(config_file: str | None = None) -> ConfigParser: """Load configuration from an INI file and/or environment variables. diff --git a/tests/test_cli.py b/tests/test_cli.py index 7d549fe..885b384 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -498,6 +498,238 @@ hosts = localhost f"Expected falsy for {false_val!r}", ) + def test_short_alias_debug(self): + """The bare DEBUG alias maps to [general] debug.""" + from parsedmarc.cli import _apply_env_overrides + + config = ConfigParser() + with patch.dict(os.environ, {"DEBUG": "true"}, clear=False): + _apply_env_overrides(config) + self.assertEqual(config.get("general", "debug"), "true") + + def test_short_alias_parsedmarc_debug(self): + """The PARSEDMARC_DEBUG alias maps to [general] debug.""" + from parsedmarc.cli import _apply_env_overrides + + config = ConfigParser() + with patch.dict(os.environ, {"PARSEDMARC_DEBUG": "true"}, clear=False): + _apply_env_overrides(config) + self.assertEqual(config.get("general", "debug"), "true") + + def test_file_env_var_reads_secret(self): + """*_FILE env vars are loaded from a file (Docker secret style).""" + from parsedmarc.cli import _apply_env_overrides + + with NamedTemporaryFile( + mode="w", suffix=".secret", delete=False, encoding="utf-8" + ) as f: + f.write("sekret-123\n") + secret_path = f.name + + try: + config = ConfigParser() + env = {"PARSEDMARC_IMAP_PASSWORD_FILE": secret_path} + with patch.dict(os.environ, env, clear=False): + _apply_env_overrides(config) + self.assertEqual(config.get("imap", "password"), "sekret-123") + finally: + os.unlink(secret_path) + + def test_file_env_var_strips_trailing_crlf(self): + """Leading and internal whitespace is preserved; trailing CR/LF is stripped.""" + from parsedmarc.cli import _apply_env_overrides + + with NamedTemporaryFile( + mode="w", suffix=".secret", delete=False, encoding="utf-8" + ) as f: + f.write(" pre inside\r\n") + secret_path = f.name + + try: + config = ConfigParser() + env = {"PARSEDMARC_IMAP_PASSWORD_FILE": secret_path} + with patch.dict(os.environ, env, clear=False): + _apply_env_overrides(config) + self.assertEqual(config.get("imap", "password"), " pre inside") + finally: + os.unlink(secret_path) + + def test_file_env_var_supersedes_direct_env(self): + """*_FILE wins when both the direct env var and _FILE are set.""" + from parsedmarc.cli import _apply_env_overrides + + with NamedTemporaryFile( + mode="w", suffix=".secret", delete=False, encoding="utf-8" + ) as f: + f.write("from-file") + secret_path = f.name + + try: + config = ConfigParser() + env = { + "PARSEDMARC_IMAP_PASSWORD": "from-env", + "PARSEDMARC_IMAP_PASSWORD_FILE": secret_path, + } + with patch.dict(os.environ, env, clear=False): + _apply_env_overrides(config) + self.assertEqual(config.get("imap", "password"), "from-file") + finally: + os.unlink(secret_path) + + def test_file_env_var_missing_file_raises(self): + """A missing secret file aborts with ConfigurationError.""" + from parsedmarc.cli import ConfigurationError, _apply_env_overrides + + config = ConfigParser() + env = {"PARSEDMARC_IMAP_PASSWORD_FILE": "/tmp/parsedmarc-nonexistent-secret"} + with patch.dict(os.environ, env, clear=False): + with self.assertRaises(ConfigurationError) as ctx: + _apply_env_overrides(config) + self.assertIn("PARSEDMARC_IMAP_PASSWORD_FILE", str(ctx.exception)) + + def test_file_env_var_unreadable_file_raises(self): + """A secret file we can't read aborts with ConfigurationError.""" + import platform + + # ``os.geteuid`` is POSIX-only; the ``platform.system() == "Windows"`` + # check short-circuits on Windows so the second clause never runs. + if platform.system() == "Windows" or os.geteuid() == 0: + self.skipTest("chmod 000 doesn't restrict the running user") + + from parsedmarc.cli import ConfigurationError, _apply_env_overrides + + with NamedTemporaryFile( + mode="w", suffix=".secret", delete=False, encoding="utf-8" + ) as f: + f.write("data") + secret_path = f.name + + try: + os.chmod(secret_path, 0o000) + config = ConfigParser() + env = {"PARSEDMARC_IMAP_PASSWORD_FILE": secret_path} + with patch.dict(os.environ, env, clear=False): + with self.assertRaises(ConfigurationError): + _apply_env_overrides(config) + finally: + os.chmod(secret_path, 0o600) + os.unlink(secret_path) + + def test_file_env_var_path_expansion(self): + """~ and $VAR references in the path are expanded.""" + from parsedmarc.cli import _apply_env_overrides + + with tempfile.TemporaryDirectory() as tmpdir: + secret_path = os.path.join(tmpdir, "secret") + with open(secret_path, "w", encoding="utf-8") as f: + f.write("expanded-value") + + config = ConfigParser() + env = { + "PARSEDMARC_TEST_SECRET_DIR": tmpdir, + "PARSEDMARC_IMAP_PASSWORD_FILE": "$PARSEDMARC_TEST_SECRET_DIR/secret", + } + with patch.dict(os.environ, env, clear=False): + _apply_env_overrides(config) + self.assertEqual(config.get("imap", "password"), "expanded-value") + + def test_file_env_var_unknown_section_ignored(self): + """_FILE vars whose base name doesn't resolve to a section are ignored. + + Uses ``clear=True`` so the assertion isn't perturbed by ambient + ``PARSEDMARC_*`` vars set in the dev shell or CI runner. + """ + from parsedmarc.cli import _apply_env_overrides + + config = ConfigParser() + env = {"PARSEDMARC_UNKNOWN_FOO_FILE": "/tmp/should-not-be-read"} + with patch.dict(os.environ, env, clear=True): + _apply_env_overrides(config) + self.assertEqual(config.sections(), []) + + def test_file_env_var_direct_file_keys_keep_direct_semantics(self): + """Config keys ending in _file (log_file, token_file, ...) stay direct.""" + from parsedmarc.cli import _apply_env_overrides + + config = ConfigParser() + env = { + "PARSEDMARC_GENERAL_LOG_FILE": "/var/log/parsedmarc.log", + "PARSEDMARC_GMAIL_API_CREDENTIALS_FILE": "/etc/parsedmarc/gmail.json", + "PARSEDMARC_GMAIL_API_TOKEN_FILE": "/etc/parsedmarc/gmail.token", + "PARSEDMARC_MSGRAPH_TOKEN_FILE": "/etc/parsedmarc/msgraph.token", + } + with patch.dict(os.environ, env, clear=False): + _apply_env_overrides(config) + self.assertEqual(config.get("general", "log_file"), "/var/log/parsedmarc.log") + self.assertEqual( + config.get("gmail_api", "credentials_file"), + "/etc/parsedmarc/gmail.json", + ) + self.assertEqual( + config.get("gmail_api", "token_file"), "/etc/parsedmarc/gmail.token" + ) + self.assertEqual( + config.get("msgraph", "token_file"), "/etc/parsedmarc/msgraph.token" + ) + + def test_file_env_var_double_suffix_wraps_direct_file_key(self): + """GMAIL_API_CREDENTIALS_FILE_FILE provides the file path via a secret.""" + from parsedmarc.cli import _apply_env_overrides + + with NamedTemporaryFile( + mode="w", suffix=".secret", delete=False, encoding="utf-8" + ) as f: + f.write("/run/secrets/real-gmail-credentials.json\n") + secret_path = f.name + + try: + config = ConfigParser() + env = {"PARSEDMARC_GMAIL_API_CREDENTIALS_FILE_FILE": secret_path} + with patch.dict(os.environ, env, clear=False): + _apply_env_overrides(config) + self.assertEqual( + config.get("gmail_api", "credentials_file"), + "/run/secrets/real-gmail-credentials.json", + ) + finally: + os.unlink(secret_path) + + def test_direct_file_keys_matches_parse_config_source(self): + """``_DIRECT_FILE_KEYS`` must cover every ``*_file`` key in ``_parse_config``. + + Regression guard for the keep-in-sync comment: when someone adds a new + ``[section] some_file`` config option in ``_parse_config`` without + also extending ``_DIRECT_FILE_KEYS``, ``PARSEDMARC_SECTION_SOME_FILE`` + would silently be treated as a Docker-secret wrapper (and try to read + a file at the supplied path) instead of as the direct value. + """ + import re + import inspect + import parsedmarc.cli as cli_module + + # Scan the cli source for every ``
_config[...]("_file")`` + # / ``["_file"]`` access and rebuild the expected upper-case set. + # Skip ``_filename`` keys (e.g. ``aggregate_json_filename``). + src = inspect.getsource(cli_module) + pattern = re.compile( + r'(\w+?)_config(?:\.get|\[)\(?["\'](\w+_file)["\']', + ) + seen: set[str] = set() + for sect_var, key in pattern.findall(src): + if key.endswith("_filename"): + continue + # Map the local variable name (graph_config / general_config / + # gmail_api_config / ...) to its config-section name. The + # convention is "
_config", but ``msgraph`` is bound to + # ``graph_config`` — handle that one alias. + section = "msgraph" if sect_var == "graph" else sect_var + seen.add(f"{section.upper()}_{key.upper()}") + self.assertEqual( + seen, + set(cli_module._DIRECT_FILE_KEYS), + "_DIRECT_FILE_KEYS is out of sync with *_file keys in _parse_config", + ) + class TestGmailAuthModes(unittest.TestCase): @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")