From defc47dd57e426b96ddba1ffaff2ebe14501eadd Mon Sep 17 00:00:00 2001 From: Anton Dziatkovskii <194927794+tonydzi@users.noreply.github.com> Date: Thu, 3 Sep 2026 08:49:56 -0700 Subject: [PATCH 1/3] Python: pin the validated address for OpenAPI plugin requests `validate_server_url` resolved the operation host and blocked private, loopback, link-local and metadata addresses, then discarded the addresses it had vetted. `run_operation` then issued the request against the hostname, so httpx resolved it a second time when opening the connection. A name that resolved to a public address during validation and to a private one at connect time (DNS rebinding) passed the check and was contacted anyway, with the `auth_callback` credentials attached. `validate_server_url` now returns the addresses it actually vetted, and the built-in client sends the request to one of them: the URL carries the address, the `Host` header and the `sni_hostname` extension carry the original hostname, so TLS verification and the request on the wire are unchanged. The remaining vetted addresses are used as fallbacks when a connection cannot be established. Nothing is pinned where no DNS validation took place: an `allowed_base_urls` match, `allow_private_network_access`, a literal IP host, a caller-supplied `http_client`, or a configured environment proxy (which resolves the name itself). The return value is additive, so existing callers are unaffected. Fixes #14312 Assisted-by: Claude Code / claude-opus-5 Machine: A-Mac16-2019-PaloAlto.local Account: tonydzi Operator: robot:connector-butcher Signed-off-by: tonydzi --- .../openapi_plugin/openapi_runner.py | 75 ++++- .../openapi_plugin/server_url_validator.py | 27 +- .../test_openapi_runner_dns_pinning.py | 307 ++++++++++++++++++ .../test_server_url_validator.py | 36 ++ 4 files changed, 433 insertions(+), 12 deletions(-) create mode 100644 python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py diff --git a/python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py b/python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py index ee88d4111ffb..b21bdcfa57e4 100644 --- a/python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py +++ b/python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py @@ -1,5 +1,6 @@ # Copyright (c) Microsoft. All rights reserved. +import ipaddress import json import logging from collections import OrderedDict @@ -7,6 +8,7 @@ from inspect import isawaitable from typing import Any from urllib.parse import urlparse, urlunparse +from urllib.request import getproxies import httpx from openapi_core import Spec @@ -29,6 +31,33 @@ logger: logging.Logger = logging.getLogger(__name__) +def _pin_url_to_address(url: str, address: ipaddress.IPv4Address | ipaddress.IPv6Address) -> tuple[str, str, str]: + """Rewrite a URL so the connection targets `address` while keeping the original host identity. + + Returns the address-form URL, the `Host` header value and the TLS SNI hostname, all + derived from the original URL so that the request on the wire is unchanged apart from + the address it is delivered to. IPv6 bracketing, the port and any userinfo are + preserved by `httpx.URL.copy_with`. + """ + original = httpx.URL(url) + return ( + str(original.copy_with(host=str(address))), + original.netloc.decode("ascii"), + original.raw_host.decode("ascii"), + ) + + +def _has_environment_proxy() -> bool: + """Return whether a proxy is configured in the environment for outbound HTTP requests. + + Deliberately conservative: any configured proxy disables address pinning, because a + proxy resolves the target name itself, so an address resolved locally is neither the + one used for the connection nor necessarily reachable or correct from the proxy. + """ + proxies = getproxies() + return any(proxies.get(scheme) for scheme in ("http", "https", "all")) + + @experimental class OpenApiRunner: """The OpenApiRunner that runs the operations defined in the OpenAPI manifest.""" @@ -134,7 +163,13 @@ async def run_operation( arguments: KernelArguments | None = None, options: RestApiRunOptions | None = None, ) -> str: - """Runs the operation defined in the OpenAPI manifest.""" + """Runs the operation defined in the OpenAPI manifest. + + When the URL is validated by DNS resolution, the request issued by the built-in + client is pinned to one of the validated addresses. Requests made through a + caller-supplied `http_client`, or while an environment proxy is configured, use + that transport's own name resolution and are not pinned. + """ if not arguments: arguments = KernelArguments() url = self.build_operation_url( @@ -143,7 +178,7 @@ async def run_operation( server_url_override=options.server_url_override if options else None, api_host_url=options.api_host_url if options else None, ) - await validate_server_url(url, self.server_url_validation_options) + validated_addresses = await validate_server_url(url, self.server_url_validation_options) headers = operation.build_headers(arguments=arguments) payload, _ = self.build_operation_payload(operation=operation, arguments=arguments) @@ -168,22 +203,52 @@ async def run_operation( timeout = options.timeout if options and hasattr(options, "timeout") and options.timeout is not None else None + # Pin the connection to an address the validator actually vetted so that a name which + # resolves to a public address during validation cannot resolve to a private one at + # connect time (DNS rebinding). The list is empty when there is nothing to pin. + pinned_addresses = validated_addresses + if pinned_addresses and _has_environment_proxy(): + logger.debug("An environment proxy is configured; the OpenAPI request address is not pinned.") + pinned_addresses = [] + async def fetch(): - async def make_request(client: httpx.AsyncClient): + async def make_request( + client: httpx.AsyncClient, + pin_to: ipaddress.IPv4Address | ipaddress.IPv6Address | None = None, + ): merged_headers = client.headers.copy() merged_headers.update(headers) + request_url = url + extensions: dict[str, Any] = {} + if pin_to is not None: + request_url, merged_headers["Host"], extensions["sni_hostname"] = _pin_url_to_address(url, pin_to) response = await client.request( method=operation.method, - url=url, + url=request_url, headers=merged_headers, json=json.loads(payload) if payload else None, + extensions=extensions, ) response.raise_for_status() return response.text if hasattr(self, "http_client") and self.http_client is not None: + # A caller-supplied client owns its transport configuration (proxies, mounts, + # custom resolvers), so its connections are left untouched. return await make_request(self.http_client) async with httpx.AsyncClient(timeout=timeout) as client: - return await make_request(client) + if not pinned_addresses: + return await make_request(client) + # Every vetted address is an acceptable target, so keep the resolver's + # fallback behaviour by trying the next one when a connection cannot be + # established. Only connection failures are retried, so no request is + # ever delivered more than once. + *fallback_addresses, final_address = pinned_addresses + for address in fallback_addresses: + try: + return await make_request(client, pin_to=address) + except (httpx.ConnectError, httpx.ConnectTimeout): + logger.debug("Could not connect to validated address %s, trying the next one.", address) + return await make_request(client, pin_to=final_address) return await fetch() diff --git a/python/semantic_kernel/connectors/openapi_plugin/server_url_validator.py b/python/semantic_kernel/connectors/openapi_plugin/server_url_validator.py index a3ede15f0f3c..5daed30d5a74 100644 --- a/python/semantic_kernel/connectors/openapi_plugin/server_url_validator.py +++ b/python/semantic_kernel/connectors/openapi_plugin/server_url_validator.py @@ -33,8 +33,17 @@ async def validate_server_url( url: str, options: ServerUrlValidationOptions | None = None, dns_resolver: DnsResolver | None = None, -) -> None: - """Validate a fully resolved OpenAPI operation URL against the supplied policy.""" +) -> list[ipaddress.IPv4Address | ipaddress.IPv6Address]: + """Validate a fully resolved OpenAPI operation URL against the supplied policy. + + Returns the DNS-resolved addresses that were vetted by this call, in resolver order, + so that the caller can pin the connection to an address the policy actually approved + (closing the DNS check-time/use-time gap known as DNS rebinding). + + The list is empty whenever there is nothing to pin, and callers must then connect + normally: when an allowed base URL matched, when ``allow_private_network_access`` + is set, or when the host is already a literal IP address (which cannot be rebound). + """ options = options or ServerUrlValidationOptions() try: parsed_url = _parse_absolute_url(url) @@ -44,7 +53,7 @@ async def validate_server_url( ) from exc if _matches_allowed_base_url(parsed_url, options.allowed_base_urls): - return + return [] if options.allowed_base_urls: raise FunctionExecutionException( @@ -59,9 +68,9 @@ async def validate_server_url( ) if options.allow_private_network_access: - return + return [] - await _ensure_public_host(parsed_url, dns_resolver) + return await _ensure_public_host(parsed_url, dns_resolver) def try_categorize_non_public_address( @@ -127,7 +136,9 @@ def _matches_path_prefix(url_path: str, base_path: str) -> bool: return url_path.lower().startswith(base_path_with_slash.lower()) -async def _ensure_public_host(parsed_url: ParseResult, dns_resolver: DnsResolver | None) -> None: +async def _ensure_public_host( + parsed_url: ParseResult, dns_resolver: DnsResolver | None +) -> list[ipaddress.IPv4Address | ipaddress.IPv6Address]: host = parsed_url.hostname if host is None: raise FunctionExecutionException(f"The request URI '{parsed_url.geturl()}' does not contain a valid host.") @@ -138,7 +149,8 @@ async def _ensure_public_host(parsed_url: ParseResult, dns_resolver: DnsResolver addresses = await _resolve_host(host, dns_resolver) else: _ensure_public_address(parsed_url.geturl(), ip_address) - return + # A literal IP address cannot be rebound between validation and connection. + return [] if not addresses: raise FunctionExecutionException( @@ -148,6 +160,7 @@ async def _ensure_public_host(parsed_url: ParseResult, dns_resolver: DnsResolver for address in addresses: _ensure_public_address(parsed_url.geturl(), address) + return addresses async def _resolve_host( diff --git a/python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py b/python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py new file mode 100644 index 000000000000..5cdb7d38ef20 --- /dev/null +++ b/python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py @@ -0,0 +1,307 @@ +# Copyright (c) Microsoft. All rights reserved. + +import ipaddress +import socket +from collections import OrderedDict +from unittest.mock import MagicMock + +import httpcore +import httpx +import pytest + +from semantic_kernel.connectors.openapi_plugin.openapi_runner import OpenApiRunner +from semantic_kernel.connectors.openapi_plugin.server_url_validator import ( + ServerUrlValidationOptions, + try_categorize_non_public_address, +) + +HOST = "rebind.example" +PUBLIC_ADDRESS = "93.184.216.34" +SECOND_PUBLIC_ADDRESS = "198.41.0.4" +REBOUND_ADDRESS = "169.254.169.254" +PUBLIC_IPV6_ADDRESS = "2606:2800:220:1:248:1893:25c8:1946" + +RAW_RESPONSE = b"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: 13\r\n\r\nresponse text" + + +def build_runner(url: str, options: ServerUrlValidationOptions | None = None) -> tuple[OpenApiRunner, MagicMock]: + """Build a runner whose operation resolves to `url` and carries no payload.""" + runner = OpenApiRunner({}, server_url_validation_options=options) + operation = MagicMock() + operation.method = "GET" + operation.build_headers.return_value = {} + operation.responses = OrderedDict() + runner.build_operation_url = MagicMock(return_value=url) + runner.build_operation_payload = MagicMock(return_value=(None, None)) + return runner, operation + + +def static_getaddrinfo(host_name: str, addresses: list[str]): + """Return a `socket.getaddrinfo` replacement that answers `host_name` with `addresses`.""" + real_getaddrinfo = socket.getaddrinfo + + def fake_getaddrinfo(host, port=None, *args, **kwargs): + if host == host_name: + return [(socket.AF_INET, socket.SOCK_STREAM, 6, "", (address, port or 0)) for address in addresses] + return real_getaddrinfo(host, port, *args, **kwargs) + + return fake_getaddrinfo + + +def rebinding_getaddrinfo(host_name: str, first_address: str, later_address: str, lookups: list[str]): + """Return a `socket.getaddrinfo` replacement that answers `host_name` differently after the first lookup.""" + real_getaddrinfo = socket.getaddrinfo + + def fake_getaddrinfo(host, port=None, *args, **kwargs): + if host == host_name: + lookups.append(host) + address = first_address if len(lookups) == 1 else later_address + family = socket.AF_INET6 if ":" in address else socket.AF_INET + return [(family, socket.SOCK_STREAM, 6, "", (address, port or 0))] + return real_getaddrinfo(host, port, *args, **kwargs) + + return fake_getaddrinfo + + +class RecordingStream(httpcore.AsyncMockStream): + """A mock network stream that records the TLS SNI hostname and the bytes written to the wire.""" + + def __init__(self, buffer: list[bytes], record: dict) -> None: + super().__init__(buffer) + self._record = record + + async def write(self, buffer: bytes, timeout: float | None = None) -> None: + self._record["written"] += buffer + + async def start_tls(self, ssl_context, server_hostname=None, timeout=None): + self._record["sni_hostname"] = server_hostname + return self + + +class RecordingBackend(httpcore.AsyncNetworkBackend): + """A network backend that records the address the connection is actually opened against. + + A real backend resolves a hostname at connect time, which is exactly the second, unvalidated + lookup this test grid is about, so hostnames are resolved here the same way. + """ + + def __init__(self, record: dict) -> None: + self._record = record + + async def connect_tcp(self, host, port, timeout=None, local_address=None, socket_options=None): + try: + ipaddress.ip_address(host) + except ValueError: + connect_target = socket.getaddrinfo(host, port, type=socket.SOCK_STREAM)[0][4][0] + else: + connect_target = host + self._record["connect_target"] = connect_target + return RecordingStream([RAW_RESPONSE], self._record) + + +def install_recording_client(monkeypatch) -> dict: + """Make the runner's built-in client speak to a recording backend through real httpx machinery.""" + record: dict = {"written": b"", "connect_target": None, "sni_hostname": None} + real_client_type = httpx.AsyncClient + + def client_factory(**kwargs): + transport = httpx.AsyncHTTPTransport() + # httpx exposes no public seam for the network backend, so the test reaches into the + # transport's pool. Everything above it (URL handling, headers, extensions) is real. + transport._pool._network_backend = RecordingBackend(record) + return real_client_type(transport=transport, **kwargs) + + monkeypatch.setattr(httpx, "AsyncClient", client_factory) + return record + + +def install_capturing_client(monkeypatch, responses: list | None = None) -> list[httpx.Request]: + """Make the runner's built-in client capture the request it sends instead of connecting.""" + requests: list[httpx.Request] = [] + real_client_type = httpx.AsyncClient + outcomes = list(responses or []) + + def handler(request: httpx.Request) -> httpx.Response: + requests.append(request) + outcome = outcomes.pop(0) if outcomes else None + if isinstance(outcome, Exception): + raise outcome + return httpx.Response(200, text="response text") + + def client_factory(**kwargs): + return real_client_type(transport=httpx.MockTransport(handler), **kwargs) + + monkeypatch.setattr(httpx, "AsyncClient", client_factory) + return requests + + +async def test_run_operation_pins_connection_to_validated_address_under_dns_rebinding(monkeypatch): + """A host that resolves public at validation time and private at connect time must not be reached.""" + lookups: list[str] = [] + monkeypatch.setattr(socket, "getaddrinfo", rebinding_getaddrinfo(HOST, PUBLIC_ADDRESS, REBOUND_ADDRESS, lookups)) + record = install_recording_client(monkeypatch) + runner, operation = build_runner(f"https://{HOST}/api/op") + + assert await runner.run_operation(operation, {}, None) == "response text" + + assert record["connect_target"] == PUBLIC_ADDRESS, ( + f"connection was opened against {record['connect_target']}, not the validated address" + ) + assert try_categorize_non_public_address(record["connect_target"]) == (False, "") + assert record["sni_hostname"] == HOST + assert b"Host: rebind.example\r\n" in record["written"] + assert len(lookups) == 1, "the host was resolved a second time at connect time" + + +async def test_run_operation_pins_request_url_and_preserves_host_identity(monkeypatch): + """The request is addressed to the validated IP while keeping the original Host and SNI.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS])) + requests = install_capturing_client(monkeypatch) + runner, operation = build_runner(f"https://{HOST}/api/op?a=1") + + await runner.run_operation(operation, {}, None) + + assert str(requests[0].url) == f"https://{PUBLIC_ADDRESS}/api/op?a=1" + assert requests[0].headers["Host"] == HOST + assert requests[0].extensions["sni_hostname"] == HOST + + +async def test_run_operation_pins_first_validated_address_when_several_are_returned(monkeypatch): + """Pinning uses the resolver's preferred address, not an arbitrary one.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS, SECOND_PUBLIC_ADDRESS])) + requests = install_capturing_client(monkeypatch) + runner, operation = build_runner(f"https://{HOST}/api/op") + + await runner.run_operation(operation, {}, None) + + assert len(requests) == 1 + assert requests[0].url.host == PUBLIC_ADDRESS + + +async def test_run_operation_falls_back_to_the_next_validated_address_on_connect_error(monkeypatch): + """A connection failure falls through to the remaining validated addresses, as the resolver would.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS, SECOND_PUBLIC_ADDRESS])) + requests = install_capturing_client(monkeypatch, responses=[httpx.ConnectError("no route")]) + runner, operation = build_runner(f"https://{HOST}/api/op") + + assert await runner.run_operation(operation, {}, None) == "response text" + + assert [request.url.host for request in requests] == [PUBLIC_ADDRESS, SECOND_PUBLIC_ADDRESS] + + +async def test_run_operation_does_not_retry_a_request_that_may_already_have_been_delivered(monkeypatch): + """Only connection failures fall through. A later failure means the request may already be on the wire.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS, SECOND_PUBLIC_ADDRESS])) + requests = install_capturing_client(monkeypatch, responses=[httpx.ReadTimeout("timed out")]) + runner, operation = build_runner(f"https://{HOST}/api/op") + + with pytest.raises(httpx.ReadTimeout): + await runner.run_operation(operation, {}, None) + + assert [request.url.host for request in requests] == [PUBLIC_ADDRESS], ( + "a request that may already have been delivered was resent to a second address" + ) + + +async def test_run_operation_brackets_ipv6_address_and_preserves_the_port(monkeypatch): + """An IPv6 pin keeps the URL parseable and does not move the request to another port.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_IPV6_ADDRESS])) + requests = install_capturing_client(monkeypatch) + runner, operation = build_runner(f"https://{HOST}:8443/api/op") + + await runner.run_operation(operation, {}, None) + + assert str(requests[0].url) == f"https://[{PUBLIC_IPV6_ADDRESS}]:8443/api/op" + assert requests[0].url.port == 8443 + assert requests[0].headers["Host"] == f"{HOST}:8443" + assert requests[0].extensions["sni_hostname"] == HOST + + +async def test_run_operation_does_not_pin_when_an_allowed_base_url_matches(monkeypatch): + """The allowed-base-url path never resolves the host, so there is no vetted address to pin.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo("api.example.com", [PUBLIC_ADDRESS])) + requests = install_capturing_client(monkeypatch) + runner, operation = build_runner( + "https://api.example.com/api/op", + ServerUrlValidationOptions(allowed_base_urls=["https://api.example.com"]), + ) + + await runner.run_operation(operation, {}, None) + + assert str(requests[0].url) == "https://api.example.com/api/op" + assert "sni_hostname" not in requests[0].extensions + + +async def test_run_operation_does_not_pin_when_private_network_access_is_allowed(monkeypatch): + """Opting into private network access skips resolution, so nothing may be pinned.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo("internal.example", ["10.0.0.5"])) + requests = install_capturing_client(monkeypatch) + runner, operation = build_runner( + "https://internal.example/api/op", + ServerUrlValidationOptions(allow_private_network_access=True), + ) + + await runner.run_operation(operation, {}, None) + + assert str(requests[0].url) == "https://internal.example/api/op" + assert "sni_hostname" not in requests[0].extensions + + +async def test_run_operation_does_not_pin_a_literal_ip_host(monkeypatch): + """A literal address cannot be rebound, so the request is left exactly as it was.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS])) + requests = install_capturing_client(monkeypatch) + runner, operation = build_runner(f"https://{PUBLIC_ADDRESS}/api/op") + + await runner.run_operation(operation, {}, None) + + assert str(requests[0].url) == f"https://{PUBLIC_ADDRESS}/api/op" + assert requests[0].headers["Host"] == PUBLIC_ADDRESS + assert "sni_hostname" not in requests[0].extensions + + +async def test_run_operation_does_not_pin_when_an_environment_proxy_is_configured(monkeypatch): + """A proxy resolves the target name itself, so a locally resolved address must not be forced on it.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS])) + monkeypatch.setattr( + "semantic_kernel.connectors.openapi_plugin.openapi_runner.getproxies", + lambda: {"https": "http://proxy.example:8080"}, + ) + requests = install_capturing_client(monkeypatch) + runner, operation = build_runner(f"https://{HOST}/api/op") + + await runner.run_operation(operation, {}, None) + + assert str(requests[0].url) == f"https://{HOST}/api/op" + assert "sni_hostname" not in requests[0].extensions + + +async def test_run_operation_does_not_pin_a_caller_supplied_client(monkeypatch): + """A caller-supplied client owns its transport, so its requests are left untouched.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS])) + requests: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + requests.append(request) + return httpx.Response(200, text="response text") + + runner, operation = build_runner(f"https://{HOST}/api/op") + runner.http_client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + + await runner.run_operation(operation, {}, None) + await runner.http_client.aclose() + + assert str(requests[0].url) == f"https://{HOST}/api/op" + assert "sni_hostname" not in requests[0].extensions + + +async def test_run_operation_still_blocks_a_host_that_resolves_to_a_private_address(monkeypatch): + """Pinning must not weaken the existing block on non-public resolutions.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [REBOUND_ADDRESS])) + requests = install_capturing_client(monkeypatch) + runner, operation = build_runner(f"https://{HOST}/api/op") + + with pytest.raises(Exception, match="link-local"): + await runner.run_operation(operation, {}, None) + + assert requests == [] diff --git a/python/tests/unit/connectors/openapi_plugin/test_server_url_validator.py b/python/tests/unit/connectors/openapi_plugin/test_server_url_validator.py index 177ff15cb2c6..c90356c7cc26 100644 --- a/python/tests/unit/connectors/openapi_plugin/test_server_url_validator.py +++ b/python/tests/unit/connectors/openapi_plugin/test_server_url_validator.py @@ -1,5 +1,6 @@ # Copyright (c) Microsoft. All rights reserved. +import ipaddress import socket import pytest @@ -168,3 +169,38 @@ async def fake_resolver(host: str): with pytest.raises(FunctionExecutionException, match="returned no addresses"): await validate_server_url("https://empty-dns.example.com/", dns_resolver=fake_resolver) + + +async def test_validate_server_url_returns_validated_addresses_for_pinning(): + async def fake_resolver(host: str): + assert host == "api.example.com" + return ["93.184.216.34", "198.41.0.4"] + + assert await validate_server_url("https://api.example.com/", dns_resolver=fake_resolver) == [ + ipaddress.ip_address("93.184.216.34"), + ipaddress.ip_address("198.41.0.4"), + ] + + +async def test_validate_server_url_returns_validated_ipv6_address_for_pinning(): + async def fake_resolver(host: str): + assert host == "api.example.com" + return ["2606:2800:220:1:248:1893:25c8:1946"] + + assert await validate_server_url("https://api.example.com/", dns_resolver=fake_resolver) == [ + ipaddress.ip_address("2606:2800:220:1:248:1893:25c8:1946") + ] + + +async def test_validate_server_url_returns_no_addresses_for_literal_ip_host(): + assert await validate_server_url("https://93.184.216.34/api") == [] + + +async def test_validate_server_url_returns_no_addresses_for_allowed_base_url(): + options = ServerUrlValidationOptions(allowed_base_urls=["http://api.example.com"]) + assert await validate_server_url("http://api.example.com/api", options) == [] + + +async def test_validate_server_url_returns_no_addresses_when_private_access_is_allowed(): + options = ServerUrlValidationOptions(allow_private_network_access=True) + assert await validate_server_url("https://internal.example/api", options) == [] From 87be6064bf424304c04fcb43a498633fe690067c Mon Sep 17 00:00:00 2001 From: Anton Dziatkovskii <194927794+tonydzi@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:13:32 -0700 Subject: [PATCH 2/3] Python: decide proxy use per URL so bypassed requests stay pinned `_has_environment_proxy()` dropped address pinning whenever any proxy variable was set, without asking whether this URL would actually travel through that proxy. With `HTTPS_PROXY` set and the target host listed in `NO_PROXY`, the request was sent directly and by hostname, so httpx resolved the name a second time and the DNS-rebinding gap this PR closes was reopened for exactly the hosts an operator had exempted. The same held for a proxy configured for a different scheme. `_proxy_carries(url)` replaces it: pinning is now dropped only when a proxy is configured for this URL's scheme (or `all_proxy`) and `urllib.request. proxy_bypass` does not exempt the host. Every other request is direct and keeps the address the policy vetted. `proxy_bypass` is the companion of the `getproxies` already used here, so the two agree on environment and on the platform proxy configuration they both read. Three tests added, driven by real environment variables rather than a patched `getproxies`, so the stdlib makes the decision under test: proxy bypassed for this host stays pinned, proxy for another scheme stays pinned, proxy that does carry the URL is still unpinned. The first two fail on the previous helper. Assisted-by: Claude Code / claude-opus-5 Machine: A-Mac16-2019-PaloAlto.local Account: tonydzi Operator: robot:thread-watch Signed-off-by: tonydzi Co-Authored-By: Claude Opus 5 (1M context) --- .../openapi_plugin/openapi_runner.py | 30 ++++++---- .../test_openapi_runner_dns_pinning.py | 55 +++++++++++++++++++ 2 files changed, 74 insertions(+), 11 deletions(-) diff --git a/python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py b/python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py index b21bdcfa57e4..8799a270b4ac 100644 --- a/python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py +++ b/python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py @@ -8,7 +8,7 @@ from inspect import isawaitable from typing import Any from urllib.parse import urlparse, urlunparse -from urllib.request import getproxies +from urllib.request import getproxies, proxy_bypass import httpx from openapi_core import Spec @@ -47,15 +47,23 @@ def _pin_url_to_address(url: str, address: ipaddress.IPv4Address | ipaddress.IPv ) -def _has_environment_proxy() -> bool: - """Return whether a proxy is configured in the environment for outbound HTTP requests. +def _proxy_carries(url: str) -> bool: + """Return whether a configured proxy would carry this specific request. - Deliberately conservative: any configured proxy disables address pinning, because a - proxy resolves the target name itself, so an address resolved locally is neither the - one used for the connection nor necessarily reachable or correct from the proxy. + A proxy resolves the target name itself, so an address resolved locally is neither the + one used for the connection nor necessarily reachable or correct from the proxy, and + pinning has to be dropped for such a request. That is decided per URL rather than per + process: a request whose scheme has no proxy, or whose host is exempted by `no_proxy`, + is sent directly and therefore keeps the address the policy vetted. """ + parsed = httpx.URL(url) proxies = getproxies() - return any(proxies.get(scheme) for scheme in ("http", "https", "all")) + if not (proxies.get(parsed.scheme) or proxies.get("all")): + return False + host = f"[{parsed.host}]" if ":" in parsed.host else parsed.host + if parsed.port is not None: + host = f"{host}:{parsed.port}" + return not proxy_bypass(host) @experimental @@ -167,8 +175,8 @@ async def run_operation( When the URL is validated by DNS resolution, the request issued by the built-in client is pinned to one of the validated addresses. Requests made through a - caller-supplied `http_client`, or while an environment proxy is configured, use - that transport's own name resolution and are not pinned. + caller-supplied `http_client`, or through an environment proxy that carries this + URL, use that transport's own name resolution and are not pinned. """ if not arguments: arguments = KernelArguments() @@ -207,8 +215,8 @@ async def run_operation( # resolves to a public address during validation cannot resolve to a private one at # connect time (DNS rebinding). The list is empty when there is nothing to pin. pinned_addresses = validated_addresses - if pinned_addresses and _has_environment_proxy(): - logger.debug("An environment proxy is configured; the OpenAPI request address is not pinned.") + if pinned_addresses and _proxy_carries(url): + logger.debug("An environment proxy carries this URL; the OpenAPI request address is not pinned.") pinned_addresses = [] async def fetch(): diff --git a/python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py b/python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py index 5cdb7d38ef20..477541bafa90 100644 --- a/python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py +++ b/python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py @@ -276,6 +276,61 @@ async def test_run_operation_does_not_pin_when_an_environment_proxy_is_configure assert "sni_hostname" not in requests[0].extensions +@pytest.fixture +def no_proxy_environment(monkeypatch): + """Clear every proxy variable so a test states the whole proxy environment it runs in.""" + for name in ("HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY", "REQUEST_METHOD"): + monkeypatch.delenv(name, raising=False) + monkeypatch.delenv(name.lower(), raising=False) + return monkeypatch + + +async def test_run_operation_pins_when_the_environment_proxy_is_bypassed_for_this_host( + monkeypatch, no_proxy_environment +): + """`no_proxy` means this request is direct, so it must keep the address the policy vetted.""" + no_proxy_environment.setenv("HTTPS_PROXY", "http://proxy.example:8080") + no_proxy_environment.setenv("NO_PROXY", HOST) + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS])) + requests = install_capturing_client(monkeypatch) + runner, operation = build_runner(f"https://{HOST}/api/op") + + await runner.run_operation(operation, {}, None) + + assert str(requests[0].url) == f"https://{PUBLIC_ADDRESS}/api/op" + assert requests[0].headers["Host"] == HOST + assert requests[0].extensions["sni_hostname"] == HOST + + +async def test_run_operation_pins_when_the_environment_proxy_is_for_another_scheme(monkeypatch, no_proxy_environment): + """An `http_proxy` does not carry an https request, which therefore stays direct and pinned.""" + no_proxy_environment.setenv("HTTP_PROXY", "http://proxy.example:8080") + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS])) + requests = install_capturing_client(monkeypatch) + runner, operation = build_runner(f"https://{HOST}/api/op") + + await runner.run_operation(operation, {}, None) + + assert str(requests[0].url) == f"https://{PUBLIC_ADDRESS}/api/op" + assert requests[0].headers["Host"] == HOST + + +async def test_run_operation_does_not_pin_when_the_environment_proxy_carries_this_url( + monkeypatch, no_proxy_environment +): + """The same environment, with this host not exempted, must still drop pinning.""" + no_proxy_environment.setenv("HTTPS_PROXY", "http://proxy.example:8080") + no_proxy_environment.setenv("NO_PROXY", "unrelated.example") + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS])) + requests = install_capturing_client(monkeypatch) + runner, operation = build_runner(f"https://{HOST}/api/op") + + await runner.run_operation(operation, {}, None) + + assert str(requests[0].url) == f"https://{HOST}/api/op" + assert "sni_hostname" not in requests[0].extensions + + async def test_run_operation_does_not_pin_a_caller_supplied_client(monkeypatch): """A caller-supplied client owns its transport, so its requests are left untouched.""" monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS])) From 277e906d7c8e5665c947463538883ec0b9c1c112 Mon Sep 17 00:00:00 2001 From: Anton Dziatkovskii <194927794+tonydzi@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:27:37 -0700 Subject: [PATCH 3/3] Python: keep the pinned address off the environment proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deciding proxy use per URL was only half of it. Pinning rewrites the request URL to the validated address, and httpx then matches its environment-proxy mounts against that address rather than against the hostname. So a host exempted by `no_proxy` — the exact case this now detects — was pinned and then handed to the proxy in address form, undoing the exemption the operator configured. Measured with real httpx 0.28.1, `HTTPS_PROXY` set and `NO_PROXY=rebind.example`: hostname url, no mounts -> direct pinned url, no mounts -> PROXIED -> proxy.example pinned url, direct mount -> direct The built-in client is now built with `mounts={"all://
": None}` for every address it may pin, which mounts those addresses on the default transport. That is the routing the pinning decision already established: this request is direct. Nothing is mounted when nothing is pinned, so unpinned requests keep the transport's own routing untouched. Four tests: the mount for a pinned address, one per fallback address, IPv6 bracketing in the mount pattern, and an empty mapping when the request is not pinned. Assisted-by: Claude Code / claude-opus-5 Machine: A-Mac16-2019-PaloAlto.local Account: tonydzi Operator: robot:thread-watch Signed-off-by: tonydzi Co-Authored-By: Claude Opus 5 (1M context) --- .../openapi_plugin/openapi_runner.py | 21 ++++++- .../test_openapi_runner_dns_pinning.py | 61 ++++++++++++++++++- 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py b/python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py index 8799a270b4ac..fd9e1b5c2bc0 100644 --- a/python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py +++ b/python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py @@ -47,6 +47,25 @@ def _pin_url_to_address(url: str, address: ipaddress.IPv4Address | ipaddress.IPv ) +def _direct_mounts( + addresses: list[ipaddress.IPv4Address | ipaddress.IPv6Address], +) -> dict[str, None]: + """Return httpx mounts that keep the pinned addresses off any environment proxy. + + Pinning rewrites the request URL to the address, and httpx then matches its own + environment-proxy mounts against that address rather than against the hostname. A host + exempted by `no_proxy` would therefore be sent to the proxy in address form, undoing the + exemption. Mapping each vetted address to `None` mounts it on the default transport, + which is what this request was already determined to be: direct. + """ + return {f"all://{_bracketed(address)}": None for address in addresses} + + +def _bracketed(address: ipaddress.IPv4Address | ipaddress.IPv6Address) -> str: + """Return the address as it appears in a URL authority, bracketing IPv6.""" + return f"[{address}]" if address.version == 6 else str(address) + + def _proxy_carries(url: str) -> bool: """Return whether a configured proxy would carry this specific request. @@ -244,7 +263,7 @@ async def make_request( # A caller-supplied client owns its transport configuration (proxies, mounts, # custom resolvers), so its connections are left untouched. return await make_request(self.http_client) - async with httpx.AsyncClient(timeout=timeout) as client: + async with httpx.AsyncClient(timeout=timeout, mounts=_direct_mounts(pinned_addresses)) as client: if not pinned_addresses: return await make_request(client) # Every vetted address is an acceptable target, so keep the resolver's diff --git a/python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py b/python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py index 477541bafa90..cf84fb6f63eb 100644 --- a/python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py +++ b/python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py @@ -115,7 +115,9 @@ def client_factory(**kwargs): return record -def install_capturing_client(monkeypatch, responses: list | None = None) -> list[httpx.Request]: +def install_capturing_client( + monkeypatch, responses: list | None = None, client_kwargs: list | None = None +) -> list[httpx.Request]: """Make the runner's built-in client capture the request it sends instead of connecting.""" requests: list[httpx.Request] = [] real_client_type = httpx.AsyncClient @@ -129,6 +131,8 @@ def handler(request: httpx.Request) -> httpx.Response: return httpx.Response(200, text="response text") def client_factory(**kwargs): + if client_kwargs is not None: + client_kwargs.append(kwargs) return real_client_type(transport=httpx.MockTransport(handler), **kwargs) monkeypatch.setattr(httpx, "AsyncClient", client_factory) @@ -331,6 +335,61 @@ async def test_run_operation_does_not_pin_when_the_environment_proxy_carries_thi assert "sni_hostname" not in requests[0].extensions +async def test_run_operation_keeps_the_pinned_address_off_an_environment_proxy(monkeypatch, no_proxy_environment): + """httpx matches its proxy mounts against the rewritten URL, so the address needs its own exemption.""" + no_proxy_environment.setenv("HTTPS_PROXY", "http://proxy.example:8080") + no_proxy_environment.setenv("NO_PROXY", HOST) + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS])) + kwargs: list[dict] = [] + install_capturing_client(monkeypatch, client_kwargs=kwargs) + runner, operation = build_runner(f"https://{HOST}/api/op") + + await runner.run_operation(operation, {}, None) + + assert kwargs[0]["mounts"] == {f"all://{PUBLIC_ADDRESS}": None} + + +async def test_run_operation_mounts_every_fallback_address_directly(monkeypatch, no_proxy_environment): + """The fallback addresses travel the same way as the first one.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS, SECOND_PUBLIC_ADDRESS])) + kwargs: list[dict] = [] + install_capturing_client(monkeypatch, client_kwargs=kwargs) + runner, operation = build_runner(f"https://{HOST}/api/op") + + await runner.run_operation(operation, {}, None) + + assert kwargs[0]["mounts"] == { + f"all://{PUBLIC_ADDRESS}": None, + f"all://{SECOND_PUBLIC_ADDRESS}": None, + } + + +async def test_run_operation_brackets_an_ipv6_address_in_its_mount(monkeypatch, no_proxy_environment): + """An IPv6 mount pattern carries the same brackets the URL authority uses.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_IPV6_ADDRESS])) + kwargs: list[dict] = [] + install_capturing_client(monkeypatch, client_kwargs=kwargs) + runner, operation = build_runner(f"https://{HOST}/api/op") + + await runner.run_operation(operation, {}, None) + + assert kwargs[0]["mounts"] == {f"all://[{PUBLIC_IPV6_ADDRESS}]": None} + + +async def test_run_operation_mounts_nothing_when_it_does_not_pin(monkeypatch, no_proxy_environment): + """An unpinned request must be left entirely to the transport's own routing.""" + monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS])) + kwargs: list[dict] = [] + install_capturing_client(monkeypatch, client_kwargs=kwargs) + runner, operation = build_runner( + f"https://{HOST}/api/op", ServerUrlValidationOptions(allow_private_network_access=True) + ) + + await runner.run_operation(operation, {}, None) + + assert kwargs[0]["mounts"] == {} + + async def test_run_operation_does_not_pin_a_caller_supplied_client(monkeypatch): """A caller-supplied client owns its transport, so its requests are left untouched.""" monkeypatch.setattr(socket, "getaddrinfo", static_getaddrinfo(HOST, [PUBLIC_ADDRESS]))