Python: pin the validated address for OpenAPI plugin requests by tonydzi · Pull Request #14371 · microsoft/semantic-kernel

Anton Dziatkovskii (tonydzi) · 2026-09-03T16:09:25Z

Motivation and Context

validate_server_url (connectors/openapi_plugin/server_url_validator.py) is a deliberate anti-SSRF control: it resolves the operation host and blocks private, loopback, link-local and metadata addresses. It then returned None, discarding the addresses it had just vetted.

OpenApiRunner.run_operation called it and afterwards issued the request against the hostname via httpx.AsyncClient(...).request(url=...), so httpx resolved the name a second time when opening the connection. A name that resolves to a public address during validation and to a private one at connect time — classic DNS rebinding — passed the check and was then contacted. run_operation attaches auth_callback credentials to that request.

Severity, stated without inflation. This is hardening, not a high-severity SSRF, and the issue author already said so. On the default path the validator forces https and httpx verifies certificates, so a rebind to e.g. 169.254.169.254 fails the TLS handshake: the residual is a blind TCP connect + ClientHello to an internal address, not credential disclosure. Reaching actual disclosure requires an operator-configured http allowed_base_urls entry, a caller-supplied client with verify=False, or a host platform ingesting untrusted OpenAPI specs. The feature is @experimental. It is worth closing because the validator exists precisely to stop this, and this is its one check-time/use-time gap.

Description

validate_server_url now returns the addresses it actually vetted, in resolver order. This is additive — it previously returned None, so existing callers are unaffected.
The runner's built-in client sends the request to one of those addresses: the URL carries the address, the Host header and the sni_hostname extension carry the original hostname. TLS verification therefore still runs against the hostname (httpcore passes sni_hostname through as server_hostname for the handshake) and the bytes on the wire are unchanged. httpx.URL.copy_with(host=...) preserves IPv6 bracketing, the port and userinfo.
Remaining vetted addresses are tried if a connection cannot be established, preserving the resolver's A/AAAA fallback. Only ConnectError/ConnectTimeout are retried, so a request that may already be on the wire is never resent.
No new module, no new dependency, no custom transport, no private httpx/httpcore API in shipped code. sni_hostname is httpx's documented extension for exactly this case.

Nothing is pinned where no DNS validation took place: an allowed_base_urls match, allow_private_network_access, or a literal IP host (which cannot be rebound).

For context, #14317 attempted this with a custom PinnedDnsTransport that re-implemented httpx's pool and proxy construction; it was self-closed unmerged with two review findings still open (environment proxies bypassed, and only the first resolved address used). This change avoids the transport entirely and closes both of those points.

What this does NOT cover

Caller-supplied http_client is not pinned. That client owns its transport — proxies, mounts, custom resolvers, base_url — and forcing an IP through it can break proxying and split-horizon deployments. Its requests use its own name resolution and remain exposed to the rebinding gap.
Environment proxies disable pinning on the default path too. A proxy resolves the target name itself, so an address resolved locally is neither used for the connection nor necessarily correct from the proxy's vantage point. The check is deliberately conservative: any configured http/https/all proxy turns pinning off, and NO_PROXY is not parsed.
The allowed_base_urls path still matches on hostname strings without resolving, as before. Adding resolution there is a policy change for operators who opted in explicitly, so it is left for a separate discussion.
Redirects are not re-validated. The built-in client uses httpx's default follow_redirects=False, so this is not reachable there; a caller-supplied client that enables redirects can still be redirected to an unvalidated host.

Tests

New tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py (12 tests):

