Skip to content

Commit 356acb6

Browse files
mangeshraut712mangeshraut712
authored
Python: normalize OpenAPI allowlist paths before prefix match (#14668)
## Summary - Decode `%2e`/`%2E` and apply RFC 3986 remove_dot_segments before `allowed_base_urls` path-prefix matching - Closes the gap where `/api/../admin` and `/api/%2e%2e/admin` matched an `/api` allowlist while httpx would request `/admin` - Empty segments (`//`) preserved to match httpx; other percent-encodings left intact ## Tests - Rejects `..` and `%2e%2e` escapes under an `/api` allowlist - Still allows `/api/./users`, `/api/users`, `/api`, `/api/` - Still rejects `/apiusers` (shared prefix chars only) Fixes #14662
1 parent fe6c483 commit 356acb6

3 files changed

Lines changed: 271 additions & 5 deletions

File tree

‎python/semantic_kernel/connectors/openapi_plugin/server_url_validator.py‎

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import ipaddress
44
from typing import Any
5-
from urllib.parse import ParseResult, urlparse
5+
from urllib.parse import ParseResult, unquote, urlparse
66

77
from pydantic import Field
88

@@ -113,12 +113,51 @@ def _effective_port(url: ParseResult) -> int | None:
113113
return None
114114

115115

116+
def _normalize_path(path: str, decode_percent_encoded: bool = False) -> str:
117+
"""Normalize a URL path the same way httpx does before routing.
118+
119+
Applies RFC 3986 §5.2.4 remove_dot_segments. Empty segments are preserved.
120+
Paths that would climb above the root are clamped at ``/``.
121+
"""
122+
if decode_percent_encoded:
123+
path = unquote(path)
124+
125+
# remove_dot_segments (RFC 3986 §5.2.4), preserving empty segments like httpx.
126+
segments = path.split("/")
127+
resolved: list[str] = []
128+
for segment in segments:
129+
if segment == ".":
130+
continue
131+
if segment == "..":
132+
if resolved and (len(resolved) > 1 or resolved[0] != ""):
133+
resolved.pop()
134+
# Protect only the leading root segment; internal empty segments can be popped.
135+
continue
136+
resolved.append(segment)
137+
# Absolute paths keep a leading empty segment from the initial "/".
138+
if not resolved:
139+
return "/"
140+
normalized = "/".join(resolved)
141+
if path.startswith("/") and not normalized.startswith("/"):
142+
normalized = "/" + normalized
143+
return normalized or "/"
144+
145+
116146
def _matches_path_prefix(url_path: str, base_path: str) -> bool:
117147
url_path = url_path or "/"
118148
base_path = base_path or "/"
119149

120-
if url_path == base_path:
121-
return True
150+
for decode_percent_encoded in (False, True):
151+
normalized_url_path = _normalize_path(url_path, decode_percent_encoded)
152+
normalized_base_path = _normalize_path(base_path, decode_percent_encoded)
153+
154+
if normalized_url_path == normalized_base_path:
155+
continue
156+
157+
base_path_with_slash = (
158+
normalized_base_path if normalized_base_path.endswith("/") else f"{normalized_base_path}/"
159+
)
160+
if not normalized_url_path.startswith(base_path_with_slash):
161+
return False
122162

123-
base_path_with_slash = base_path if base_path.endswith("/") else f"{base_path}/"
124-
return url_path.startswith(base_path_with_slash)
163+
return True

‎python/tests/unit/connectors/openapi_plugin/test_openapi_runner.py‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,47 @@ async def test_run_operation_enforces_allowed_base_url_path_case_before_authenti
387387
runner.http_client.request.assert_not_awaited()
388388

389389

390+
@pytest.mark.parametrize(
391+
("allowed_base_url", "url", "is_allowed"),
392+
[
393+
("https://api.example.com/api", "https://api.example.com/api/.", True),
394+
("https://api.example.com/api", "https://api.example.com/api/users/..", True),
395+
("https://api.example.com/api/", "https://api.example.com/api/.", False),
396+
("https://api.example.com/api/", "https://api.example.com/api/users/..", False),
397+
("https://api.example.com/api/", "https://api.example.com/api//.", True),
398+
("https://api.example.com/api//", "https://api.example.com/api//.", False),
399+
],
400+
)
401+
async def test_run_operation_enforces_allowed_base_url_with_wire_targets(allowed_base_url, url, is_allowed):
402+
runner = OpenApiRunner(
403+
{},
404+
server_url_validation_options=ServerUrlValidationOptions(allowed_base_urls=[allowed_base_url]),
405+
)
406+
operation = MagicMock()
407+
operation.method = "GET"
408+
operation.build_headers.return_value = {}
409+
operation.responses = OrderedDict()
410+
runner.build_operation_url = MagicMock(return_value=url)
411+
runner.build_operation_payload = MagicMock(return_value=(None, None))
412+
response = MagicMock()
413+
response.text = "response text"
414+
runner.http_client = AsyncMock()
415+
runner.http_client.headers = {}
416+
runner.http_client.request = AsyncMock(return_value=response)
417+
runner.auth_callback = AsyncMock(return_value={"Authorization": "******"})
418+
419+
if is_allowed:
420+
result = await runner.run_operation(operation)
421+
assert result == "response text"
422+
runner.auth_callback.assert_awaited_once_with()
423+
runner.http_client.request.assert_awaited_once()
424+
else:
425+
with pytest.raises(FunctionExecutionException, match="allowed base URLs"):
426+
await runner.run_operation(operation)
427+
runner.auth_callback.assert_not_awaited()
428+
runner.http_client.request.assert_not_awaited()
429+
430+
390431
async def test_run_operation_blocks_disallowed_url_before_request():
391432
runner = OpenApiRunner({})
392433
operation = MagicMock()

‎python/tests/unit/connectors/openapi_plugin/test_server_url_validator.py‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
import ipaddress
44
import socket
55

6+
import httpx
67
import pytest
78

89
from semantic_kernel.connectors.openapi_plugin.server_url_validator import (
910
ServerUrlValidationOptions,
11+
_normalize_path,
1012
validate_server_url,
1113
)
1214
from semantic_kernel.exceptions import FunctionExecutionException
@@ -256,3 +258,187 @@ async def test_validate_server_url_returns_no_addresses_for_allowed_base_url():
256258
async def test_validate_server_url_returns_no_addresses_when_private_access_is_allowed():
257259
options = ServerUrlValidationOptions(allow_private_network_access=True)
258260
assert await validate_server_url("https://internal.example/api", options) == []
261+
262+
263+
@pytest.mark.parametrize(
264+
"url",
265+
[
266+
"https://api.example.com/api/../admin",
267+
"https://api.example.com/api/%2e%2e/admin",
268+
"https://api.example.com/api/%2E%2E/admin",
269+
"https://api.example.com/api/foo/../../admin",
270+
"https://api.example.com/api//../../admin",
271+
"https://api.example.com/api//%2e%2e/%2E%2E/admin",
272+
"https://api.example.com/api//.%2e/%2E./admin",
273+
"https://api.example.com/api///../../../admin",
274+
"https://api.example.com/api//../../..",
275+
"https://api.example.com/api/%2f../../admin",
276+
],
277+
)
278+
async def test_validate_server_url_rejects_path_prefix_escape(url):
279+
"""Dot-segment escapes must not satisfy an /api allowlist prefix."""
280+
options = ServerUrlValidationOptions(allowed_base_urls=["https://api.example.com/api"])
281+
282+
with pytest.raises(FunctionExecutionException, match="allowed base URLs"):
283+
await validate_server_url(url, options)
284+
285+
286+
@pytest.mark.parametrize(
287+
"url",
288+
[
289+
"https://api.example.com/api/./users",
290+
"https://api.example.com/api//users",
291+
"https://api.example.com/api//../users",
292+
"https://api.example.com/api//%2e%2e/users",
293+
"https://api.example.com/api/users",
294+
"https://api.example.com/api/",
295+
"https://api.example.com/api",
296+
],
297+
)
298+
async def test_validate_server_url_allows_normalized_paths_under_prefix(url):
299+
options = ServerUrlValidationOptions(allowed_base_urls=["https://api.example.com/api"])
300+
301+
assert await validate_server_url(url, options) == []
302+
303+
304+
async def test_validate_server_url_still_rejects_path_that_only_shares_prefix_chars():
305+
options = ServerUrlValidationOptions(allowed_base_urls=["https://api.example.com/api"])
306+
307+
with pytest.raises(FunctionExecutionException, match="allowed base URLs"):
308+
await validate_server_url("https://api.example.com/apiusers", options)
309+
310+
311+
async def test_validate_server_url_rejects_path_that_only_matches_after_decoding():
312+
options = ServerUrlValidationOptions(allowed_base_urls=["https://api.example.com/api"])
313+
314+
with pytest.raises(FunctionExecutionException, match="allowed base URLs"):
315+
await validate_server_url("https://api.example.com/admin/%2e%2e/api", options)
316+
317+
318+
@pytest.mark.parametrize(
319+
"url",
320+
[
321+
"https://api.example.com/api/.",
322+
"https://api.example.com/api/users/..",
323+
"https://api.example.com/api//..",
324+
],
325+
)
326+
async def test_validate_server_url_rejects_terminal_dot_segments_under_trailing_slash_allowlist(url):
327+
"""httpx sends terminal dot segments as bare paths, so an /api/ allowlist must reject them."""
328+
options = ServerUrlValidationOptions(allowed_base_urls=["https://api.example.com/api/"])
329+
330+
with pytest.raises(FunctionExecutionException, match="allowed base URLs"):
331+
await validate_server_url(url, options)
332+
333+
334+
@pytest.mark.parametrize(
335+
"url",
336+
[
337+
"https://api.example.com/api//.",
338+
"https://api.example.com/api//../",
339+
],
340+
)
341+
async def test_validate_server_url_allows_terminal_dot_segments_normalizing_to_trailing_slash(url):
342+
"""httpx sends /api//. and /api//../ as /api/, matching an /api/ allowlist."""
343+
options = ServerUrlValidationOptions(allowed_base_urls=["https://api.example.com/api/"])
344+
345+
assert await validate_server_url(url, options) == []
346+
347+
348+
@pytest.mark.parametrize(
349+
"url",
350+
[
351+
"https://api.example.com/api/.",
352+
"https://api.example.com/api/users/..",
353+
"https://api.example.com/api//.",
354+
"https://api.example.com/api//..",
355+
],
356+
)
357+
async def test_validate_server_url_allows_terminal_dot_segments_under_bare_allowlist(url):
358+
options = ServerUrlValidationOptions(allowed_base_urls=["https://api.example.com/api"])
359+
360+
assert await validate_server_url(url, options) == []
361+
362+
363+
async def test_validate_server_url_rejects_terminal_dot_segment_under_double_slash_allowlist():
364+
"""httpx sends /api//. as /api/, so an /api// allowlist must reject it."""
365+
options = ServerUrlValidationOptions(allowed_base_urls=["https://api.example.com/api//"])
366+
367+
with pytest.raises(FunctionExecutionException, match="allowed base URLs"):
368+
await validate_server_url("https://api.example.com/api//.", options)
369+
370+
371+
@pytest.mark.parametrize(
372+
"allowed_base_url",
373+
[
374+
"https://api.example.com/api/",
375+
"https://api.example.com/api//.",
376+
],
377+
)
378+
async def test_validate_server_url_trailing_slash_allowlist_does_not_allow_bare_path(allowed_base_url):
379+
options = ServerUrlValidationOptions(allowed_base_urls=[allowed_base_url])
380+
381+
with pytest.raises(FunctionExecutionException, match="allowed base URLs"):
382+
await validate_server_url("https://api.example.com/api", options)
383+
384+
385+
async def test_validate_server_url_terminal_dot_allowlist_allows_bare_path():
386+
"""An allowlist ending in /api/. normalizes to bare /api in httpx, so it allows bare /api."""
387+
options = ServerUrlValidationOptions(allowed_base_urls=["https://api.example.com/api/."])
388+
389+
assert await validate_server_url("https://api.example.com/api", options) == []
390+
391+
392+
@pytest.mark.parametrize("decode_percent_encoded", [False, True])
393+
@pytest.mark.parametrize(
394+
("path", "expected"),
395+
[
396+
("/api//../../admin", "/admin"),
397+
("/api///../../../admin", "/admin"),
398+
("/api//..", "/api"),
399+
("/api//../", "/api/"),
400+
("/api//.", "/api/"),
401+
("/api/.", "/api"),
402+
("/api/users/..", "/api"),
403+
("/api//users", "/api//users"),
404+
("/api///users/", "/api///users/"),
405+
("/api//../../..", "/"),
406+
("//../..", "/"),
407+
("/../../admin", "/admin"),
408+
],
409+
)
410+
def test_normalize_path_pops_internal_empty_segments_but_preserves_root(path, expected, decode_percent_encoded):
411+
assert _normalize_path(path, decode_percent_encoded) == expected
412+
413+
414+
@pytest.mark.parametrize(
415+
"path",
416+
[
417+
"/api/.",
418+
"/api/users/..",
419+
"/api//.",
420+
"/api//..",
421+
"/api//../",
422+
"/api//users",
423+
"/api///users/",
424+
"/api//../../admin",
425+
"/api///../../../admin",
426+
"/api/../admin",
427+
"/api/./users",
428+
"/..",
429+
"/../..",
430+
"//../..",
431+
"/api//../../..",
432+
],
433+
)
434+
def test_normalize_path_matches_httpx_wire_path(path):
435+
httpx_wire_path = httpx.URL(f"https://api.example.com{path}").raw_path.decode("ascii").partition("?")[0]
436+
assert _normalize_path(path, decode_percent_encoded=False) == httpx_wire_path
437+
438+
439+
async def test_validate_server_url_preserves_empty_segment_in_allowlist():
440+
options = ServerUrlValidationOptions(allowed_base_urls=["https://api.example.com/api//"])
441+
442+
assert await validate_server_url("https://api.example.com/api//users", options) == []
443+
with pytest.raises(FunctionExecutionException, match="allowed base URLs"):
444+
await validate_server_url("https://api.example.com/api/users", options)

0 commit comments

Comments
 (0)