Skip to content

🛡️ Sentinel: [HIGH] Fix SSRF protection by resolving domains#109

Merged
abhimehro merged 1 commit intomainfrom
sentinel-ssrf-fix-13102230351974662327
Jan 18, 2026
Merged

🛡️ Sentinel: [HIGH] Fix SSRF protection by resolving domains#109
abhimehro merged 1 commit intomainfrom
sentinel-ssrf-fix-13102230351974662327

Conversation

@google-labs-jules
Copy link

@google-labs-jules google-labs-jules bot commented Jan 17, 2026

🚨 Severity: HIGH
💡 Vulnerability: SSRF protection was incomplete as it only checked IP literals but not domains resolving to private IPs.
🎯 Impact: Attackers could access internal services by providing a domain that resolves to a private IP (e.g., DNS Rebinding).
🔧 Fix: Updated validate_folder_url in main.py to resolve domains using socket.getaddrinfo and check the resulting IPs against private/loopback ranges.
✅ Verification: Added tests/test_ssrf.py which verifies that domains resolving to private IPs are blocked. Verified with dry-run on valid URLs.

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

@google-labs-jules
Copy link
Author

google-labs-jules bot commented Jan 17, 2026

👋 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
Copy link

trunk-io bot commented Jan 17, 2026

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.

github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 17, 2026
View reviewed changes
.jules/sentinel.md
1. Parse URLs and check hostnames against `localhost` and private IP ranges using `ipaddress` module.
2. Enforce strict length limits on user inputs (e.g., profile IDs) to prevent resource exhaustion or buffer abuse.

## 2026-01-17 - [SSRF Protection Enhancement]

Check notice

Code scanning / Remark-lint (reported by Codacy)

Warn when references to undefined definitions are found. Note

[no-undefined-references] Found reference to undefined definition
.jules/sentinel.md
1. Parse URLs and check hostnames against `localhost` and private IP ranges using `ipaddress` module.
2. Enforce strict length limits on user inputs (e.g., profile IDs) to prevent resource exhaustion or buffer abuse.

## 2026-01-17 - [SSRF Protection Enhancement]

Check notice

Code scanning / Remark-lint (reported by Codacy)

Warn when shortcut reference links are used. Note

[no-shortcut-reference-link] Use the trailing [] on reference links
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 17, 2026
View reviewed changes
main.py
ip_str = res[4][0]
ip = ipaddress.ip_address(ip_str)
if ip.is_private or ip.is_loopback:
log.warning(f"Skipping unsafe URL (domain {hostname} resolves to private IP {ip}): {sanitize_for_log(url)}")

Check warning

Code scanning / Prospector (reported by Codacy)

Use lazy % formatting in logging functions (logging-fstring-interpolation) Warning

Use lazy % formatting in logging functions (logging-fstring-interpolation)
main.py
log.warning(f"Skipping unsafe URL (domain {hostname} resolves to private IP {ip}): {sanitize_for_log(url)}")
return False
except (socket.gaierror, ValueError, OSError) as e:
log.warning(f"Failed to resolve/validate domain {hostname}: {e}")

Check warning

Code scanning / Prospector (reported by Codacy)

Use lazy % formatting in logging functions (logging-fstring-interpolation) Warning

Use lazy % formatting in logging functions (logging-fstring-interpolation)
tests/test_ssrf.py
@@ -0,0 +1,54 @@
import unittest
from unittest.mock import patch, MagicMock

Check warning

Code scanning / Prospector (reported by Codacy)

Unused MagicMock imported from unittest.mock (unused-import) Warning test

Unused MagicMock imported from unittest.mock (unused-import)
tests/test_ssrf.py
# Add root to path to import main
sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))

import main

Check warning

Code scanning / Prospector (reported by Codacy)

Import "import main" should be placed at the top of the module (wrong-import-position) Warning test

Import "import main" should be placed at the top of the module (wrong-import-position)
tests/test_ssrf.py

self.assertTrue(result, "Should allow domain resolving to public IP")

if __name__ == '__main__':

Check warning

Code scanning / Prospector (reported by Codacy)

expected 2 blank lines after class or function definition, found 1 (E305) Warning test

expected 2 blank lines after class or function definition, found 1 (E305)
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 17, 2026
View reviewed changes
main.py
# res is (family, type, proto, canonname, sockaddr)
# sockaddr is (address, port) for AF_INET/AF_INET6
ip_str = res[4][0]
ip = ipaddress.ip_address(ip_str)

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Variable name "ip" doesn't conform to snake_case naming style Warning

Variable name "ip" doesn't conform to snake_case naming style
main.py
ip_str = res[4][0]
ip = ipaddress.ip_address(ip_str)
if ip.is_private or ip.is_loopback:
log.warning(f"Skipping unsafe URL (domain {hostname} resolves to private IP {ip}): {sanitize_for_log(url)}")

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Line too long (132/100) Warning

