fix: mitigate SSRF risk in report URL validity checks#82
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50a5aeff2d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except ValueError: | ||
| # Hostname is not a direct IP literal; keep it eligible. | ||
| pass |
There was a problem hiding this comment.
Resolve hostnames before classifying URLs as safe
The new SSRF guard only blocks IP literals; any hostname that is not directly parseable as an IP is marked eligible (except ValueError: ... pass). That means attacker-controlled domains (for example DNS names that resolve to 169.254.169.254 or RFC1918 targets) will still be fetched by session.head, so internal-network probing remains possible when evaluating untrusted reports. Resolve hostnames and reject non-global resolved addresses before making outbound requests.
Useful? React with 👍 / 👎.
Motivation
HEADrequests for everyhttp(s)URL extracted from reports, enabling SSRF/inner-network probing when run on untrusted inputs.Description
ipaddressimport and a helperis_safe_public_urlto reject non-HTTP(S), missing-host,localhost/.local, and private/loopback/link-local/multicast/reserved/unspecified IP literal targets inevaluate_url_validity_asyncinevaluations/research-agent-team-eval/evaluator.py.safe_urls, recordedblocked_urlscount in results, and updated feedback to mention skipped private/internal URLs.HEADrequests (allow_redirects=False) to reduce redirect-based SSRF exposure.Testing
python -m compileall evaluations/research-agent-team-eval/evaluator.py.pytest -q tests/unit_tests/test_oauth_url_validation.pywithPYTHONPATH=., but collection failed due to a missing external dependency (autogen_agentchat) in the environment, so unit tests could not be fully executed here.Codex Task