🛡️ Sentinel: [CRITICAL] Fix SSRF via DNS Rebinding by abhimehro · Pull Request #148 · abhimehro/ctrld-sync

abhimehro · 2026-01-31T11:05:59Z

🚨 Severity: CRITICAL
💡 Vulnerability: SSRF (Server-Side Request Forgery) via DNS Rebinding. An attacker could control a domain to resolve to a public IP during initial validation checks, but switch to a private/local IP (e.g., 127.0.0.1) during the actual HTTP request, bypassing security controls.
🎯 Impact: This could allow an attacker to access internal services or loopback interfaces that should not be accessible from the outside.
🔧 Fix: Implemented Post-Connect Verification. By inspecting the server_addr from the established connection's socket information, we verify the actual IP address used. If it is a private or multicast IP, the request is aborted.
✅ Verification: Added a new test tests/test_ssrf_fix.py which mocks the httpx response stream to simulate a connection to a private IP and asserts that a ValueError is raised. Verified that normal public IP connections are still allowed.

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

…fication Added verification of the remote server's IP address after the connection is established in `_gh_get`. This ensures that even if DNS rebinding occurs (where the domain resolves to a public IP initially but a private IP during connection), the request is blocked if it connects to a private or multicast IP. - Check `stream.get_extra_info("server_addr")` to get the connected IP. - Validate IP using `ipaddress` module to ensure it is global and not multicast. - Raise `ValueError` if validation fails. - Added regression test `tests/test_ssrf_fix.py`. Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>

google-labs-jules · 2026-01-31T11:06:01Z

👋 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 · 2026-01-31T11:06:02Z

Merging to main in this repository is managed by Trunk.

To merge this pull request, check the box to the left or comment /trunk merge below.

gemini-code-assist · 2026-01-31T11:06:14Z

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 introduces a crucial security enhancement to mitigate a critical SSRF vulnerability stemming from DNS Rebinding attacks. By implementing a post-connection IP verification, the system now rigorously checks the actual endpoint of HTTP requests, preventing malicious redirection to internal networks. The changes also include comprehensive test cases to validate the fix and updates to project dependencies and documentation to reflect the security measure.

Highlights

Critical Security Fix: This pull request addresses a critical Server-Side Request Forgery (SSRF) vulnerability caused by DNS Rebinding. The fix implements a 'Post-Connect Verification' mechanism to ensure the actual IP address connected to is not a private or multicast IP, even if initial DNS resolution passed validation.
Post-Connect Verification Logic: The _gh_get function in main.py now inspects the server_addr from the established httpx network stream. If the connected IP is identified as private or multicast, a ValueError is raised, effectively blocking the request.
New Test Coverage: A new test file, tests/test_ssrf_fix.py, has been added. It includes tests that simulate DNS Rebinding by mocking httpx responses to private IPs, asserting that the security fix correctly raises a ValueError, while also verifying that connections to public IPs remain functional.
Dependency and Environment Updates: The minimum required Python version in uv.lock has been adjusted from 3.13 to 3.12, and typing-extensions has been added as a conditional dependency for Python versions below 3.13 to ensure broader compatibility.
Security Documentation: A new markdown file, .jules/sentinel.md, has been added to document the DNS Rebinding vulnerability, the lessons learned, and the prevention strategy implemented.

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.

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. ↩

tests/test_ssrf_fix.py

+
+    # Should not raise
+    result = main._gh_get("https://example.com/config.json")
+    assert result == {"valid": "json"}


gemini-code-assist

Code Review

This pull request introduces a critical security fix for a potential SSRF vulnerability via DNS rebinding. The approach of using post-connect verification by checking the socket's server address is robust and well-implemented. The new tests effectively validate the fix for the intended scenarios.

I've added one critical review comment in main.py to address a case where the fix could be bypassed, and a suggestion in tests/test_ssrf_fix.py to add a test case for this scenario to ensure the fix is comprehensive. Overall, this is an excellent and important contribution to the security of the application.

