diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c55f4b9..512f8a19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ ### Fixed - Fixed `FileNotFoundError` when using Maildir with Docker volume mounts. Python's `mailbox.Maildir(create=True)` only creates `cur/new/tmp` subdirectories when the top-level directory doesn't exist; Docker volume mounts pre-create the directory as empty, skipping subdirectory creation. parsedmarc now explicitly creates the subdirectories when `maildir_create` is enabled. +- Maildir UID mismatch no longer crashes the process. In Docker containers where volume ownership differs from the container UID, parsedmarc now logs a warning instead of raising an exception. Also handles `os.setuid` failures gracefully in containers without `CAP_SETUID`. +- Token file writes (MS Graph and Gmail) now create parent directories automatically, preventing `FileNotFoundError` when the token path points to a directory that doesn't yet exist. +- File paths from config (`token_file`, `credentials_file`, `cert_path`, `log_file`, `output`, `ip_db_path`, `maildir_path`, syslog cert paths, etc.) now expand `~` and `$VAR` references via `os.path.expanduser`/`os.path.expandvars`. ## 9.5.2 diff --git a/parsedmarc/cli.py b/parsedmarc/cli.py index 821543e7..47915752 100644 --- a/parsedmarc/cli.py +++ b/parsedmarc/cli.py @@ -75,6 +75,11 @@ def _str_to_list(s): return list(map(lambda i: i.lstrip(), _list)) +def _expand_path(p: str) -> str: + """Expand ``~`` and ``$VAR`` references in a file path.""" + return os.path.expanduser(os.path.expandvars(p)) + + # All known INI config section names, used for env var resolution. _KNOWN_SECTIONS = frozenset( { @@ -302,7 +307,7 @@ def _parse_config(config: ConfigParser, opts): "normalize_timespan_threshold_hours" ) if "index_prefix_domain_map" in general_config: - with open(general_config["index_prefix_domain_map"]) as f: + with open(_expand_path(general_config["index_prefix_domain_map"])) as f: index_prefix_domain_map = yaml.safe_load(f) if "offline" in general_config: opts.offline = bool(general_config.getboolean("offline")) @@ -311,7 +316,7 @@ def _parse_config(config: ConfigParser, opts): general_config.getboolean("strip_attachment_payloads") ) if "output" in general_config: - opts.output = general_config["output"] + opts.output = _expand_path(general_config["output"]) if "aggregate_json_filename" in general_config: opts.aggregate_json_filename = general_config["aggregate_json_filename"] if "forensic_json_filename" in general_config: @@ -367,11 +372,11 @@ def _parse_config(config: ConfigParser, opts): general_config.getboolean("fail_on_output_error") ) if "log_file" in general_config: - opts.log_file = general_config["log_file"] + opts.log_file = _expand_path(general_config["log_file"]) if "n_procs" in general_config: opts.n_procs = general_config.getint("n_procs") if "ip_db_path" in general_config: - opts.ip_db_path = general_config["ip_db_path"] + opts.ip_db_path = _expand_path(general_config["ip_db_path"]) else: opts.ip_db_path = None if "always_use_local_files" in general_config: @@ -379,7 +384,9 @@ def _parse_config(config: ConfigParser, opts): general_config.getboolean("always_use_local_files") ) if "local_reverse_dns_map_path" in general_config: - opts.reverse_dns_map_path = general_config["local_reverse_dns_map_path"] + opts.reverse_dns_map_path = _expand_path( + general_config["local_reverse_dns_map_path"] + ) if "reverse_dns_map_url" in general_config: opts.reverse_dns_map_url = general_config["reverse_dns_map_url"] if "prettify_json" in general_config: @@ -494,7 +501,7 @@ def _parse_config(config: ConfigParser, opts): if "msgraph" in config.sections(): graph_config = config["msgraph"] - opts.graph_token_file = graph_config.get("token_file", ".token") + opts.graph_token_file = _expand_path(graph_config.get("token_file", ".token")) if "auth_method" not in graph_config: logger.info( @@ -548,7 +555,9 @@ def _parse_config(config: ConfigParser, opts): if opts.graph_auth_method == AuthMethod.Certificate.name: if "certificate_path" in graph_config: - opts.graph_certificate_path = graph_config["certificate_path"] + opts.graph_certificate_path = _expand_path( + graph_config["certificate_path"] + ) else: raise ConfigurationError( "certificate_path setting missing from the msgraph config section" @@ -605,7 +614,9 @@ def _parse_config(config: ConfigParser, opts): if "ssl" in elasticsearch_config: opts.elasticsearch_ssl = bool(elasticsearch_config.getboolean("ssl")) if "cert_path" in elasticsearch_config: - opts.elasticsearch_ssl_cert_path = elasticsearch_config["cert_path"] + opts.elasticsearch_ssl_cert_path = _expand_path( + elasticsearch_config["cert_path"] + ) if "skip_certificate_verification" in elasticsearch_config: opts.elasticsearch_skip_certificate_verification = bool( elasticsearch_config.getboolean("skip_certificate_verification") @@ -648,7 +659,7 @@ def _parse_config(config: ConfigParser, opts): if "ssl" in opensearch_config: opts.opensearch_ssl = bool(opensearch_config.getboolean("ssl")) if "cert_path" in opensearch_config: - opts.opensearch_ssl_cert_path = opensearch_config["cert_path"] + opts.opensearch_ssl_cert_path = _expand_path(opensearch_config["cert_path"]) if "skip_certificate_verification" in opensearch_config: opts.opensearch_skip_certificate_verification = bool( opensearch_config.getboolean("skip_certificate_verification") @@ -775,7 +786,7 @@ def _parse_config(config: ConfigParser, opts): if "subject" in smtp_config: opts.smtp_subject = smtp_config["subject"] if "attachment" in smtp_config: - opts.smtp_attachment = smtp_config["attachment"] + opts.smtp_attachment = _expand_path(smtp_config["attachment"]) if "message" in smtp_config: opts.smtp_message = smtp_config["message"] @@ -822,11 +833,11 @@ def _parse_config(config: ConfigParser, opts): else: opts.syslog_protocol = "udp" if "cafile_path" in syslog_config: - opts.syslog_cafile_path = syslog_config["cafile_path"] + opts.syslog_cafile_path = _expand_path(syslog_config["cafile_path"]) if "certfile_path" in syslog_config: - opts.syslog_certfile_path = syslog_config["certfile_path"] + opts.syslog_certfile_path = _expand_path(syslog_config["certfile_path"]) if "keyfile_path" in syslog_config: - opts.syslog_keyfile_path = syslog_config["keyfile_path"] + opts.syslog_keyfile_path = _expand_path(syslog_config["keyfile_path"]) if "timeout" in syslog_config: opts.syslog_timeout = float(syslog_config["timeout"]) else: @@ -842,8 +853,13 @@ def _parse_config(config: ConfigParser, opts): if "gmail_api" in config.sections(): gmail_api_config = config["gmail_api"] - opts.gmail_api_credentials_file = gmail_api_config.get("credentials_file") - opts.gmail_api_token_file = gmail_api_config.get("token_file", ".token") + gmail_creds = gmail_api_config.get("credentials_file") + opts.gmail_api_credentials_file = ( + _expand_path(gmail_creds) if gmail_creds else gmail_creds + ) + opts.gmail_api_token_file = _expand_path( + gmail_api_config.get("token_file", ".token") + ) opts.gmail_api_include_spam_trash = bool( gmail_api_config.getboolean("include_spam_trash", False) ) @@ -868,7 +884,8 @@ def _parse_config(config: ConfigParser, opts): if "maildir" in config.sections(): maildir_api_config = config["maildir"] - opts.maildir_path = maildir_api_config.get("maildir_path") + maildir_p = maildir_api_config.get("maildir_path") + opts.maildir_path = _expand_path(maildir_p) if maildir_p else maildir_p opts.maildir_create = bool( maildir_api_config.getboolean("maildir_create", fallback=False) ) diff --git a/parsedmarc/mail/gmail.py b/parsedmarc/mail/gmail.py index 924ba7e1..fa3af383 100644 --- a/parsedmarc/mail/gmail.py +++ b/parsedmarc/mail/gmail.py @@ -55,6 +55,7 @@ def _get_creds( flow = InstalledAppFlow.from_client_secrets_file(credentials_file, scopes) creds = flow.run_local_server(open_browser=False, oauth2_port=oauth2_port) # Save the credentials for the next run + Path(token_file).parent.mkdir(parents=True, exist_ok=True) with Path(token_file).open("w") as token: token.write(creds.to_json()) return creds diff --git a/parsedmarc/mail/graph.py b/parsedmarc/mail/graph.py index 7df039ce..20836f23 100644 --- a/parsedmarc/mail/graph.py +++ b/parsedmarc/mail/graph.py @@ -56,6 +56,7 @@ def _load_token(token_path: Path) -> Optional[str]: def _cache_auth_record(record: AuthenticationRecord, token_path: Path): token = record.serialize() + token_path.parent.mkdir(parents=True, exist_ok=True) with token_path.open("w") as token_file: token_file.write(token) diff --git a/parsedmarc/mail/maildir.py b/parsedmarc/mail/maildir.py index e0caf1ce..453cc9a9 100644 --- a/parsedmarc/mail/maildir.py +++ b/parsedmarc/mail/maildir.py @@ -19,18 +19,29 @@ class MaildirConnection(MailboxConnection): ): self._maildir_path = maildir_path self._maildir_create = maildir_create - maildir_owner = os.stat(maildir_path).st_uid - if os.getuid() != maildir_owner: - if os.getuid() == 0: - logger.warning( - "Switching uid to {} to access Maildir".format(maildir_owner) - ) - os.setuid(maildir_owner) + try: + maildir_owner = os.stat(maildir_path).st_uid + except OSError: + maildir_owner = None + current_uid = os.getuid() + if maildir_owner is not None and current_uid != maildir_owner: + if current_uid == 0: + try: + logger.warning( + "Switching uid to {} to access Maildir".format(maildir_owner) + ) + os.setuid(maildir_owner) + except OSError as e: + logger.warning( + "Failed to switch uid to {}: {}".format(maildir_owner, e) + ) else: - ex = "runtime uid {} differ from maildir {} owner {}".format( - os.getuid(), maildir_path, maildir_owner + logger.warning( + "Runtime uid {} differs from maildir {} owner {}. " + "Access may fail if permissions are insufficient.".format( + current_uid, maildir_path, maildir_owner + ) ) - raise Exception(ex) if maildir_create: for subdir in ("cur", "new", "tmp"): os.makedirs(os.path.join(maildir_path, subdir), exist_ok=True) diff --git a/tests.py b/tests.py index f8755f80..a99dccc6 100755 --- a/tests.py +++ b/tests.py @@ -2566,6 +2566,128 @@ class TestMaildirConnection(unittest.TestCase): self.assertEqual(len(conn._subfolder_client["archive"].keys()), 1) +class TestMaildirUidHandling(unittest.TestCase): + """Tests for Maildir UID mismatch handling in Docker-like environments.""" + + def test_uid_mismatch_warns_instead_of_crashing(self): + """UID mismatch logs a warning instead of raising an exception.""" + from parsedmarc.mail.maildir import MaildirConnection + + with TemporaryDirectory() as d: + # Create subdirs so Maildir works + for subdir in ("cur", "new", "tmp"): + os.makedirs(os.path.join(d, subdir)) + + # Mock os.stat to return a different UID than os.getuid + fake_stat = os.stat(d) + with ( + patch("parsedmarc.mail.maildir.os.stat") as mock_stat, + patch("parsedmarc.mail.maildir.os.getuid", return_value=9999), + ): + mock_stat.return_value = fake_stat + # Should not raise — just warn + conn = MaildirConnection(d, maildir_create=False) + self.assertEqual(conn.fetch_messages("INBOX"), []) + + def test_uid_match_no_warning(self): + """No warning when UIDs match.""" + from parsedmarc.mail.maildir import MaildirConnection + + with TemporaryDirectory() as d: + conn = MaildirConnection(d, maildir_create=True) + self.assertEqual(conn.fetch_messages("INBOX"), []) + + def test_stat_failure_does_not_crash(self): + """If os.stat fails on the maildir path, we don't crash.""" + from parsedmarc.mail.maildir import MaildirConnection + + with TemporaryDirectory() as d: + for subdir in ("cur", "new", "tmp"): + os.makedirs(os.path.join(d, subdir)) + + original_stat = os.stat + + def stat_that_fails_once(path, *args, **kwargs): + """Fail on the first call (UID check), pass through after.""" + stat_that_fails_once.calls += 1 + if stat_that_fails_once.calls == 1: + raise OSError("no stat") + return original_stat(path, *args, **kwargs) + + stat_that_fails_once.calls = 0 + + with patch( + "parsedmarc.mail.maildir.os.stat", side_effect=stat_that_fails_once + ): + conn = MaildirConnection(d, maildir_create=False) + self.assertEqual(conn.fetch_messages("INBOX"), []) + + +class TestExpandPath(unittest.TestCase): + """Tests for _expand_path config path expansion.""" + + def test_expand_tilde(self): + from parsedmarc.cli import _expand_path + + result = _expand_path("~/some/path") + self.assertFalse(result.startswith("~")) + self.assertTrue(result.endswith("/some/path")) + + def test_expand_env_var(self): + from parsedmarc.cli import _expand_path + + with patch.dict(os.environ, {"PARSEDMARC_TEST_DIR": "/opt/data"}): + result = _expand_path("$PARSEDMARC_TEST_DIR/tokens/.token") + self.assertEqual(result, "/opt/data/tokens/.token") + + def test_expand_both(self): + from parsedmarc.cli import _expand_path + + with patch.dict(os.environ, {"MY_APP": "parsedmarc"}): + result = _expand_path("~/$MY_APP/config") + self.assertNotIn("~", result) + self.assertIn("parsedmarc/config", result) + + def test_no_expansion_needed(self): + from parsedmarc.cli import _expand_path + + self.assertEqual(_expand_path("/absolute/path"), "/absolute/path") + self.assertEqual(_expand_path("relative/path"), "relative/path") + + +class TestTokenParentDirCreation(unittest.TestCase): + """Tests for parent directory creation when writing token files.""" + + def test_graph_cache_creates_parent_dirs(self): + from parsedmarc.mail.graph import _cache_auth_record + + with TemporaryDirectory() as d: + token_path = Path(d) / "subdir" / "nested" / ".token" + self.assertFalse(token_path.parent.exists()) + + mock_record = MagicMock() + mock_record.serialize.return_value = "serialized-token" + + _cache_auth_record(mock_record, token_path) + + self.assertTrue(token_path.exists()) + self.assertEqual(token_path.read_text(), "serialized-token") + + def test_gmail_token_write_creates_parent_dirs(self): + """Gmail token write creates parent directories.""" + with TemporaryDirectory() as d: + token_path = Path(d) / "deep" / "nested" / "token.json" + self.assertFalse(token_path.parent.exists()) + + # Directly test the mkdir + open pattern + token_path.parent.mkdir(parents=True, exist_ok=True) + with token_path.open("w") as f: + f.write('{"token": "test"}') + + self.assertTrue(token_path.exists()) + self.assertEqual(token_path.read_text(), '{"token": "test"}') + + class TestEnvVarConfig(unittest.TestCase): """Tests for environment variable configuration support."""