Line too long (132/100)
main.py
if ip.is_private or ip.is_loopback:
log.warning(f"Skipping unsafe URL (domain {hostname} resolves to private IP {ip}): {sanitize_for_log(url)}")
return False
except (socket.gaierror, ValueError, OSError) as e:

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Variable name "e" doesn't conform to snake_case naming style Warning

Variable name "e" doesn't conform to snake_case naming style
tests/test_ssrf.py
@@ -0,0 +1,54 @@
import unittest

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Missing module docstring Warning test

Missing module docstring
tests/test_ssrf.py
# Add root to path to import main
sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))

import main

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Import "import main" should be placed at the top of the module Warning test

Import "import main" should be placed at the top of the module
tests/test_ssrf.py

import main

class TestSSRF(unittest.TestCase):

Check warning

Code scanning / Pylintpython3 (reported by Codacy)

Missing class docstring Warning test

Missing class docstring
main.py
ip_str = res[4][0]
ip = ipaddress.ip_address(ip_str)
if ip.is_private or ip.is_loopback:
log.warning(f"Skipping unsafe URL (domain {hostname} resolves to private IP {ip}): {sanitize_for_log(url)}")

Check notice

Code scanning / Pylintpython3 (reported by Codacy)

Use lazy % formatting in logging functions Note

Use lazy % formatting in logging functions
main.py
log.warning(f"Skipping unsafe URL (domain {hostname} resolves to private IP {ip}): {sanitize_for_log(url)}")
return False
except (socket.gaierror, ValueError, OSError) as e:
log.warning(f"Failed to resolve/validate domain {hostname}: {e}")

Check notice

Code scanning / Pylintpython3 (reported by Codacy)

Use lazy % formatting in logging functions Note

Use lazy % formatting in logging functions
tests/test_ssrf.py
@@ -0,0 +1,54 @@
import unittest
from unittest.mock import patch, MagicMock

Check notice

Code scanning / Pylintpython3 (reported by Codacy)

Unused MagicMock imported from unittest.mock Note test

Unused MagicMock imported from unittest.mock
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 17, 2026
View reviewed changes
main.py
# res is (family, type, proto, canonname, sockaddr)
# sockaddr is (address, port) for AF_INET/AF_INET6
ip_str = res[4][0]
ip = ipaddress.ip_address(ip_str)

Check warning

Code scanning / Pylint (reported by Codacy)

Variable name "ip" doesn't conform to snake_case naming style Warning

Variable name "ip" doesn't conform to snake_case naming style
main.py
ip_str = res[4][0]
ip = ipaddress.ip_address(ip_str)
if ip.is_private or ip.is_loopback:
log.warning(f"Skipping unsafe URL (domain {hostname} resolves to private IP {ip}): {sanitize_for_log(url)}")

Check warning

Code scanning / Pylint (reported by Codacy)

Line too long (132/100) Warning

Line too long (132/100)
main.py
if ip.is_private or ip.is_loopback:
log.warning(f"Skipping unsafe URL (domain {hostname} resolves to private IP {ip}): {sanitize_for_log(url)}")
return False
except (socket.gaierror, ValueError, OSError) as e:

Check warning

Code scanning / Pylint (reported by Codacy)

Variable name "e" doesn't conform to snake_case naming style Warning

Variable name "e" doesn't conform to snake_case naming style
tests/test_ssrf.py
@@ -0,0 +1,54 @@
import unittest

Check warning

Code scanning / Pylint (reported by Codacy)

Missing module docstring Warning test

Missing module docstring
tests/test_ssrf.py
# Add root to path to import main
sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))

import main

Check warning

Code scanning / Pylint (reported by Codacy)

Import "import main" should be placed at the top of the module Warning test

Import "import main" should be placed at the top of the module
tests/test_ssrf.py

import main

class TestSSRF(unittest.TestCase):

Check warning

Code scanning / Pylint (reported by Codacy)

Missing class docstring Warning test

Missing class docstring
tests/test_ssrf.py
@@ -0,0 +1,54 @@
import unittest
from unittest.mock import patch, MagicMock

Check notice

Code scanning / Pylint (reported by Codacy)

Unused MagicMock imported from unittest.mock Note test

Unused MagicMock imported from unittest.mock
@abhimehro abhimehro merged commit 9259fef into main Jan 18, 2026
18 checks passed
@abhimehro abhimehro deleted the sentinel-ssrf-fix-13102230351974662327 branch January 18, 2026 05:29
This was referenced Jan 18, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
abhimehro pushed a commit that referenced this pull request Jan 18, 2026
@google-labs-jules
docs: Update Sentinel journal with SSRF learnings
5b88070 
    The implemented code fix for SSRF was superseded by PR #109.
    Retaining the security journal entry for future reference regarding
    DNS resolution and private IP validation.
abhimehro pushed a commit that referenced this pull request Jan 18, 2026
@google-labs-jules
Revert "feat(security): add DNS resolution to SSRF protection"
28627ac 
This reverts commit [previous_commit_hash].

This PR is superseded by PR #109 which implements the same fix.
Reverting changes to close this branch cleanly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@abhimehro