gemini-code-assist · 2026-01-31T11:08:15Z

main.py

+                try:
+                    ip = ipaddress.ip_address(ip_str)
+                except ValueError:
+                    # server_addr[0] might not be an IP string (unlikely for TCP)
+                    ip = None
+
+                if ip and (not ip.is_global or ip.is_multicast):
+                    raise ValueError(
+                        f"Security Alert: Domain resolved to private IP {ip_str} (DNS Rebinding protection)"
+                    )


This try...except block currently fails open. If ipaddress.ip_address(ip_str) raises a ValueError (e.g., if ip_str is not a valid IP address), ip is set to None. The subsequent check if ip and ... will then be false, and the request processing will continue, bypassing the security validation.

For security-critical checks, it's crucial to fail closed. If the IP address cannot be parsed, we should treat it as a potential threat and abort the request. I suggest modifying this to raise an exception if parsing fails.

Suggested change

try:

ip = ipaddress.ip_address(ip_str)

except ValueError:

# server_addr[0] might not be an IP string (unlikely for TCP)

ip = None

if ip and (not ip.is_global or ip.is_multicast):

raise ValueError(

f"Security Alert: Domain resolved to private IP {ip_str} (DNS Rebinding protection)"

)

try:

ip = ipaddress.ip_address(ip_str)

except ValueError as e:

# Fail closed: if we can't parse the IP, we can't trust it.

raise ValueError(f"Security Alert: Could not parse server IP '{ip_str}' for validation.") from e

if not ip.is_global or ip.is_multicast:

raise ValueError(

f"Security Alert: Domain resolved to private IP {ip_str} (DNS Rebinding protection)"

)

gemini-code-assist · 2026-01-31T11:08:15Z

tests/test_ssrf_fix.py

+
+    # Should not raise
+    result = main._gh_get("https://example.com/config.json")
+    assert result == {"valid": "json"}


To complement the proposed change in main.py and ensure the security check is robust, it would be beneficial to add a test case that verifies the system fails closed when the connected address is not a valid IP string. This ensures that unexpected return values from get_extra_info are handled securely.

assert result == {"valid": "json"} def test_gh_get_blocks_invalid_ip_after_connect(monkeypatch): """ Test that _gh_get raises ValueError if the connection was established to an invalid IP. This ensures we fail-closed if the server address is not a parsable IP. """ # Mock response stream with an invalid IP string mock_stream = MagicMock() mock_stream.get_extra_info.return_value = ('not-a-valid-ip', 443) mock_response = MagicMock(spec=httpx.Response) mock_response.extensions = {"network_stream": mock_stream} mock_response.headers = {} mock_response.iter_bytes.return_value = [b'{}'] mock_response.raise_for_status.return_value = None # Context manager mock for stream() mock_context = MagicMock() mock_context.__enter__.return_value = mock_response mock_context.__exit__.return_value = None # Mock _gh.stream mock_gh = MagicMock() mock_gh.stream.return_value = mock_context monkeypatch.setattr(main, "_gh", mock_gh) # We expect ValueError because the IP is invalid and we should fail closed. with pytest.raises(ValueError, match="Security Alert: Could not parse server IP"): main._gh_get("https://example.com/config.json")

tests/test_ssrf_fix.py

+
+    # Should not raise
+    result = main._gh_get("https://example.com/config.json")
+    assert result == {"valid": "json"}


.jules/sentinel.md

@@ -0,0 +1,4 @@
+## 2025-02-18 - [Preventing SSRF via DNS Rebinding with Post-Connect Verification]


.jules/sentinel.md

@@ -0,0 +1,4 @@
+## 2025-02-18 - [Preventing SSRF via DNS Rebinding with Post-Connect Verification]


tests/test_ssrf_fix.py

@@ -0,0 +1,70 @@
+import pytest


tests/test_ssrf_fix.py

