🛡️ Sentinel: [MEDIUM] Fix sensitive data leak in debug logs#126
🛡️ Sentinel: [MEDIUM] Fix sensitive data leak in debug logs#126
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Merging to
|
|
|
||
| def test_retry_request_sanitizes_token_in_debug_logs(caplog): | ||
| # Setup sensitive data | ||
| sensitive_token = "SECRET_TOKEN_123" |
Check notice
Code scanning / Bandit
Possible hardcoded password: 'SECRET_TOKEN_123' Note test
| main._retry_request(mock_func, max_retries=1, delay=0) | ||
|
|
||
| # Check logs | ||
| assert "Response content:" in caplog.text |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| # Check logs | ||
| assert "Response content:" in caplog.text | ||
| assert sensitive_token not in caplog.text |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # Check logs | ||
| assert "Response content:" in caplog.text | ||
| assert sensitive_token not in caplog.text | ||
| assert "[REDACTED]" in caplog.text |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| def test_push_rules_sanitizes_token_in_debug_logs(caplog): | ||
| # Setup sensitive data | ||
| sensitive_token = "SECRET_TOKEN_456" |
Check notice
Code scanning / Bandit
Possible hardcoded password: 'SECRET_TOKEN_456' Note test
|
|
||
| with main._api_client() as client: | ||
| # Check User-Agent | ||
| assert client.headers["User-Agent"] == "Control-D-Sync/0.1.0" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # Check User-Agent | ||
| assert client.headers["User-Agent"] == "Control-D-Sync/0.1.0" | ||
| # Check Authorization | ||
| assert client.headers["Authorization"] == "Bearer test_token" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # 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 |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| def test_gh_client_configuration(): | ||
| client = main._gh | ||
| # Check User-Agent | ||
| assert client.headers["User-Agent"] == "Control-D-Sync/0.1.0" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # Check User-Agent | ||
| assert client.headers["User-Agent"] == "Control-D-Sync/0.1.0" | ||
| # Check follow_redirects | ||
| assert client.follow_redirects is False |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| 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] |
Check notice
Code scanning / Remark-lint (reported by Codacy)
Warn when references to undefined definitions are found. Note
| 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] |
Check notice
Code scanning / Remark-lint (reported by Codacy)
Warn when shortcut reference links are used. Note
| ) | ||
|
|
||
| _gh = httpx.Client(timeout=30) | ||
| _gh = httpx.Client( |
Check warning
Code scanning / Prospector (reported by Codacy)
expected 2 blank lines after class or function definition, found 1 (E305) Warning
| 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)}") |
Check warning
Code scanning / Prospector (reported by Codacy)
Use lazy % formatting in logging functions (logging-fstring-interpolation) Warning
| 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)}") |
Check warning
Code scanning / Prospector (reported by Codacy)
Use lazy % formatting in logging functions (logging-fstring-interpolation) Warning
| @@ -0,0 +1,95 @@ | |||
| import logging | |||
| import pytest | |||
Check warning
Code scanning / Prospector (reported by Codacy)
Unable to import 'pytest' (import-error) Warning test
| import logging | ||
| import pytest | ||
| from unittest.mock import MagicMock, patch | ||
| import httpx |
Check warning
Code scanning / Prospector (reported by Codacy)
Unable to import 'httpx' (import-error) Warning test
| # 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 warning
Code scanning / Prospector (reported by Codacy)
Access to a protected member _retry_request of a client class (protected-access) Warning test
| # Setup token | ||
| main.TOKEN = "test_token" | ||
|
|
||
| with main._api_client() as client: |
Check warning
Code scanning / Prospector (reported by Codacy)
Access to a protected member _api_client of a client class (protected-access) Warning test
| assert client.follow_redirects is False | ||
|
|
||
| def test_gh_client_configuration(): | ||
| client = main._gh |
Check warning
Code scanning / Prospector (reported by Codacy)
Access to a protected member _gh of a client class (protected-access) Warning test
| ) | ||
|
|
||
| _gh = httpx.Client(timeout=30) | ||
| _gh = httpx.Client( |
Check warning
Code scanning / Pylint (reported by Codacy)
Constant name "_gh" doesn't conform to UPPER_CASE naming style Warning
| @@ -0,0 +1,95 @@ | |||
| import logging | |||
Check warning
Code scanning / Pylint (reported by Codacy)
Missing module docstring Warning test
| @@ -0,0 +1,95 @@ | |||
| import logging | |||
| import pytest | |||
| from unittest.mock import MagicMock, patch | |||
Check warning
Code scanning / Pylint (reported by Codacy)
standard import "from unittest.mock import MagicMock, patch" should be placed before "import pytest" Warning test
| import main | ||
|
|
||
| # Mock httpx.HTTPError to include a response with sensitive data | ||
| def create_mock_error(status_code, text, request_url="https://example.com"): |
Check warning
Code scanning / Pylint (reported by Codacy)
Missing function docstring Warning test
| 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) |
Check warning
Code scanning / Pylint (reported by Codacy)
Line too long (107/100) Warning test
| assert sensitive_token not in caplog.text | ||
| assert "[REDACTED]" in caplog.text | ||
|
|
||
| def test_api_client_configuration(): |
Check warning
Code scanning / Pylint (reported by Codacy)
Missing function docstring Warning test
| # 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(): |
Check warning
Code scanning / Pylint (reported by Codacy)
Missing function docstring Warning test
| # 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 notice
Code scanning / Pylint (reported by Codacy)
Access to a protected member _retry_request of a client class Note test
| # Setup token | ||
| main.TOKEN = "test_token" | ||
|
|
||
| with main._api_client() as client: |
Check notice
Code scanning / Pylint (reported by Codacy)
Access to a protected member _api_client of a client class Note test
| assert client.follow_redirects is False | ||
|
|
||
| def test_gh_client_configuration(): | ||
| client = main._gh |
Check notice
Code scanning / Pylint (reported by Codacy)
Access to a protected member _gh of a client class Note test
| main._retry_request(mock_func, max_retries=1, delay=0) | ||
|
|
||
| # Check logs | ||
| assert "Response content:" in caplog.text |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| # Check logs | ||
| assert "Response content:" in caplog.text | ||
| assert sensitive_token not in caplog.text |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # Check logs | ||
| assert "Response content:" in caplog.text | ||
| assert sensitive_token not in caplog.text | ||
| assert "[REDACTED]" in caplog.text |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| ) | ||
|
|
||
| # push_rules catches the error and returns False (or continues if batch failed) | ||
| assert res is False |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| assert res is False | ||
|
|
||
| # Check logs | ||
| assert "Response content:" in caplog.text |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| with main._api_client() as client: | ||
| # Check User-Agent | ||
| assert client.headers["User-Agent"] == "Control-D-Sync/0.1.0" |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # Check User-Agent | ||
| assert client.headers["User-Agent"] == "Control-D-Sync/0.1.0" | ||
| # Check Authorization | ||
| assert client.headers["Authorization"] == "Bearer test_token" |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # 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 |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| def test_gh_client_configuration(): | ||
| client = main._gh | ||
| # Check User-Agent | ||
| assert client.headers["User-Agent"] == "Control-D-Sync/0.1.0" |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # Check User-Agent | ||
| assert client.headers["User-Agent"] == "Control-D-Sync/0.1.0" | ||
| # Check follow_redirects | ||
| assert client.follow_redirects is False |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| @@ -0,0 +1,95 @@ | |||
| import logging | |||
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Missing module docstring Warning test
| @@ -0,0 +1,95 @@ | |||
| import logging | |||
| import pytest | |||
| from unittest.mock import MagicMock, patch | |||
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
standard import "from unittest.mock import MagicMock, patch" should be placed before "import pytest" Warning test
| import main | ||
|
|
||
| # Mock httpx.HTTPError to include a response with sensitive data | ||
| def create_mock_error(status_code, text, request_url="https://example.com"): |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Missing function or method docstring Warning test
| 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) |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Line too long (107/100) Warning test
| 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): |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Missing function or method docstring Warning test
| 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)}") |
Check notice
Code scanning / Pylintpython3 (reported by Codacy)
Use lazy % formatting in logging functions Note
| 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)}") |
Check notice
Code scanning / Pylintpython3 (reported by Codacy)
Use lazy % formatting in logging functions Note
| # 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 notice
Code scanning / Pylintpython3 (reported by Codacy)
Access to a protected member _retry_request of a client class Note test
| # Setup token | ||
| main.TOKEN = "test_token" | ||
|
|
||
| with main._api_client() as client: |
Check notice
Code scanning / Pylintpython3 (reported by Codacy)
Access to a protected member _api_client of a client class Note test
| assert client.follow_redirects is False | ||
|
|
||
| def test_gh_client_configuration(): | ||
| client = main._gh |
Check notice
Code scanning / Pylintpython3 (reported by Codacy)
Access to a protected member _gh of a client class Note test
🛡️ Sentinel: [MEDIUM] Fix sensitive data leak in debug logs
🚨 Severity: MEDIUM
💡 Vulnerability: API response bodies containing secrets (e.g. tokens in error messages) were logged in plain text when debug logging was enabled.
🎯 Impact: Potential leakage of API tokens or PII to log files.
🔧 Fix: Applied sanitize_for_log redaction to debug log outputs.
✨ Enhancement: Also hardened HTTP client by disabling redirects and adding User-Agent.
✅ Verification: Added tests/test_security_hardening.py verifying redaction and client config.
PR created automatically by Jules for task 9089717741045128790 started by @abhimehro