mirror of
https://github.com/domainaware/parsedmarc.git
synced 2026-09-10 16:08:00 +00:00
Fix CodeQL findings: use sys.exit(), remove dead index_date stores (#898)
py/use-of-exit-or-quit (29 sites): replace bare `exit(...)` calls with `sys.exit(...)` in cli.py (20 sites), sortlists.py (4 sites), and find_unknown_base_reverse_dns.py (5 sites). `exit` is injected into the interactive namespace by the `site` module, so it is not guaranteed to exist under `python -S` or an embedded interpreter — the very error paths that called it would raise NameError instead of exiting cleanly. `sys.exit` is always available and was already used elsewhere in these files. sortlists.py did not import `sys`; the import was added to its existing stdlib import block. find_unknown_base_reverse_dns.py already imported `sys` (used at line 71), so only the call sites changed there. Checked tests/test_cli.py for anything patching `exit` directly — none do; the SystemExit-based assertions exercise the same behavior for either spelling, so no test changes were needed. py/multiple-definition (4 sites): removed the dead `index_date` computation in save_aggregate_report_to_elasticsearch() (elastic.py:944-947) and save_aggregate_report_to_opensearch() (opensearch.py:875-878). Both blocks derived index_date from the report-level begin_date, but the per-record loop immediately below unconditionally recomputes index_date from each record's own begin_date before the only read of the variable (the index name build near the end of the loop body). Verified by reading both functions that no code between the outer assignment and the loop's recompute reads index_date, and that the recompute is unconditional on every path that reaches the read, before deleting. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
e1c2e42d5d
commit
4298603713
@@ -5,6 +5,7 @@
|
||||
### Changes
|
||||
|
||||
- **The prebuilt Docker image (`ghcr.io/domainaware/parsedmarc`) is roughly 40% smaller to pull** ([#893](https://github.com/domainaware/parsedmarc/pull/893)). The runtime stage copied the built wheel out of the build stage and deleted it again at the end of the next `RUN`, but a `RUN` can only write a whiteout over a layer an earlier instruction already committed: the wheel shipped in every published image and every `docker pull` downloaded it (10,713,473 bytes of the 11.0.0 image, on both architectures). The wheel is now bind-mounted from the build stage instead, and a bind mount is never committed to a layer. `pip install` also runs with `--no-cache-dir`, which drops a further ~99 MB of pip's download cache that the image had been carrying in the same layer as `site-packages`. Measured on linux/amd64: 272,138,724 compressed bytes across six layers before, 163,757,439 across five after.
|
||||
- **Bare `exit(...)` calls are now `sys.exit(...)` throughout the CLI and the maintainer map-tooling scripts.** `exit` is installed into `builtins` by the `site` module, not just for interactive sessions, so it isn't guaranteed to exist under `python -S` or an embedded interpreter that skips `site` — the failure paths that called it would raise `NameError` instead of actually exiting. `sys.exit` is always available and was already used elsewhere in `cli.py` and `find_unknown_base_reverse_dns.py`. Also removed the dead, immediately-recomputed `index_date` stores in the Elasticsearch and OpenSearch aggregate-report savers; behavior is unchanged.
|
||||
|
||||
## 11.0.1
|
||||
|
||||
|
||||
+23
-23
@@ -231,7 +231,7 @@ def _log_msgraph_failure(
|
||||
fetch, send, or watch failure, identifying the mailbox/tenant/auth
|
||||
method and the Graph request-id/client-request-id when available.
|
||||
The full traceback is preserved at --debug via a follow-up DEBUG
|
||||
record. Never calls exit() - the call site keeps its own exit(1)."""
|
||||
record. Never calls sys.exit() - the call site keeps its own sys.exit(1)."""
|
||||
if isinstance(error, APIError):
|
||||
detail = getattr(error, "primary_message", None) or error.message or str(error)
|
||||
detail = " ".join(str(detail).split())
|
||||
@@ -2766,7 +2766,7 @@ def _main():
|
||||
index_prefix_domain_map = _parse_config(config, opts)
|
||||
except ConfigurationError as e:
|
||||
logger.critical(str(e))
|
||||
exit(-1)
|
||||
sys.exit(-1)
|
||||
|
||||
logger.setLevel(logging.ERROR)
|
||||
|
||||
@@ -2798,7 +2798,7 @@ def _main():
|
||||
and len(opts.file_path) == 0
|
||||
):
|
||||
logger.error("You must supply input files or a mailbox connection")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
|
||||
logger.info("Starting parsedmarc")
|
||||
|
||||
@@ -2814,7 +2814,7 @@ def _main():
|
||||
configure_ipinfo_api(opts.ipinfo_api_token)
|
||||
except InvalidIPinfoAPIKey as e:
|
||||
logger.critical(str(e))
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
|
||||
load_psl_overrides(
|
||||
always_use_local_file=opts.always_use_local_files,
|
||||
@@ -2835,7 +2835,7 @@ def _main():
|
||||
break
|
||||
except ConfigurationError as e:
|
||||
logger.critical(str(e))
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
except Exception as error_:
|
||||
if attempt < max_retries:
|
||||
logger.warning(
|
||||
@@ -2849,10 +2849,10 @@ def _main():
|
||||
retry_delay *= 2
|
||||
else:
|
||||
logger.error(f"Output client error: {error_}")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
|
||||
# Always close output clients on the way out (normal return,
|
||||
# exit(N), uncaught exception, or SystemExit from a signal-driven
|
||||
# sys.exit(N), uncaught exception, or SystemExit from a signal-driven
|
||||
# shutdown). atexit does NOT fire on os._exit(130) — that's
|
||||
# intentional for the SIGINT double-tap. The lambda closes whatever
|
||||
# `clients` currently points at, so a SIGHUP reload that swaps the
|
||||
@@ -2992,7 +2992,7 @@ def _main():
|
||||
logger.error(
|
||||
"IMAP user and password must be specified if host is specified"
|
||||
)
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
|
||||
ssl = True
|
||||
verify = True
|
||||
@@ -3022,7 +3022,7 @@ def _main():
|
||||
|
||||
except Exception:
|
||||
logger.exception("IMAP Error")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
|
||||
if opts.graph_client_id:
|
||||
try:
|
||||
@@ -3088,10 +3088,10 @@ def _main():
|
||||
tenant_id=opts.graph_tenant_id,
|
||||
auth_method=opts.graph_auth_method,
|
||||
)
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
except Exception:
|
||||
logger.exception("MS Graph Error")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
|
||||
if opts.gmail_api_credentials_file:
|
||||
# Any effective delete flag needs the deletion scope: the per-report-type
|
||||
@@ -3135,7 +3135,7 @@ def _main():
|
||||
|
||||
except Exception:
|
||||
logger.exception("Gmail API Error")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
|
||||
if opts.maildir_path:
|
||||
try:
|
||||
@@ -3145,7 +3145,7 @@ def _main():
|
||||
)
|
||||
except Exception:
|
||||
logger.exception("Maildir Error")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
|
||||
if mailbox_connection:
|
||||
mailbox_batch_size_value = (
|
||||
@@ -3219,10 +3219,10 @@ def _main():
|
||||
tenant_id=opts.graph_tenant_id,
|
||||
auth_method=opts.graph_auth_method,
|
||||
)
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
except Exception:
|
||||
logger.exception("Mailbox Error")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
|
||||
# Filtered here rather than relying on process_reports()'s in-place
|
||||
# filtering: the dicts it filters are the file snapshot and the mailbox
|
||||
@@ -3286,7 +3286,7 @@ def _main():
|
||||
)
|
||||
except Exception:
|
||||
logger.exception("Failed to email results")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
elif msgraph_connection is not None and smtp_to_value:
|
||||
try:
|
||||
email_results_via_msgraph(
|
||||
@@ -3305,10 +3305,10 @@ def _main():
|
||||
tenant_id=opts.graph_tenant_id,
|
||||
auth_method=opts.graph_auth_method,
|
||||
)
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
except Exception:
|
||||
logger.exception("Failed to email results via Microsoft Graph")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
|
||||
if mailbox_connection and opts.mailbox_watch:
|
||||
logger.info("Watching for email - Ctrl-C once to quit, twice to force")
|
||||
@@ -3346,10 +3346,10 @@ def _main():
|
||||
)
|
||||
except FileExistsError as error:
|
||||
logger.error(f"{error.__str__()}")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
except ParserError as error:
|
||||
logger.error(error.__str__())
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
except (ClientAuthenticationError, APIError, httpx.HTTPError) as error:
|
||||
if msgraph_connection is None:
|
||||
logger.exception("Mailbox Error")
|
||||
@@ -3361,7 +3361,7 @@ def _main():
|
||||
tenant_id=opts.graph_tenant_id,
|
||||
auth_method=opts.graph_auth_method,
|
||||
)
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
|
||||
# Prioritize shutdown over reload if both flags are set (e.g.
|
||||
# SIGHUP followed by SIGTERM). atexit closes output clients.
|
||||
@@ -3426,7 +3426,7 @@ def _main():
|
||||
)
|
||||
except InvalidIPinfoAPIKey as e:
|
||||
logger.critical(str(e))
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
|
||||
for k, v in vars(new_opts).items():
|
||||
setattr(opts, k, v)
|
||||
@@ -3492,7 +3492,7 @@ def _main():
|
||||
|
||||
# Close output clients on the success path (one-shot or graceful
|
||||
# watch-loop exit). atexit-registered above is the safety net for
|
||||
# exit(1) / uncaught-exception paths.
|
||||
# sys.exit(1) / uncaught-exception paths.
|
||||
_close_output_clients(clients)
|
||||
|
||||
|
||||
|
||||
@@ -941,11 +941,6 @@ def save_aggregate_report_to_elasticsearch(
|
||||
begin_date = human_timestamp_to_datetime(metadata["begin_date"], to_utc=True)
|
||||
end_date = human_timestamp_to_datetime(metadata["end_date"], to_utc=True)
|
||||
|
||||
if monthly_indexes:
|
||||
index_date = begin_date.strftime("%Y-%m")
|
||||
else:
|
||||
index_date = begin_date.strftime("%Y-%m-%d")
|
||||
|
||||
org_name_query = Q(dict(match_phrase=dict(org_name=org_name))) # type: ignore
|
||||
report_id_query = Q(dict(match_phrase=dict(report_id=report_id))) # pyright: ignore[reportArgumentType]
|
||||
domain_query = Q(dict(match_phrase={"published_policy.domain": domain})) # pyright: ignore[reportArgumentType]
|
||||
|
||||
@@ -872,11 +872,6 @@ def save_aggregate_report_to_opensearch(
|
||||
begin_date = human_timestamp_to_datetime(metadata["begin_date"], to_utc=True)
|
||||
end_date = human_timestamp_to_datetime(metadata["end_date"], to_utc=True)
|
||||
|
||||
if monthly_indexes:
|
||||
index_date = begin_date.strftime("%Y-%m")
|
||||
else:
|
||||
index_date = begin_date.strftime("%Y-%m-%d")
|
||||
|
||||
org_name_query = Q(dict(match_phrase=dict(org_name=org_name)))
|
||||
report_id_query = Q(dict(match_phrase=dict(report_id=report_id)))
|
||||
domain_query = Q(dict(match_phrase={"published_policy.domain": domain}))
|
||||
|
||||
@@ -174,7 +174,7 @@ def _main():
|
||||
domain = line.lower().strip()
|
||||
if domain in list_var:
|
||||
print(f"Error: {domain} is in {file_path} multiple times")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
elif domain != "":
|
||||
list_var.append(domain)
|
||||
|
||||
@@ -182,7 +182,7 @@ def _main():
|
||||
load_list(psl_overrides_file_path, psl_overrides)
|
||||
if not os.path.exists(mmdb_file_path):
|
||||
print(f"Error: {mmdb_file_path} does not exist")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
print(f"Loading {mmdb_file_path}")
|
||||
as_name_index = _load_as_name_index(mmdb_file_path)
|
||||
print(f"Indexed {len(as_name_index)} as_names from the MMDB")
|
||||
@@ -196,7 +196,7 @@ def _main():
|
||||
print(
|
||||
f"Error: {domain} is in {base_reverse_dns_map_file_path} multiple times"
|
||||
)
|
||||
exit()
|
||||
sys.exit(1)
|
||||
else:
|
||||
known_domains.append(domain)
|
||||
if domain in known_unknown_domains and known_domains:
|
||||
@@ -204,10 +204,10 @@ def _main():
|
||||
f"Error:{domain} is in {known_unknown_list_file_path} and \
|
||||
{base_reverse_dns_map_file_path}"
|
||||
)
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
if not os.path.exists(args.input):
|
||||
print(f"Error: {args.input} does not exist")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
for row in _read_input_rows(args.input):
|
||||
domain = row["source_name"].lower().strip()
|
||||
if domain == "":
|
||||
|
||||
@@ -5,6 +5,7 @@ from __future__ import annotations
|
||||
import os
|
||||
import csv
|
||||
import re
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from collections.abc import Mapping, Iterable, Collection
|
||||
|
||||
@@ -243,23 +244,23 @@ def _main():
|
||||
|
||||
if not os.path.exists(readme_file):
|
||||
print(f"Error: {readme_file} does not exist")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
try:
|
||||
types = normalize_types_in_readme(readme_file)
|
||||
except ValueError as e:
|
||||
print(f"Error: {e}")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
|
||||
map_allowed_values = {"type": types}
|
||||
|
||||
for list_file in list_files:
|
||||
if not os.path.exists(list_file):
|
||||
print(f"Error: {list_file} does not exist")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
sort_list_file(list_file)
|
||||
if not os.path.exists(map_file):
|
||||
print(f"Error: {map_file} does not exist")
|
||||
exit(1)
|
||||
sys.exit(1)
|
||||
try:
|
||||
sort_csv(
|
||||
map_file,
|
||||
|
||||
Reference in New Issue
Block a user