From 0cf699b11d1a7ee459932ce5b539045bc0ed2657 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 23 Jan 2026 12:15:09 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[MEDIUM]=20?= =?UTF-8?q?Fix=20sensitive=20data=20leak=20in=20debug=20logs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/sentinel.md | 7 +++ main.py | 13 ++++- tests/test_security_hardening.py | 95 ++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 tests/test_security_hardening.py diff --git a/.jules/sentinel.md b/.jules/sentinel.md index a1ed0d5d..f1ff71fd 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -60,3 +60,10 @@ **Prevention:** 1. Implement strict validation on all data items from external lists (`is_valid_rule`). 2. Filter out items containing dangerous characters (`<`, `>`, `"` etc.) or control codes. + +## 2026-05-15 - [Inconsistent Redaction in Debug Logs] +**Vulnerability:** While `sanitize_for_log` existed, it was not applied to `log.debug(e.response.text)` calls in exception handlers. This meant enabling debug logs could expose raw secrets returned by the API in error messages, bypassing the redaction mechanism. +**Learning:** Security controls (like redaction helpers) must be applied consistently at all exit points, especially in verbose/debug paths which are often overlooked during security reviews. +**Prevention:** +1. Audit all logging calls, especially `DEBUG` level ones, for potential secret exposure. +2. Wrapper functions or custom loggers can help enforce sanitization automatically, reducing reliance on manual application of helper functions. diff --git a/main.py b/main.py index c7810580..e52d5a5c 100644 --- a/main.py +++ b/main.py @@ -97,6 +97,7 @@ def format(self, record): # 1. Constants – tweak only here # --------------------------------------------------------------------------- # API_BASE = "https://api.controld.com/profiles" +USER_AGENT = "Control-D-Sync/0.1.0" def sanitize_for_log(text: Any) -> str: @@ -184,11 +185,17 @@ def _api_client() -> httpx.Client: headers={ "Accept": "application/json", "Authorization": f"Bearer {TOKEN}", + "User-Agent": USER_AGENT, }, timeout=30, + follow_redirects=False, ) -_gh = httpx.Client(timeout=30) +_gh = httpx.Client( + headers={"User-Agent": USER_AGENT}, + timeout=30, + follow_redirects=False, +) MAX_RESPONSE_SIZE = 10 * 1024 * 1024 # 10 MB limit for external resources # --------------------------------------------------------------------------- # @@ -309,7 +316,7 @@ def _retry_request(request_func, max_retries=MAX_RETRIES, delay=RETRY_DELAY): except (httpx.HTTPError, httpx.TimeoutException) as e: if attempt == max_retries - 1: if hasattr(e, 'response') and e.response is not None: - log.debug(f"Response content: {e.response.text}") + log.debug(f"Response content: {sanitize_for_log(e.response.text)}") raise wait_time = delay * (2 ** attempt) log.warning(f"Request failed (attempt {attempt + 1}/{max_retries}): {e}. Retrying in {wait_time}s...") @@ -653,7 +660,7 @@ def process_batch(batch_idx: int, batch_data: List[str]) -> Optional[List[str]]: sys.stderr.write("\n") log.error(f"Failed to push batch {batch_idx} for folder {sanitize_for_log(folder_name)}: {sanitize_for_log(e)}") if hasattr(e, 'response') and e.response is not None: - log.debug(f"Response content: {e.response.text}") + log.debug(f"Response content: {sanitize_for_log(e.response.text)}") return None # Optimization 3: Parallelize batch processing diff --git a/tests/test_security_hardening.py b/tests/test_security_hardening.py new file mode 100644 index 00000000..79edf8e5 --- /dev/null +++ b/tests/test_security_hardening.py @@ -0,0 +1,95 @@ +import logging +import pytest +from unittest.mock import MagicMock, patch +import httpx +import main + +# Mock httpx.HTTPError to include a response with sensitive data +def create_mock_error(status_code, text, request_url="https://example.com"): + response = MagicMock(spec=httpx.Response) + response.status_code = status_code + response.text = text + response.request = MagicMock(spec=httpx.Request) + response.request.url = request_url + + # Use HTTPStatusError which accepts request and response + error = httpx.HTTPStatusError(f"HTTP Error {status_code}", request=response.request, response=response) + return error + +def test_retry_request_sanitizes_token_in_debug_logs(caplog): + # Setup sensitive data + sensitive_token = "SECRET_TOKEN_123" + main.TOKEN = sensitive_token + + # Configure logging to capture DEBUG + caplog.set_level(logging.DEBUG) + + # Mock a request function that always raises an error with the token in response + mock_func = MagicMock() + error_text = f"Invalid token: {sensitive_token}" + mock_func.side_effect = create_mock_error(401, error_text) + + # Call _retry_request (it re-raises the exception) + with pytest.raises(httpx.HTTPError): + # Set retries to 1 to fail fast + main._retry_request(mock_func, max_retries=1, delay=0) + + # Check logs + assert "Response content:" in caplog.text + assert sensitive_token not in caplog.text + assert "[REDACTED]" in caplog.text + +def test_push_rules_sanitizes_token_in_debug_logs(caplog): + # Setup sensitive data + sensitive_token = "SECRET_TOKEN_456" + main.TOKEN = sensitive_token + + # Configure logging to capture DEBUG + caplog.set_level(logging.DEBUG) + + # Mock dependencies + mock_client = MagicMock(spec=httpx.Client) + + # Let's mock client.post to raise error + error_text = f"Bad Rule with token {sensitive_token}" + mock_client.post.side_effect = create_mock_error(400, error_text) + + # Patch time.sleep to avoid waiting + with patch("time.sleep"): + res = main.push_rules( + profile_id="p1", + folder_name="f1", + folder_id="fid1", + do=0, + status=1, + hostnames=["rule1"], + existing_rules=set(), + client=mock_client + ) + + # push_rules catches the error and returns False (or continues if batch failed) + assert res is False + + # Check logs + assert "Response content:" in caplog.text + assert sensitive_token not in caplog.text + assert "[REDACTED]" in caplog.text + +def test_api_client_configuration(): + # Setup token + main.TOKEN = "test_token" + + with main._api_client() as client: + # Check User-Agent + assert client.headers["User-Agent"] == "Control-D-Sync/0.1.0" + # Check Authorization + assert client.headers["Authorization"] == "Bearer test_token" + # Check follow_redirects (in httpx < 0.20 it was allow_redirects, now follow_redirects) + assert client.follow_redirects is False + +def test_gh_client_configuration(): + client = main._gh + # Check User-Agent + assert client.headers["User-Agent"] == "Control-D-Sync/0.1.0" + # Check follow_redirects + assert client.follow_redirects is False