Repository navigation
Preflight /fetch URLs with the shared SSRF hostname check - #345
Merged
Merged
Conversation
cmd_fetch and cmd_fetch_bytes only blocked private peers at connect time. Metadata hostnames that resolve public, and private/metadata targets reached through HTTP_PROXY (opensocket sees the proxy), skipped the denylist /browse already runs. Call ssrf_preflight_url before libcurl; opensocket still re-checks every redirect connect. Co-authored-by: Tyler Reckart <tylerreckart@users.noreply.github.com>
# Conflicts: # CHANGELOG.md
tylerreckart
marked this pull request as ready for review
September 21, 2026 12:59
tylerreckart
enabled auto-merge (squash)
September 21, 2026 12:59
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
/browsealready runsssrf_preflight_url(metadata-host denylist, literal private ranges, resolve-to-private).cmd_fetchandcmd_fetch_bytesonly installedCURLOPT_OPENSOCKETFUNCTION, which sees the TCP peer after curl's resolver — and after an envHTTP(S)_PROXYhop.That left two gaps vs
/browse:metadata.google.internal, …) that resolve to a public IPAgent
/fetch, TUI/CLI fetch, and URL-form image parts all share those entry points. The browse comment already claimed/fetchenforced the same blocklist.Fix
Call the existing
preflight_ssrf_checkwrapper beforecurl_easy_initin both fetch paths. Opensocket still re-validates every connect, including after redirects. Error shape matches/browse:refused — <reason> (SSRF guard).Tests
unit_commands— metadata hostname +127.0.0.1/169.254.169.254/[::1]literals forcmd_fetchandcmd_fetch_bytes(no DNS). 4 fetch cases locally; ASan/UBSan driver clean. Fullunit_commands66/66.Independently mergeable against
main(0456350). Code hunks (cmd_fetch/cmd_fetch_bytes+ fetch-section tests) auto-merge with #328 (cmd_writebak/FIFO, different hunks). CHANGELOG[Unreleased]sibling-conflicts as usual after the first of #321–#344 merges.Note
Low Risk
Narrows SSRF on existing fetch entry points using the same preflight already used by
/browse; connect-time guards remain, with new unit tests for blocked URLs.Overview
/fetchnow runs the shared SSRF hostname preflight before libcurl, matching/browseinstead of relying only on connect-timeCURLOPT_OPENSOCKETFUNCTION.cmd_fetchandcmd_fetch_bytescallpreflight_ssrf_check(wrapper aroundssrf_preflight_url) immediately after the http(s) scheme check and beforecurl_easy_init. That closes gaps where metadata hostnames that resolve to public IPs, or private/metadata targets sent viaHTTP(S)_PROXY, could slip past opensocket validation. Failures use the same shape as browse:refused — <reason> (SSRF guard)(orERR:forcmd_fetch). Opensocket still re-validates every connect, including redirects;/browsecomments were updated to note parity.Tests add
unit_commandscoverage for denylisted metadata hostnames and literal private ranges on both fetch entry points (no DNS). CHANGELOG records the fix under[Unreleased]→ Fixed.Reviewed by Cursor Bugbot for commit 1a6c6d2. Bugbot is set up for automated code reviews on this repo. Configure here.