diff --git a/.jules/sentinel.md b/.jules/sentinel.md index b6ec1219..038829ea 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -53,3 +53,11 @@ **Learning:** Even trusted APIs should be treated as untrusted sources for critical identifiers used in system operations or path construction. **Prevention:** Whitelist valid characters for all identifiers (e.g. `^[a-zA-Z0-9_.-]+$`) and validate them immediately upon receipt, before any use. + +## 2026-10-24 - Fail-Closed Logic Error in DNS Validation (Broken Availability) + +**Vulnerability:** The `validate_hostname` function implicitly returned `None` (falsy) for valid domains because it lacked a `return True` statement after the successful DNS check loop. This caused the tool to reject *all* domain-based URLs (DoS), only accepting IP literals. It also contained unreachable code due to a structural error. + +**Learning:** Logic errors in security controls often lead to "fail-closed" states that break functionality entirely, or "fail-open" states that bypass security. Implicit returns in Python (`None`) can be dangerous when boolean validation is expected. + +**Prevention:** Always use explicit return statements for both success and failure paths in validation functions. Use static analysis (linting) to catch unreachable code and implicit returns. Ensure unit tests cover positive cases (valid inputs) as rigorously as negative cases (attack vectors). diff --git a/main.py b/main.py index a9ce7d7c..da9c0d2c 100644 --- a/main.py +++ b/main.py @@ -929,16 +929,13 @@ def validate_hostname(hostname: str) -> bool: f"Skipping unsafe hostname {sanitize_for_log(hostname)} (resolves to non-global/multicast IP {ip})" ) return False + return True except (socket.gaierror, ValueError, OSError) as e: log.warning( f"Failed to resolve/validate domain {sanitize_for_log(hostname)}: {sanitize_for_log(e)}" ) return False - if not addr_info: - return False - for res in addr_info: - @lru_cache(maxsize=128) def validate_folder_url(url: str) -> bool: diff --git a/tests/test_push_rules_perf.py b/tests/test_push_rules_perf.py index afd0bafb..9d42d682 100644 --- a/tests/test_push_rules_perf.py +++ b/tests/test_push_rules_perf.py @@ -25,7 +25,6 @@ def setUp(self): self.do = 1 self.status = 1 self.existing_rules = set() - self.main = main @patch("main.concurrent.futures.ThreadPoolExecutor") def test_push_rules_single_batch_optimization(self, mock_executor): diff --git a/tests/test_rate_limit.py b/tests/test_rate_limit.py index a7b17299..bbf8cec4 100644 --- a/tests/test_rate_limit.py +++ b/tests/test_rate_limit.py @@ -238,7 +238,8 @@ def test_failed_request_parses_headers(self): with main._rate_limit_lock: assert main._rate_limit_info["remaining"] == 50 - def test_429_without_retry_after_uses_exponential_backoff(self): + @patch('random.random', return_value=0.5) + def test_429_without_retry_after_uses_exponential_backoff(self, mock_random): """Test that 429 without Retry-After falls back to exponential backoff.""" mock_request = MagicMock() mock_response = MagicMock(spec=httpx.Response) @@ -260,7 +261,7 @@ def test_429_without_retry_after_uses_exponential_backoff(self): request_func = MagicMock(side_effect=[error, error, success_response]) # With delay=1, backoff should be: 1s, 2s - # Total wait should be >= 3 seconds + # Total wait should be >= 3 seconds (assuming jitter factor 1.0) start_time = time.time() result = main._retry_request(request_func, max_retries=3, delay=1) elapsed = time.time() - start_time diff --git a/tests/test_retry_jitter.py b/tests/test_retry_jitter.py index e1e0435c..414f901b 100644 --- a/tests/test_retry_jitter.py +++ b/tests/test_retry_jitter.py @@ -124,6 +124,7 @@ def test_four_hundred_errors_still_fail_fast(self): def test_429_rate_limit_retries_with_jitter(self): """Verify 429 rate limit errors retry with jittered backoff.""" response = Mock(status_code=429) + response.headers = {} error = httpx.HTTPStatusError( "Too many requests", request=Mock(),