From adbf07f2422e40f665df8440ddb0bc26fc38272b Mon Sep 17 00:00:00 2001
From: Serhii Kupriienko <61395455+skupriienko@users.noreply.github.com>
Date: Sat, 12 Sep 2026 23:57:43 +0300
Subject: [PATCH 01/11] feat(security): harden input validation and log
redaction in guardrails
- Support typing.NamedTuple and object __dict__/model_dump redaction in RedactingFilter
- Enforce regular file check before size evaluation to prevent CWE-400 resource exhaustion
- Disallow boolean types in timeout evaluation to prevent silent coercion
- Harden IDN normalization via rpartition and catch specific UnicodeError bounds
- Filter control characters [\x00-\x1f\x7f] and normalize whitespace in log traces
- Update test_guardrails suite with edge cases covering CWE-22, CWE-400, and logging extras
---
.pre-commit-config.yaml | 17 +-
mailjet_rest/utils/guardrails.py | 81 ++++--
tests/unit/test_guardrails.py | 472 ++++++++++++++++++++-----------
3 files changed, 381 insertions(+), 189 deletions(-)
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 8787ef2..a15ac36 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -105,7 +105,7 @@ repos:
# Git commit quality
- repo: https://github.com/commitizen-tools/commitizen
- rev: v4.17.0
+ rev: v4.18.0
hooks:
- id: commitizen
name: "🌳 git · Validate commit message"
@@ -134,7 +134,7 @@ repos:
additional_dependencies: [".[toml]"]
- repo: https://github.com/semgrep/pre-commit
- rev: 'v1.173.0'
+ rev: 'v1.177.0'
hooks:
- id: semgrep
name: "🔒 security · Static analysis (semgrep)"
@@ -145,16 +145,17 @@ repos:
hooks:
- id: pip-audit
name: "🔒 security · Audit Python dependencies"
+ additional_dependencies: ["pip>=26.2"]
- repo: https://github.com/woodruffw/zizmor-pre-commit
- rev: v1.29.0
+ rev: v1.30.1
hooks:
- id: zizmor
name: "🔒 security · Scan GitHub Actions (zizmor)"
# Spelling and typos
- repo: https://github.com/crate-ci/typos
- rev: v1.49.0
+ rev: v1.50.1
hooks:
- id: typos
name: "📝 spelling · Check typos"
@@ -170,7 +171,7 @@ repos:
files: ^\.github/workflows/.*\.ya?ml$
- repo: https://github.com/ariebovenberg/slotscheck
- rev: v0.20.1
+ rev: v0.21.0
hooks:
- id: slotscheck
name: "🔍 check · slotscheck"
@@ -183,7 +184,7 @@ repos:
- hypothesis
- repo: https://github.com/astral-sh/ruff-pre-commit
- rev: v0.16.3
+ rev: v0.16.7
hooks:
- id: ruff-check
name: "🐍 lint · Check with Ruff"
@@ -213,14 +214,14 @@ repos:
exclude: ^samples/
- repo: https://github.com/RobertCraigie/pyright-python
- rev: v1.1.411
+ rev: v1.1.414
hooks:
- id: pyright
name: "🐍 types · Check with pyright"
# Python project configuration
- repo: https://github.com/abravalheri/validate-pyproject
- rev: v0.25
+ rev: '0.26'
hooks:
- id: validate-pyproject
name: "🐍 config · Validate pyproject.toml"
diff --git a/mailjet_rest/utils/guardrails.py b/mailjet_rest/utils/guardrails.py
index 5d796eb..4776ccf 100644
--- a/mailjet_rest/utils/guardrails.py
+++ b/mailjet_rest/utils/guardrails.py
@@ -198,15 +198,49 @@ def _redact_str(data: str) -> str:
except Exception: # ruff: ignore[blind-except]
return "[REDACTION_FAILED_UNSAFE_STRING]"
+ def _redact_tuple(self, data: tuple[Any, ...], depth: int) -> tuple[Any, ...]:
+ """Recursively sanitize tuple items preserving namedtuple structure.
+
+ Returns:
+ The sanitized tuple or NamedTuple instance.
+ """
+ if hasattr(data, "_fields"): # Preserves typing.NamedTuple
+ with contextlib.suppress(Exception):
+ return type(data)(*(self._deep_redact(item, depth + 1) for item in data))
+ return tuple(self._deep_redact(item, depth + 1) for item in data)
+
+ def _redact_object(self, data: Any, depth: int) -> Any:
+ """Recursively sanitize custom objects, dataclasses, and Pydantic models.
+
+ Returns:
+ The sanitized dictionary, string, or primitive representation.
+ """
+ if hasattr(data, "model_dump") and callable(data.model_dump):
+ with contextlib.suppress(Exception):
+ return self._deep_redact(data.model_dump(), depth + 1)
+
+ if hasattr(data, "__dict__"):
+ with contextlib.suppress(Exception):
+ return self._deep_redact(vars(data), depth + 1)
+
+ try:
+ str_val = str(data)
+ except Exception: # ruff: ignore[blind-except]
+ return ""
+
+ return self._redact_str(str_val)
+
def _deep_redact(self, data: Any, depth: int = 0) -> Any:
"""Recursively search and scrub secrets from complex nested data structures.
Returns:
- Any: The fully scrubbed and redacted data structure representation.
+ The fully sanitized data structure representation.
"""
if depth > self.MAX_REDACTION_DEPTH:
return "[MAX_DEPTH_REACHED]"
+ if isinstance(data, (int, float, bool, type(None))):
+ return data
if isinstance(data, str):
return self._redact_str(data)
if isinstance(data, dict):
@@ -214,11 +248,11 @@ def _deep_redact(self, data: Any, depth: int = 0) -> Any:
if isinstance(data, list):
return [self._deep_redact(item, depth + 1) for item in data]
if isinstance(data, tuple):
- return tuple(self._deep_redact(item, depth + 1) for item in data)
+ return self._redact_tuple(data, depth)
if isinstance(data, set):
return {self._deep_redact(item, depth + 1) for item in data}
- return data
+ return self._redact_object(data, depth)
@override
def filter(self, record: logging.LogRecord) -> bool:
@@ -492,9 +526,14 @@ def validate_attachment_path(file_path: Path | str, safe_base_dir: Path | str |
@staticmethod
def check_file_size(path: Path, max_size_bytes: int = 15 * 1024 * 1024) -> None:
"""Prevent Resource Exhaustion (CWE-400). Limit defaults to 15MB."""
- size = path.stat().st_size
+ target = Path(path)
+ if not target.is_file():
+ msg = f"Security Alert (CWE-400): Path is not a regular file: {target}"
+ raise ValueError(msg)
+
+ size = target.stat().st_size
if size > max_size_bytes:
- msg = f"Security Violation: File '{path.name}' exceeds safe threshold."
+ msg = f"Security Violation: File '{target.name}' exceeds safe threshold."
raise ValueError(msg)
@staticmethod
@@ -507,7 +546,8 @@ def _validate_scalar_timeout(timeout: Any) -> float:
Returns:
float: The validated scalar timeout in seconds.
"""
- if not isinstance(timeout, (int, float)):
+ # Explicitly check for bool to prevent True/False coercing to 1.0/0.0
+ if isinstance(timeout, bool) or not isinstance(timeout, (int, float)):
msg = f"Timeout must be a numeric float or int, got {type(timeout).__name__}."
raise TypeError(msg)
@@ -580,23 +620,21 @@ def normalize_domain(email_or_domain: str) -> str:
"""
if not email_or_domain:
return email_or_domain
- parts = email_or_domain.rsplit("@", 1)
- if len(parts) == 2:
- local, domain = parts
+ local_part, sep, domain_part = email_or_domain.rpartition("@")
+ if sep:
try:
- puny_domain = domain.encode("idna").decode("ascii")
- except Exception as e:
+ puny_domain = domain_part.encode("idna").decode("ascii")
+ except UnicodeError as e:
msg = f"Invalid IDN in email: {email_or_domain}"
raise ValueError(msg) from e
- else:
- return f"{local}@{puny_domain}"
- else:
- try:
- return email_or_domain.encode("idna").decode("ascii")
- except Exception as e:
- msg = f"Invalid IDN: {email_or_domain}"
- raise ValueError(msg) from e
+ return f"{local_part}@{puny_domain}"
+
+ try:
+ return email_or_domain.encode("idna").decode("ascii")
+ except UnicodeError as e:
+ msg = f"Invalid IDN: {email_or_domain}"
+ raise ValueError(msg) from e
@staticmethod
def sanitize_segment(segment: Any) -> str:
@@ -669,7 +707,10 @@ def sanitize_log_trace(trace_val: Any) -> str:
"""
if not trace_val:
return ""
- return re.sub(r"\s+", "_", str(trace_val))
+
+ # Neutralize control characters [\x00-\x1f\x7f] and normalize whitespace
+ clean_str = _PATH_CONTROL_CHAR_RE.sub("_", str(trace_val))
+ return re.sub(r"\s+", "_", clean_str)
@staticmethod
def _validate_token(auth: str) -> str:
diff --git a/tests/unit/test_guardrails.py b/tests/unit/test_guardrails.py
index 56e5d30..21d7fe7 100644
--- a/tests/unit/test_guardrails.py
+++ b/tests/unit/test_guardrails.py
@@ -1,7 +1,10 @@
# pyright: reportIndexIssue=false
"""Unit tests for the guardrails.py security module."""
+from __future__ import annotations
+
import logging
+import tempfile
from html.parser import HTMLParser
from pathlib import Path
from typing import Any
@@ -9,10 +12,17 @@
import pytest
-from mailjet_rest.utils.guardrails import RedactingFilter, SecretAuth, SecurityGuard
+from mailjet_rest.utils.guardrails import (
+ RedactingFilter,
+ SecretAuth,
+ SecureHTTPAdapter,
+ SecurityGuard,
+)
class TestRedactingFilter:
+ """Test suite covering secret redaction across log strings, nested structures, and objects."""
+
def test_redacting_filter_scrubs_secrets_from_string(self) -> None:
"""Coverage: Hits the string redaction branch."""
filter_ = RedactingFilter()
@@ -65,85 +75,107 @@ def test_deep_redact_stops_at_max_depth(self) -> None:
assert isinstance(record.args, dict)
assert record.args["a"]["b"]["c"]["d"]["e"] == "[MAX_DEPTH_REACHED]"
+ def test_redacting_filter_exceptions(self, monkeypatch: pytest.MonkeyPatch) -> None:
+ """Coverage: Trigger string parsing failures inside logging filter."""
+ filter_ = RedactingFilter()
-class TestSecurityGuard:
- def test_validate_config_url_valid(self) -> None:
- """Coverage: Valid URL passes cleanly."""
- SecurityGuard.validate_config_url("https://api.mailjet.com/v3", "mailjet.com")
+ # Create a mock pattern object to bypass re.Pattern immutability
+ class MockPattern:
+ def sub(self, *args: Any, **kwargs: Any) -> str:
+ raise ValueError("Forced error")
- def test_validate_config_url_http(self) -> None:
- """Coverage: Invalid scheme is blocked (CWE-918)."""
- with pytest.raises(ValueError, match="Invalid scheme 'ftp'"):
- SecurityGuard.validate_config_url("ftp://api.mailjet.com")
+ # Mock the internal function returning the pattern instead of the pattern type itself
+ monkeypatch.setattr("mailjet_rest.utils.guardrails._get_secret_pattern", lambda: MockPattern())
- def test_validate_config_url_malicious_domain(self) -> None:
- """Coverage: Unrecognized domains blocked (CWE-918)."""
- with pytest.raises(ValueError, match="not permitted"):
- SecurityGuard.validate_config_url("https://attacker.com/v3", "mailjet.com")
+ # It should catch the error and return the original string transparently
+ assert RedactingFilter._redact_str("test") == "[REDACTION_FAILED_UNSAFE_STRING]"
- def test_sanitize_headers_catches_crlf(self) -> None:
- """Coverage: HTTP Header Injection (CWE-113)."""
- with pytest.raises(ValueError, match="CRLF injection"):
- SecurityGuard.sanitize_headers({"X-Custom": "val\r\ninjected"})
+ # It should catch any error in the outer filter() block and return True (allow log writing)
+ record = logging.LogRecord("test", logging.INFO, "", 0, "msg", (), None)
+ assert filter_.filter(record) is True
- def test_check_control_characters(self) -> None:
- """Coverage: Null byte injection (CWE-20)."""
- with pytest.raises(ValueError, match="Unprintable control character"):
- SecurityGuard.check_control_characters("field", "bad\x00string")
+ def test_redacting_filter_advanced_types_and_extras(self) -> None:
+ """Cover NamedTuples, sets, dataclass/object models, and extra attributes in logging."""
+ filter_ = RedactingFilter()
- def test_analyze_html_safety_blocks_xss(self) -> None:
- """Coverage: SpamGuard catches script tags."""
- with pytest.raises(ValueError, match="executable Javascript/XSS vectors"):
- SecurityGuard.analyze_html_safety("")
+ # 1. Custom objects with __dict__
+ class CustomPayload:
+ def __init__(self) -> None:
+ self.api_key = "api_key: secret_token_value" # pragma: allowlist secret
- def test_analyze_html_safety_blocks_events(self) -> None:
- """Coverage: SpamGuard catches inline events."""
- with pytest.raises(ValueError, match="blocked script/event execution tags"):
- SecurityGuard.analyze_html_safety("")
+ # 2. Objects with model_dump (Pydantic style)
+ class PydanticDummy:
+ def model_dump(self) -> dict[str, str]:
+ return {"token": "token 12345secret"}
- def test_generate_payload_fingerprint(self) -> None:
- """Coverage: Idempotency hashing mechanism."""
- payload1 = {"a": 1, "b": 2, "CustomID": "ignore"}
- payload2 = {"b": 2, "a": 1, "EventPayload": "ignore"}
- assert SecurityGuard.generate_payload_fingerprint(payload1) == SecurityGuard.generate_payload_fingerprint(
- payload2
+ # 3. Set & frozenset redaction
+ raw_set = {"api_key: set_secret"}
+ raw_frozenset = frozenset(["api_key: frozenset_secret"])
+
+ record = logging.LogRecord(
+ name="test_logger",
+ level=logging.INFO,
+ pathname="",
+ lineno=0,
+ msg="Logging structured records",
+ args=(CustomPayload(), PydanticDummy(), raw_set, raw_frozenset),
+ exc_info=None,
)
+ # Inject an 'extra' attribute containing key context
+ record.custom_audit_field = {"header": "Authorization: Bearer secret_extra_token"} # type: ignore[attr-defined]
- def test_validate_attachment_path_traversal(self, tmp_path: Path) -> None:
- """Coverage: Path traversal enforcement (CWE-22)."""
- # The check ordering now evaluates bounds before touching the filesystem,
- # throwing the correct security error!
- with pytest.raises(ValueError, match="Traversal detected"):
- SecurityGuard.validate_attachment_path("../../etc/passwd", tmp_path)
+ filter_.filter(record)
- def test_check_file_size_exceeded(self, tmp_path: Path) -> None:
- """Coverage: Hits CWE-400 resource exhaustion."""
- test_file = tmp_path / "large.txt"
- test_file.write_bytes(b"0" * 1025)
- with pytest.raises(ValueError, match="exceeds safe threshold"):
- SecurityGuard.check_file_size(test_file, max_size_bytes=1000)
+ # Validate redactions
+ assert "secret_token_value" not in str(record.args)
+ assert "12345secret" not in str(record.args)
+ assert "set_secret" not in str(record.args)
+ assert "frozenset_secret" not in str(record.args)
+ assert "secret_extra_token" not in str(getattr(record, "custom_audit_field"))
- def test_validate_timeout_nan_inf(self) -> None:
- """Coverage: CWE-400 Float evaluation."""
- with pytest.raises(ValueError, match="Timeout cannot be Infinity or NaN"):
- SecurityGuard.validate_timeout(float("inf"))
- def test_normalize_domain_punycode(self) -> None:
- """Coverage: IDN homograph normalization."""
- puny = SecurityGuard.normalize_domain("info@münchen.de")
- assert puny == "info@xn--mnchen-3ya.de"
+class TestSecretAuth:
+ """Test suite covering SecretAuth encapsulation, hashing, and memory protection."""
- def test_sanitize_log_trace(self) -> None:
- """Coverage: CWE-117 Log Forging."""
- clean = SecurityGuard.sanitize_log_trace("My\nTrace\rID")
- assert clean == "My_Trace_ID"
+ def test_secretauth_repr(self) -> None:
+ """Coverage: Confirm string representations scrub memory securely."""
+ auth = SecretAuth(("user", "pass"))
+ assert repr(auth) == "SecretAuth(***REDACTED***)"
- @patch("sys.audit")
- def test_audit_hooks_are_emitted(self, mock_audit: MagicMock) -> None:
- """Coverage: PEP 578 integration."""
- with pytest.raises(ValueError, match="Path traversal attempt"):
- SecurityGuard.sanitize_segment("..")
- mock_audit.assert_called_with("mailjet.security.path_traversal", "..")
+ def test_secret_auth_security_boundaries(self) -> None:
+ """Cover SecretAuth __hash__, __eq__, __getitem__, and __iter__ protections."""
+ auth1 = SecretAuth(("user_key", "secret_pass"))
+ auth2 = SecretAuth(("user_key", "secret_pass"))
+ auth_diff = SecretAuth(("diff_key", "diff_pass"))
+
+ # Equality comparisons
+ assert auth1 == auth2
+ assert auth1 != auth_diff
+ assert auth1 != "invalid_type"
+ assert auth1 == ("user_key", "secret_pass")
+
+ # Hash support for dict lookups
+ lookup = {auth1: "authenticated"}
+ assert lookup[auth2] == "authenticated"
+
+ # Blocked indexing (CWE-316)
+ with pytest.raises(TypeError, match="Credential extraction via indexing is blocked"):
+ _ = auth1[0]
+
+ # Blocked iteration / tuple unpacking (CWE-316)
+ with pytest.raises(TypeError, match="Credential extraction via iteration is blocked"):
+ for _ in auth1:
+ pass
+
+
+class TestSecureHTTPAdapter:
+ """Test suite covering custom TLS 1.2+ adapter and proxy manager security."""
+
+ def test_secure_http_adapter_proxy_manager(self) -> None:
+ """Cover SecureHTTPAdapter.proxy_manager_for TLS configuration."""
+ adapter = SecureHTTPAdapter()
+ proxy_manager = adapter.proxy_manager_for("http://127.0.0.1:8080")
+ assert proxy_manager.connection_pool_kw.get("ssl_context") is not None
class TestAuthCoercionAndValidation:
@@ -207,129 +239,247 @@ def test_invalid_auth_type(self) -> None:
SecurityGuard.validate_and_coerce_auth(12345) # type: ignore[arg-type]
-def test_generate_payload_fingerprint_cyclic() -> None:
- """Coverage: Prevent recursion errors on cyclic references."""
- cyclic: dict[str, Any] = {}
- cyclic["a"] = cyclic
- # Should gracefully return a string hash without crashing
- assert SecurityGuard.generate_payload_fingerprint(cyclic)
-
-
-def test_generate_payload_fingerprint_max_depth() -> None:
- """Coverage: Enforce maximum nesting depth limits."""
- deep: Any = {"a": 1}
- for _ in range(55):
- deep = {"a": deep}
-
- with pytest.raises(ValueError, match="Payload hashing failed due to malformed"):
- SecurityGuard.generate_payload_fingerprint(deep)
-
-
-def test_validate_attachment_path_no_sandbox() -> None:
- """Coverage: Fallback zero-trust checks for OS roots and path traversal."""
- with pytest.raises(ValueError, match="Path traversal tokens"):
- SecurityGuard.validate_attachment_path("../etc/passwd")
-
- with pytest.raises(ValueError, match="explicitly forbidden"):
- SecurityGuard.validate_attachment_path("/etc/passwd")
-
-
-def test_sanitize_segment_template_injection() -> None:
- """Coverage: Block Jinja/Template injection signatures."""
- with pytest.raises(ValueError, match="Template injection attempt"):
- SecurityGuard.sanitize_segment("{{ config.secret }}")
+class TestSecurityGuard:
+ """Test suite covering perimeter guardrails, URI sanitization, inputs, and payloads."""
+ # -------------------------------------------------------------------------
+ # Configuration & URL Validation (SSRF / CWE-918)
+ # -------------------------------------------------------------------------
+ def test_validate_config_url_valid(self) -> None:
+ """Coverage: Valid URL passes cleanly."""
+ SecurityGuard.validate_config_url("https://api.mailjet.com/v3", "mailjet.com")
-def test_sanitize_segment_invalid_type() -> None:
- """Coverage: Block dicts/lists in path segments."""
- with pytest.raises(TypeError, match="Invalid segment type"):
- SecurityGuard.sanitize_segment({"dict": "not allowed"}) # type: ignore[arg-type]
+ def test_validate_config_url_http(self) -> None:
+ """Coverage: Invalid scheme is blocked (CWE-918)."""
+ with pytest.raises(ValueError, match="Invalid scheme 'ftp'"):
+ SecurityGuard.validate_config_url("ftp://api.mailjet.com")
+ def test_validate_config_url_malicious_domain(self) -> None:
+ """Coverage: Unrecognized domains blocked (CWE-918)."""
+ with pytest.raises(ValueError, match="not permitted"):
+ SecurityGuard.validate_config_url("https://attacker.com/v3", "mailjet.com")
-def test_spam_guard_html_analysis_htmlparser_error() -> None:
- """Coverage: Trigger Failsafe on HTMLParser crash."""
+ def test_check_request_security_proxy_warning(self) -> None:
+ """Cover unencrypted HTTP proxy warning."""
+ with pytest.warns(UserWarning, match="Unencrypted HTTP proxy detected"):
+ SecurityGuard.check_request_security({"proxies": {"https": "http://insecure-proxy.com:8080"}})
+
+ def test_filter_safe_kwargs(self) -> None:
+ """Cover filtering out disallowed client kwargs (CWE-915)."""
+ input_kwargs = {"verify": True, "timeout": 10, "disallowed_header_injection": "bad"}
+ filtered = SecurityGuard.filter_safe_kwargs(input_kwargs)
+ assert "verify" in filtered
+ assert "timeout" not in filtered
+ assert "disallowed_header_injection" not in filtered
+
+ # -------------------------------------------------------------------------
+ # Headers & Control Characters (CWE-113, CWE-20, CWE-117)
+ # -------------------------------------------------------------------------
+ def test_sanitize_headers_catches_crlf(self) -> None:
+ """Coverage: HTTP Header Injection (CWE-113)."""
+ with pytest.raises(ValueError, match="CRLF injection"):
+ SecurityGuard.sanitize_headers({"X-Custom": "val\r\ninjected"})
- class CrashParser(HTMLParser):
- def feed(self, data: str) -> None:
- raise RecursionError("Simulated crash")
+ def test_check_control_characters(self) -> None:
+ """Coverage: Null byte injection (CWE-20)."""
+ with pytest.raises(ValueError, match="Unprintable control character"):
+ SecurityGuard.check_control_characters("field", "bad\x00string")
- with patch.object(SecurityGuard, "_SpamGuardParser", CrashParser):
- with pytest.raises(Exception, match="Fatal HTML parsing error"):
- SecurityGuard.analyze_html_safety("