@@ -0,0 +1,70 @@
+import pytest
+import httpx


tests/test_ssrf_fix.py

+
+@pytest.fixture(autouse=True)
+def clear_cache():
+    main._cache.clear()


tests/test_ssrf_fix.py

+    # We expect ValueError because of our security fix
+    # Before the fix, this test will FAIL (it won't raise)
+    with pytest.raises(ValueError, match="Security Alert: Domain resolved to private IP"):
+        main._gh_get("https://example.com/config.json")


tests/test_ssrf_fix.py

+    monkeypatch.setattr(main, "_gh", mock_gh)
+
+    # Should not raise
+    result = main._gh_get("https://example.com/config.json")


main.py

tests/test_ssrf_fix.py

@@ -0,0 +1,70 @@
+import pytest


tests/test_ssrf_fix.py

@@ -0,0 +1,70 @@
+import pytest
+import httpx
+from unittest.mock import MagicMock


tests/test_ssrf_fix.py

+import main
+
+@pytest.fixture(autouse=True)
+def clear_cache():


tests/test_ssrf_fix.py

+def test_gh_get_blocks_private_ip_after_connect(monkeypatch):
+    """
+    Test that _gh_get raises ValueError if the connection was established to a private IP.
+    This simulates a DNS Rebinding attack where the initial check passes but the connection goes to private IP.


tests/test_ssrf_fix.py

+
+@pytest.fixture(autouse=True)
+def clear_cache():
+    main._cache.clear()


tests/test_ssrf_fix.py

+    # We expect ValueError because of our security fix
+    # Before the fix, this test will FAIL (it won't raise)
+    with pytest.raises(ValueError, match="Security Alert: Domain resolved to private IP"):
+        main._gh_get("https://example.com/config.json")


tests/test_ssrf_fix.py

+    monkeypatch.setattr(main, "_gh", mock_gh)
+
+    # Should not raise
+    result = main._gh_get("https://example.com/config.json")


main.py

tests/test_ssrf_fix.py

@@ -0,0 +1,70 @@
+import pytest


tests/test_ssrf_fix.py

@@ -0,0 +1,70 @@
+import pytest
+import httpx
+from unittest.mock import MagicMock


tests/test_ssrf_fix.py

+import main
+
+@pytest.fixture(autouse=True)
+def clear_cache():


tests/test_ssrf_fix.py

+def test_gh_get_blocks_private_ip_after_connect(monkeypatch):
+    """
+    Test that _gh_get raises ValueError if the connection was established to a private IP.
+    This simulates a DNS Rebinding attack where the initial check passes but the connection goes to private IP.


tests/test_ssrf_fix.py

+
+@pytest.fixture(autouse=True)
+def clear_cache():
+    main._cache.clear()


tests/test_ssrf_fix.py

+    # We expect ValueError because of our security fix
+    # Before the fix, this test will FAIL (it won't raise)
+    with pytest.raises(ValueError, match="Security Alert: Domain resolved to private IP"):
+        main._gh_get("https://example.com/config.json")


tests/test_ssrf_fix.py

+    monkeypatch.setattr(main, "_gh", mock_gh)
+
+    # Should not raise
+    result = main._gh_get("https://example.com/config.json")


Copilot

Pull request overview

This PR implements a critical security fix for Server-Side Request Forgery (SSRF) via DNS Rebinding attacks. The vulnerability allowed attackers to bypass URL validation by controlling DNS to resolve to a public IP initially, then switching to a private IP during the actual HTTP request. The fix adds post-connect verification by inspecting the actual IP address from the established socket connection.

Changes:

Added post-connect IP verification in _gh_get() to check the actual connected IP address
Created new test file test_ssrf_fix.py with tests for private and public IP scenarios
Updated Python version requirement from >=3.13 to >=3.12 in uv.lock (inconsistent with pyproject.toml)
Added sentinel documentation explaining the vulnerability and prevention strategy

Reviewed changes

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

Show a summary per file