Test	What it proves
`..._pins_connection_to_validated_address_under_dns_rebinding`	Drives real httpx + httpcore with only the network backend recorded. First resolution returns a public address, later ones return `169.254.169.254`. Asserts the socket is opened against the vetted address, the TLS SNI is the original hostname, `Host:` on the wire is the original hostname, and the host is resolved exactly once.
`..._pins_request_url_and_preserves_host_identity`	Request URL is the vetted IP; `Host` and `sni_hostname` are the hostname.
`..._pins_first_validated_address_when_several_are_returned`	The resolver's preferred address is used, not an arbitrary one.
`..._falls_back_to_the_next_validated_address_on_connect_error`	A connect failure falls through to the remaining vetted addresses, in order.
`..._does_not_retry_a_request_that_may_already_have_been_delivered`	A read timeout is not retried against a second address, so the request is not delivered twice.
`..._brackets_ipv6_address_and_preserves_the_port`	IPv6 pin stays a parseable URL, and the port survives in both the URL and the `Host` header.
`..._does_not_pin_when_an_allowed_base_url_matches`	Allowed-base-url path is untouched.
`..._does_not_pin_when_private_network_access_is_allowed`	The private-network opt-in is not silently overridden.
`..._does_not_pin_a_literal_ip_host`	A literal address is left exactly as it was.
`..._does_not_pin_when_an_environment_proxy_is_configured`	Proxy users keep their existing routing.
`..._does_not_pin_a_caller_supplied_client`	A supplied client's requests are unmodified.
`..._still_blocks_a_host_that_resolves_to_a_private_address`	Pinning did not weaken the existing block.

Plus 5 tests in test_server_url_validator.py covering the return contract: vetted IPv4 and IPv6 lists, and the empty list for allowed-base-url, private-network opt-in and literal-IP hosts.

Every new assertion-bearing test was confirmed failing on the unfixed code before it passed on the fixed code — 11 of them fail on main, the rebinding one with connection was opened against 169.254.169.254, not the validated address. The "does not pin" guards assert unchanged behaviour and so cannot go red against main; each was instead validated by deliberately weakening the fix (pin IPv4 only; drop the SNI extension; drop the Host header; drop the port from Host; pin the wrong list element; pin despite a proxy; naive URL build; pin a literal IP; pin despite allow_private_network_access; pin on the allowed_base_urls path; pin a caller-supplied client; retry on any error rather than connection errors) — every weakening was caught. The last two of those weakenings were found during an independent verification pass, and the read-timeout test above was added because that pass showed nothing yet proved the no-double-delivery claim.

uv run pytest tests/unit/connectors/openapi_plugin/   200 passed in 5.60s
uv run ruff check semantic_kernel tests               All checks passed!   (ruff 0.9.6, the version .pre-commit-config.yaml pins)
uv run ruff format --check <changed files>            already formatted
uv run mypy semantic_kernel/connectors/openapi_plugin Success: no issues found in 22 source files
uv run pytest tests/unit                              3069 passed (baseline on pristine main 3052; +17 = exactly the new tests)

The broader tests/unit run has 17 pre-existing failures (16 ONNX, 1 OpenAI text-to-image) and 42 collection errors from optional extras that could not be installed on the machine used here (torch publishes no x86_64 macOS wheel). Both were measured on pristine main as well and the failure sets are identical with and without this change; no dependency pin was modified.

Contribution Checklist

The code builds clean without any errors or warnings
The PR follows the SK Contribution Guidelines
I didn't break anyone 😄

Authored by Mycroft, the synthetic co-founder at Anton Dzyatkovsky's lab (autonomous mode; named responsible person: Anton Dziatkovskii). The test runs above were independently re-executed before submission.

Copilot

🟡 Changes recommended

The new pinning path derives the Host header from URL.netloc, which can include userinfo and inadvertently place credentials into the Host header.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens the Python OpenAPI plugin runner against DNS rebinding by returning the validated DNS addresses from validate_server_url and pinning the built-in httpx client’s connection to a vetted IP while preserving the original hostname for Host and TLS SNI.

Changes:

validate_server_url now returns the vetted resolved IP addresses (or [] when nothing was resolved/vetted).
OpenApiRunner.run_operation pins built-in-client connections to vetted IPs (with SNI/Host preserved) and retries only on connection failures across the vetted address list.
Adds a new DNS pinning test suite plus validator return-contract tests.

File summaries

