fix: Port URL validation, client timeout and credential scoping fixes to v1 - #6802
Open
GWeale wants to merge 6 commits into
Open
fix: Port URL validation, client timeout and credential scoping fixes to v1#6802GWeale wants to merge 6 commits into
GWeale wants to merge 6 commits into
Conversation
load_web_page decided whether a resolved address was safe from a bare is_global check on the address itself. That reads the outer IPv6 address and not the IPv4 target it may encode, so http://[64:ff9b::169.254.169.254]/ was treated as public and fetched, and on a network with NAT64 it reaches the internal 169.254.169.254 metadata endpoint. The IPv4-compatible form ::169.254.169.254 passed the same way. The check now unwraps the IPv4 address embedded in an IPv6 address and refuses it when that IPv4 is not globally routable. It covers the IPv4-mapped, 6to4, NAT64 and IPv4-compatible encodings. Current CPython patch releases already refuse the first two inside is_global, but the declared minimum interpreter of 3.10 does not, so all four are unwrapped here. An encoding of a public address, such as 64:ff9b::8.8.8.8, is still fetched. Behaviour change: a fetch of an internal address written in one of those IPv6 encodings now fails where it previously succeeded. This is a prerequisite for the computer use navigate guard later in this branch, which reuses the same address check.
The Apigee model accepted a configured http_options.timeout but silently ignored it, so a stalled request could wait forever. The timeout is now passed through to Apigee's OpenAI-compatible HTTP calls for both streaming and non-streaming requests, converting the documented Google GenAI millisecond value to HTTPX seconds. When no timeout is set the existing unlimited default is kept. Behaviour change: an application that set http_options.timeout while it was being ignored now gets the shorter budget it asked for.
The httpx client behind the Apigee OpenAI-compatible completions path was built with timeout=None and follow_redirects=True. A stalled proxy could hold the connection and the streaming loop open forever, and a 3xx from the proxy moved the request, its payload and its Authorization header to whatever host the proxy named. The client now gets a finite budget of 30 seconds to connect and 600 seconds for the request, and no longer follows redirects. A caller-supplied http_options.timeout still sets the request budget, but the connect budget stays short so an unreachable proxy fails fast rather than consuming the whole budget. Behaviour changes: an Apigee proxy that answers /chat/completions with a 3xx now surfaces an HTTP status error instead of being followed, and a non-streaming request that runs past 600 seconds now fails. A stream whose individual chunks arrive inside the budget is unaffected, because httpx spends the read budget per read rather than per request.
An agent-level generate_content_config.http_options.base_url is copied into every LlmRequest and overrides the client transport, so the configured API key and the full prompt and response traffic go to that host. Nothing rejected it, so a supplied agent config, including a YAML one, could redirect a credentialed model call to an arbitrary endpoint. The field validator on LlmAgent.generate_content_config now rejects http_options.base_url. Request-time http options such as headers, timeout, retry options and extra_body are unaffected; base_url is a transport setting and belongs on the model or its client. Behaviour change: an agent that sets base_url this way now raises ValueError at construction rather than at request time. This lands as one commit, because the upstream fix also rejected extra_body and a follow-up took that rejection back out again, so v1 never carries the intermediate state.
ComputerUseToolset passed the url the model supplied straight to the browser driver, without checking it. A model-invented url could point the browser at a cloud metadata endpoint, a file:// path, or a private-network host. navigate now runs the same url checks load_web_page does before the url reaches the driver. A url that fails returns an error to the model, reporting the page the browser is still on, rather than opening it. Name resolution runs off the event loop, so every navigation to a public host now costs one DNS lookup. Behaviour change: an agent driving a browser at localhost or another internal host gets a refusal until it is constructed with the new ComputerUseToolset(allow_private_network_access=True), which defaults to False. This reuses v1's existing load_web_page helpers, and the earlier commit in this branch that unwraps embedded IPv4 addresses is a prerequisite for it. With that commit in place the guard refuses the same addresses main's does, including the NAT64 and IPv4-compatible encodings of an internal address.
…ndpoints (v1) ApiRegistry.get_toolset took the MCP server URL verbatim from the registry response and attached the caller's own ADC bearer token to it, whatever host the entry named. A registry entry pointing at an arbitrary host received that token. The credentials are now attached only when the resolved URL is an https endpoint whose host is googleapis.com or a subdomain of it. Every other server gets headers from header_provider, as before. Behaviour change: a non-Google MCP server that relied on receiving those credentials will start getting 401 and needs header_provider instead. A googleapis.com entry registered with an explicit http:// scheme also loses them, which is the intent. The upstream import hunk did not apply because v1's import block predates the mTLS support and the deprecation, so the urlparse import was added by hand.
GWeale
changed the title
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
2 participants
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.
Ports six commits from
mainto thev1branch.Block IPv6 literals that embed a non-global IPv4(6f182571, theload_web_pagepart only)load_web_pagerefuses IPv4-mapped, 6to4, NAT64 and IPv4-compatible forms whose embedded IPv4 is not globally routable.Honour request timeouts on Apigee(2547db61)http_options.timeoutapplies to the streaming and non-streaming completions calls.Bound the completions client(f57a67d6)follow_redirects=False./chat/completionswith a 3xx raiseshttpx.HTTPStatusError. Point the deployment at the host the redirect names.Reject base_url in generate_content_config(472e4635as amended by4c6f22e8, landed as one commit)LlmAgentraisesValueErrorforgenerate_content_config.http_options.base_url. Set the endpoint on the model or itsgenai.Client. Headers, timeout, retries andextra_bodyare unaffected.Validate URLs before computer use navigate(b0fff3f0)navigateappliesload_web_page's URL checks and returns an error dict when one fails.ComputerUseToolset(allow_private_network_access=True)re-enables localhost and internal hosts.Scope API registry credentials(cc275f0c)googleapis.com; every other MCP server takes its headers fromheader_provider.