File	Description
main.py	Implements post-connect SSRF verification using httpx's network_stream extension to check the actual server IP address after connection establishment
tests/test_ssrf_fix.py	Adds pytest-based tests to verify private IPs are blocked and public IPs are allowed after connection
uv.lock	Changes Python requirement to >=3.12 and adds typing-extensions dependency for Python <3.13 compatibility
.python-version	Formatting change only, maintains Python 3.13
.jules/sentinel.md	Documents the DNS rebinding vulnerability, learning, and prevention strategy

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-31T11:13:46Z

main.py

+                if ip and (not ip.is_global or ip.is_multicast):
+                    raise ValueError(
+                        f"Security Alert: Domain resolved to private IP {ip_str} (DNS Rebinding protection)"
+                    )


Consider adding logging when the post-connect SSRF verification cannot be performed (e.g., when network_stream is not available in extensions). While the initial URL validation provides protection, logging would help detect if the defense-in-depth layer is being bypassed due to httpx implementation changes or unexpected conditions. For example, add a log.debug or log.warning when stream is None.

Suggested change

)

)

else:

# If httpx does not expose a network_stream, we cannot perform the

# post-connect SSRF verification; log this for observability.

log.warning(

"Post-connect SSRF verification skipped for %s: network_stream extension missing",

sanitize_for_log(url),

)

Copilot · 2026-01-31T11:13:47Z

tests/test_ssrf_fix.py

+
+@pytest.fixture(autouse=True)
+def clear_cache():
+    main._cache.clear()


The fixture should clear the cache after each test as well to ensure proper test isolation. Change the fixture to use yield:

@pytest.fixture(autouse=True) def clear_cache(): main._cache.clear() yield main._cache.clear()

This pattern is consistent with the setUp/tearDown pattern used in other test files like test_cache_optimization.py.

Suggested change

main._cache.clear()

main._cache.clear()

yield

main._cache.clear()

Copilot · 2026-01-31T11:13:47Z

tests/test_ssrf_fix.py

+import pytest
+import httpx
+from unittest.mock import MagicMock
+import main
+
+@pytest.fixture(autouse=True)
+def clear_cache():
+    main._cache.clear()
+
+def test_gh_get_blocks_private_ip_after_connect(monkeypatch):
+    """
+    Test that _gh_get raises ValueError if the connection was established to a private IP.
+    This simulates a DNS Rebinding attack where the initial check passes but the connection goes to private IP.
+    """
+
+    # Mock response stream with private IP
+    mock_stream = MagicMock()
+    mock_stream.get_extra_info.return_value = ('127.0.0.1', 443)
+
+    mock_response = MagicMock(spec=httpx.Response)
+    mock_response.extensions = {"network_stream": mock_stream}
+    mock_response.headers = {}
+    mock_response.iter_bytes.return_value = [b'{}']
+    mock_response.raise_for_status.return_value = None
+
+    # Context manager mock for stream()
+    mock_context = MagicMock()
+    mock_context.__enter__.return_value = mock_response
+    mock_context.__exit__.return_value = None
+
+    # Mock _gh.stream
+    mock_gh = MagicMock()
+    mock_gh.stream.return_value = mock_context
+
+    monkeypatch.setattr(main, "_gh", mock_gh)
+
+    # We expect ValueError because of our security fix
+    # Before the fix, this test will FAIL (it won't raise)
+    with pytest.raises(ValueError, match="Security Alert: Domain resolved to private IP"):
+        main._gh_get("https://example.com/config.json")
+
+def test_gh_get_allows_public_ip_after_connect(monkeypatch):
+    """
+    Test that _gh_get allows connection if established to a public IP.
+    """
+
+    # Mock response stream with public IP
+    mock_stream = MagicMock()
+    mock_stream.get_extra_info.return_value = ('8.8.8.8', 443)
+
+    mock_response = MagicMock(spec=httpx.Response)
+    mock_response.extensions = {"network_stream": mock_stream}
+    mock_response.headers = {}
+    mock_response.iter_bytes.return_value = [b'{"valid": "json"}']
+    mock_response.raise_for_status.return_value = None
+
+    # Context manager mock
+    mock_context = MagicMock()
+    mock_context.__enter__.return_value = mock_response
+    mock_context.__exit__.return_value = None
+
+    # Mock _gh.stream
+    mock_gh = MagicMock()
+    mock_gh.stream.return_value = mock_context
+
+    monkeypatch.setattr(main, "_gh", mock_gh)
+
+    # Should not raise
+    result = main._gh_get("https://example.com/config.json")
+    assert result == {"valid": "json"}