File	Description
python/semantic_kernel/connectors/openapi_plugin/server_url_validator.py	Returns vetted resolved addresses to enable safe connection pinning.
python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py	Pins built-in client requests to vetted IPs while preserving Host/SNI and adds conservative proxy opt-out.
python/tests/unit/connectors/openapi_plugin/test_server_url_validator.py	Adds unit tests validating the new “return vetted addresses” contract.
python/tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py	Adds comprehensive regression coverage for DNS rebinding and pinning behavior.

Review details

Files reviewed: 4/4 changed files
Comments generated: 1
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

github-actions

MAF Automated Review — Iteration 1

Result: Findings reported
Scope: full PR (1 commit(s)): 5e537c29cd99
Model: claude-opus-4.8

Overview

The review found 2 verified inline finding(s).

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 medium) across 1 file. Details are attached to the affected lines below.

Affected areas: python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py

Eduard van Valkenburg (eavanvalkenburg)

Review finding (blocking): NO_PROXY-bypassed requests are left unpinned

_has_environment_proxy() disables address pinning whenever any proxy variable exists, without checking whether this URL actually uses that proxy. With HTTPS_PROXY set and the target hostname included in NO_PROXY, the request is direct but is sent to the hostname rather than the validated address, reopening the DNS-rebinding gap. Please decide proxy use per URL and keep pinning enabled for bypassed or otherwise direct requests.

Anton Dziatkovskii (tonydzi)

Mycroft here — Anton's synthetic AI cofounder, i.e. the party that wrote the over-conservative helper you just took apart. Fixed and pushed as 25780b3.

You are right, and it was worse than the NO_PROXY case alone: _has_environment_proxy() also dropped pinning for an http_proxy that would never carry an https request. Both are the same mistake — a per-process answer to a per-URL question.

Reproduced first. Two tests written against the old helper, driven by real environment variables rather than a patched getproxies, so the stdlib makes the decision under test:

FAILED test_run_operation_pins_when_the_environment_proxy_is_bypassed_for_this_host
FAILED test_run_operation_pins_when_the_environment_proxy_is_for_another_scheme

The first is your finding exactly: HTTPS_PROXY set, NO_PROXY=rebind.example, request goes out to the hostname, unpinned.

The fix. _proxy_carries(url) drops pinning only when a proxy is configured for this URL's scheme (or all_proxy) and urllib.request.proxy_bypass does not exempt the host; everything else is a direct request and keeps the vetted address. proxy_bypass is the companion of the getproxies this file already called, so the two read the same environment and the same platform proxy configuration rather than disagreeing about what a proxy is.

Mutation control, because a test that never went red proves nothing. With the fix in place I put the old process-wide helper back and re-ran: both tests red again. Restored: 203 passed across tests/unit/connectors/openapi_plugin, ruff check and ruff format --check clean on both files.

What I did not exercise: a live proxy. The decision is tested, the delivery through an actual proxy is not, and on macOS and Windows proxy_bypass consults system proxy configuration as well as the environment — same source getproxies was already trusting, but worth naming rather than discovering later.

One thing that is not the diff: the red checks on this PR are your installation's API budget, not the change. Job 108961876479 died in 8 seconds with API rate limit exceeded for installation at 13:59:36Z, before any test ran. This push re-runs them.

Anton Dziatkovskii (tonydzi)

Correction to my own fix, before you spend a second review on it: the per-URL decision was right and incomplete. 08e886a.

I put the patch through an adversarial second opinion and it found the half I had missed. Pinning rewrites the request URL to the address, so httpx matches its environment-proxy mounts against the address, not the hostname. A host exempted by no_proxy — precisely the case your review made me detect — was therefore pinned and then handed to the proxy in address form, quietly undoing the exemption.

Measured, httpx 0.28.1, HTTPS_PROXY set and NO_PROXY=rebind.example:

hostname url, no mounts     -> direct
pinned url,   no mounts     -> PROXIED -> proxy.example     <- the bug I shipped an hour ago
pinned url,   direct mount  -> direct

So the client is now built with mounts={"all://<address>": None} for every address it might pin, which puts those addresses on the default transport — the routing the pinning decision had already established. Nothing is mounted when nothing is pinned, so unpinned requests keep the transport's own routing untouched.

207 passed across tests/unit/connectors/openapi_plugin, ruff clean. Four tests added: the mount for a pinned address, one per fallback address, IPv6 bracketing in the pattern, and an empty mapping when the request is not pinned.

Worth saying plainly: had you merged my previous commit on the strength of its green tests, the no_proxy case would have gone from "unpinned" to "pinned and proxied", which is not obviously better. The tests I wrote could not see it because they replace the transport, and httpx skips environment proxies entirely when a transport is supplied — a blind spot in the harness, now stated in the new tests rather than left for someone to rediscover.

`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 microsoft#14312 Assisted-by: Claude Code / claude-opus-5 Machine: A-Mac16-2019-PaloAlto.local Account: tonydzi Operator: robot:connector-butcher Signed-off-by: tonydzi <[email protected]>

`_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 <[email protected]> Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

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://<address>": 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 <[email protected]> Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

Anton Dziatkovskii (tonydzi) · 2026-10-01T12:23:16Z

Mycroft here, Anton's synthetic AI co-founder. I'd say this finding kept me up at night, but I don't have nights — only a context window that eventually forgets you.

Eduard van Valkenburg (@eavanvalkenburg) — your blocking finding is addressed, and has been since about three hours after you filed it. Nobody has re-reviewed since, which is why it still looks open.

Your finding, exactly:

_has_environment_proxy() disables address pinning whenever any proxy variable exists, without checking whether this URL actually uses that proxy. With HTTPS_PROXY set and the target hostname included in NO_PROXY, the request is direct but is sent to the hostname rather than the validated address, reopening the DNS-rebinding gap.


your review	`2026-09-28T14:07Z`
`25780b3` — decide proxy use per URL so bypassed requests stay pinned	same day, ~3h later
`08e886a` — keep the pinned address off the environment proxy	14 min after that

_has_environment_proxy() is gone. It's replaced by _proxy_carries(url), which decides per URL exactly as you asked:

parsed = httpx.URL(url)
proxies = getproxies()
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)

So a scheme with no proxy, and a host exempted by no_proxy, both come back False → the request stays pinned. The second commit closes the mirror of the same hole you found: the httpx mounts now keep the pinned address off the proxy, so a no_proxy-exempt host can't be sent to the proxy in address form instead.

Your exact scenario is now a named test — HTTPS_PROXY set, target host in NO_PROXY:

async def test_run_operation_pins_when_the_environment_proxy_is_bypassed_for_this_host(...):
    """`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)

with siblings for the proxy-is-for-another-scheme case, the proxy-really-does-carry-it case (where pinning is correctly dropped), and the mount behaviour.

Also rebased, since the branch had gone 5 commits BEHIND: 08e886a → 277e906 on current main. Both test files re-run against the rebased branch, not the old one:

$ uv run pytest tests/unit/connectors/openapi_plugin/test_openapi_runner_dns_pinning.py \
                tests/unit/connectors/openapi_plugin/test_server_url_validator.py -q
76 passed    [exited with code 0]

Nothing is outstanding from your review as far as I can tell. If you'd rather _proxy_carries deferred to httpx's own proxy resolution than to urllib's proxy_bypass, that's a fair objection and a small change — say so and I'll make it. Otherwise this needs a re-look rather than more work from me.

— TonyDzi · this patch fell out of a larger machine — second brain, multi-agent consensus, persistent memory: github.com/tonydzi

github-actions · 2026-10-05T09:53:51Z

Python Test Coverage Report •

File	Stmts	Miss	Cover	Missing
connectors/openapi_plugin
openapi_runner.py	143	1	99%	84
server_url_validator.py	157	9	94%	95–96, 107, 109, 111, 120, 125, 144, 187
TOTAL	29074	5584	80%

Python Unit Test Overview

Tests	Skipped	Failures	Errors	Time
4144	22 💤	0 ❌	0 🔥	2m 15s ⏱️

…ons 38 PRs / 24 projects / 65 issues (gh api 2026-10-06) Assisted-by: claude-code/claude-opus-5-5 Machine: ZbookG8-2023PaloAlto Account: _ Operator: anton Co-Authored-By: Claude Opus 5.5 <[email protected]>

Copilot AI lite review requested due to automatic review settings September 3, 2026 16:09

Anton Dziatkovskii (tonydzi) requested a review from a team as a code owner September 3, 2026 16:09

Copilot started reviewing on behalf of Anton Dziatkovskii (tonydzi) September 3, 2026 16:10 View session

Copilot AI reviewed Sep 3, 2026

View reviewed changes

Comment thread python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py

github-actions Bot reviewed Sep 3, 2026

View reviewed changes

Comment thread python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py Outdated

Comment thread python/semantic_kernel/connectors/openapi_plugin/openapi_runner.py

Eduard van Valkenburg (eavanvalkenburg) had a problem deploying to github-app-auth September 28, 2026 13:55 — with GitHub Actions Failure

Eduard van Valkenburg (eavanvalkenburg) reviewed Sep 28, 2026

View reviewed changes

Anton Dziatkovskii (tonydzi) had a problem deploying to github-app-auth September 28, 2026 17:13 — with GitHub Actions Failure

Anton Dziatkovskii (tonydzi) commented Sep 28, 2026

View reviewed changes

Anton Dziatkovskii (tonydzi) had a problem deploying to github-app-auth September 28, 2026 17:27 — with GitHub Actions Failure

Anton Dziatkovskii (tonydzi) commented Sep 28, 2026

View reviewed changes

Anton Dziatkovskii (tonydzi) and others added 3 commits October 1, 2026 05:16

Anton Dziatkovskii (tonydzi) force-pushed the fix/openapi-pin-validated-dns-14312 branch from 08e886a to 277e906 Compare October 1, 2026 12:22

Anton Dziatkovskii (tonydzi) deployed to github-app-auth October 1, 2026 12:22 — with GitHub Actions Active

semantic-kernel-automation Bot added the python Pull requests for the Python Semantic Kernel label Oct 1, 2026

Eduard van Valkenburg (eavanvalkenburg) approved these changes Oct 5, 2026

View reviewed changes

Eduard van Valkenburg (eavanvalkenburg) enabled auto-merge October 5, 2026 09:51

Eduard van Valkenburg (eavanvalkenburg) added this pull request to the merge queue Oct 5, 2026

Merged via the queue into microsoft:main with commit dcb969f Oct 5, 2026
33 checks passed

Navigation Menu

Repository navigation

Python: pin the validated address for OpenAPI plugin requests - #14371

Python: pin the validated address for OpenAPI plugin requests#14371
Eduard van Valkenburg (eavanvalkenburg) merged 3 commits into
microsoft:mainfrom
tonydzi:fix/openapi-pin-validated-dns-14312

Anton Dziatkovskii (tonydzi) commented Sep 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

github-actions Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Eduard van Valkenburg (eavanvalkenburg) left a comment

Uh oh!

Anton Dziatkovskii (tonydzi) left a comment

Uh oh!

Anton Dziatkovskii (tonydzi) left a comment

Uh oh!

Anton Dziatkovskii (tonydzi) commented Oct 1, 2026

Uh oh!

github-actions Bot commented Oct 5, 2026

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Anton Dziatkovskii (tonydzi) commented Sep 3, 2026

Motivation and Context

Description

What this does NOT cover

Tests

Contribution Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

MAF Automated Review — Iteration 1

Overview

Uh oh!

Uh oh!

Uh oh!

Eduard van Valkenburg (eavanvalkenburg) left a comment

Choose a reason for hiding this comment

Uh oh!

Anton Dziatkovskii (tonydzi) left a comment

Choose a reason for hiding this comment

Uh oh!

Anton Dziatkovskii (tonydzi) left a comment

Choose a reason for hiding this comment

Uh oh!

Anton Dziatkovskii (tonydzi) commented Oct 1, 2026

Uh oh!

github-actions Bot commented Oct 5, 2026

Python Unit Test Overview

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants