🛡️ Sentinel: [CRITICAL/HIGH] Fix fail-closed DNS validation logic

[abhimehro]

🚨 Severity: HIGH
💡 Vulnerability: The validate_hostname function contained a logic error where it implicitly returned None (falsy) for valid domains that passed DNS checks, causing the tool to reject all domain-based URLs (DoS) and only accept IP literals. It also contained unreachable code due to a structural error.
🎯 Impact: The tool was functionally broken for its primary use case (fetching blocklists from domains), and the security control (validating hostnames) was effectively failing closed in an unintended way.
🔧 Fix: Removed unreachable code and added an explicit return True for the success path in validate_hostname. Also fixed unrelated test failures in tests/test_push_rules_perf.py, tests/test_retry_jitter.py, and tests/test_rate_limit.py to ensure a clean CI.
✅ Verification: Ran tests/test_hostname_validation.py and the full test suite (pytest), verifying that valid domains are now accepted and invalid ones are still rejected.

---

PR created automatically by Jules for task 1617236494771150175 started by @abhimehro

[google-labs-jules]

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[trunk-io]

😎 Merged manually by @abhimehro - details.

[github-actions]

👋 Development Partner is reviewing this PR. Will provide feedback shortly.

[gemini-code-assist]

Summary of Changes

Hello @abhimehro, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a critical issue where the validate_hostname function incorrectly rejected all valid domain-based URLs due to a missing explicit return statement, leading to a "fail-closed" state. The fix ensures proper hostname validation and restores the tool's intended functionality. Additionally, several test suite issues were resolved to maintain CI stability, and a new vulnerability entry was added to the Sentinel documentation.

Highlights

• Critical DNS Validation Fix: Corrected a logic error in the validate_hostname function that caused valid domains to be rejected, effectively breaking the tool's primary function for domain-based URLs. An explicit return True was added for successful validation, and unreachable code was removed.
• Test Suite Stability Improvements: Addressed and fixed several unrelated test failures in test_push_rules_perf.py, test_retry_jitter.py, and test_rate_limit.py to ensure a clean CI pipeline.
• Documentation Update: Documented the "Fail-Closed Logic Error in DNS Validation" in .jules/sentinel.md, outlining the vulnerability, learning points, and prevention strategies.

Changelog

• .jules/sentinel.md
  • Added a new section detailing a "Fail-Closed Logic Error in DNS Validation," including its vulnerability, learning, and prevention.
• main.py
  • Inserted return True after successful IP validation within validate_hostname to correctly indicate a valid hostname.
  • Removed unreachable code related to addr_info checks that were incorrectly placed after a return False.
• tests/test_push_rules_perf.py
  • Removed the assignment self.main = main from the setUp method, as it was no longer needed.
• tests/test_rate_limit.py
  • Applied a patch('random.random', return_value=0.5) decorator to test_429_without_retry_after_uses_exponential_backoff to control randomness during testing.
  • Updated a comment to clarify the total wait time calculation, explicitly mentioning the jitter factor.
• tests/test_retry_jitter.py
  • Initialized response.headers = {} within test_429_rate_limit_retries_with_jitter to prevent potential AttributeError if headers were accessed before being set.

Activity

• PR created automatically by Jules for task 1617236494771150175 started by @abhimehro.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[Copilot]

Pull request overview

This PR fixes a critical logic error in the validate_hostname function where valid domain names were incorrectly rejected due to an implicit None return instead of an explicit return True. The bug effectively caused a denial-of-service for the tool's primary use case (fetching blocklists from domains), only allowing IP literals. The PR also removes unreachable code and fixes unrelated test failures to ensure clean CI.

Changes:

• Fixed DNS validation logic by adding explicit return True for valid domains and removing unreachable code
• Fixed test failures by adding required mock attributes (headers={}) and making jitter-based timing tests deterministic
• Documented the vulnerability and prevention strategy in the security log

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
main.py	Added explicit return True at line 932 in validate_hostname for successful domain validation; removed unreachable code after exception handler
tests/test_retry_jitter.py	Added headers={} to mock response object to support rate limit header parsing
tests/test_rate_limit.py	Mocked random.random to make exponential backoff test deterministic; updated comment to reflect mocked jitter
tests/test_push_rules_perf.py	Removed unused self.main variable assignment
.jules/sentinel.md	Added security incident documentation for the DNS validation logic error

[gemini-code-assist]

Code Review

This pull request correctly fixes a critical logic error in validate_hostname where it would implicitly return None for valid hostnames, causing a denial of service for domain-based URLs. The fix involves adding an explicit return True and removing unreachable code. The associated test fixes in test_push_rules_perf.py, test_rate_limit.py, and test_retry_jitter.py are also correct and improve test robustness. I have one suggestion regarding the removed code in main.py to handle an edge case where a hostname might not resolve to any IP addresses.

[gemini-code-assist]

This check for an empty addr_info was correct in principle but located in unreachable code. By removing it entirely, a subtle bug is introduced: if socket.getaddrinfo returns an empty list (for a hostname that exists but has no A/AAAA records), the function now incorrectly returns True. This check should have been moved into the try block after the getaddrinfo call to ensure that hostnames that don't resolve to an IP address are correctly identified as invalid.

For example:

        try:
            addr_info = socket.getaddrinfo(hostname, None, proto=socket.IPPROTO_TCP)
            if not addr_info:
                log.warning(f"Hostname {sanitize_for_log(hostname)} did not resolve to any addresses.")
                return False
            for res in addr_info:
                # ... rest of the loop