Consider adding additional test cases to improve coverage of the SSRF fix:

IPv6 loopback address (::1)

IPv6 private address (e.g., fc00::1)

Multicast IP address (e.g., 224.0.0.1)

Case where network_stream is not available in extensions

Case where server_addr is None or empty

These additional tests would help ensure the security fix handles all edge cases correctly.

…itization, add dry-run plan details Incorporates the best changes from 36 Jules PRs, addressing review feedback: Bolt (Performance) - from PR #173: - Pre-compile PROFILE_ID_PATTERN and RULE_PATTERN at module level - Use compiled patterns in is_valid_profile_id_format, validate_profile_id, and is_valid_rule - Supersedes PRs: #140, #143, #152, #155, #158, #161, #167, #170, #173 Sentinel (Security) - from PR #172 with review feedback: - Enhance sanitize_for_log to redact Basic Auth credentials in URLs - Redact sensitive query parameters (token, key, secret, password, etc.) - Handle fragment separators (#) per Gemini Code Assist review - Use [^&#\s]* pattern per Copilot reviewer suggestion - Update docstring per reviewer suggestion - Supersedes PRs: #142, #145, #148, #151, #154, #157, #160, #169, #172 Palette (UX) - from PR #174 with lint fixes: - Add print_plan_details function for dry-run visibility - Fix duplicate render_progress_bar definition bug - Supersedes PRs: #139, #141, #144, #147, #150, #153, #156, #159, #162, #165, #168, #171, #174 Also: #146, #149, #164 (parallel folder deletion) and #166 (auto-fix .env perms) are independent features not consolidated here. Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>

Copilot AI review requested due to automatic review settings January 31, 2026 11:06

github-actions bot added the python label Jan 31, 2026

Copilot started reviewing on behalf of abhimehro January 31, 2026 11:06 View session

github-advanced-security bot found potential problems Jan 31, 2026

View reviewed changes

gemini-code-assist bot reviewed Jan 31, 2026

View reviewed changes

github-advanced-security bot found potential problems Jan 31, 2026

View reviewed changes

Copilot AI reviewed Jan 31, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 8, 2026

Consolidate 36 Jules PRs: pre-compile regex, harden log sanitization, add dry-run plan details #175

Merged

abhimehro closed this Feb 9, 2026

abhimehro deleted the sentinel-ssrf-fix-2214977954974444498 branch February 9, 2026 00:18

		@@ -0,0 +1,4 @@
		## 2025-02-18 - [Preventing SSRF via DNS Rebinding with Post-Connect Verification]

-                    )
+                    )
+        else:
+            # If httpx does not expose a network_stream, we cannot perform the
+            # post-connect SSRF verification; log this for observability.
+            log.warning(
+                "Post-connect SSRF verification skipped for %s: network_stream extension missing",
+                sanitize_for_log(url),
+            )

Conversation

abhimehro commented Jan 31, 2026

Uh oh!

google-labs-jules bot commented Jan 31, 2026

Uh oh!

trunk-io bot commented Jan 31, 2026

Uh oh!

gemini-code-assist bot commented Jan 31, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

Check notice

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Check notice

Check notice

Check notice

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check notice

Check notice

Check notice

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check warning

Check notice

Check notice

Check notice

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants