Skip to content

Commit fe6c483

Browse files
eavanvalkenburgeavanvalkenburg
authored
Python: preserve path case in OpenAPI allowed URLs (#14693)
### Motivation and Context OpenAPI allowed base URLs can include a path prefix. URL paths are case-sensitive, so path-prefix matching must preserve case while the existing scheme, host, port, and slash-boundary behavior remains unchanged. No linked issue. ### Description - Compare allowed URL paths without lowercasing them. - Keep case-insensitive scheme and host matching and effective-port comparison. - Add validator coverage for matching paths, descendants, case variants, and prefix controls. - Add GET and POST runner coverage that verifies authentication and request dispatch only occur for matching path case. Validation: - `cd python && uv run --python 3.12 pytest -q tests/unit/connectors/openapi_plugin/test_server_url_validator.py tests/unit/connectors/openapi_plugin/test_openapi_runner.py tests/unit/connectors/openapi_plugin/test_sk_openapi.py` - passed. - `cd python && uv run --python 3.12 pre-commit run -a` - passed. - `cd python && uv run --python 3.12 pytest tests/unit` - not completed; collection stopped because the local environment does not include optional connector dependencies. ### Contribution Checklist <!-- Before submitting this PR, please make sure: --> - [x] The code builds clean without any errors or warnings - [x] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations - [ ] All unit tests pass, and I have added new tests where possible - [x] I didn't break anyone 😄
1 parent 96c9e86 commit fe6c483

3 files changed

Lines changed: 76 additions & 2 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ def _matches_path_prefix(url_path: str, base_path: str) -> bool:
117117
url_path = url_path or "/"
118118
base_path = base_path or "/"
119119

120-
if url_path.lower() == base_path.lower():
120+
if url_path == base_path:
121121
return True
122122

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

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
import pytest
77

88
from semantic_kernel.connectors.openapi_plugin.models.rest_api_operation import RestApiOperation
9+
from semantic_kernel.connectors.openapi_plugin.models.rest_api_parameter import RestApiParameter
10+
from semantic_kernel.connectors.openapi_plugin.models.rest_api_parameter_location import (
11+
RestApiParameterLocation,
12+
)
913
from semantic_kernel.connectors.openapi_plugin.models.rest_api_payload import RestApiPayload
1014
from semantic_kernel.connectors.openapi_plugin.openapi_manager import OpenApiRunner
1115
from semantic_kernel.connectors.openapi_plugin.server_url_validator import ServerUrlValidationOptions
@@ -328,6 +332,61 @@ async def mock_request(*args, **kwargs):
328332
assert result == "response text"
329333

330334

335+
@pytest.mark.parametrize(
336+
("method", "route", "is_allowed"),
337+
[
338+
("GET", "safe", True),
339+
("POST", "safe", True),
340+
("GET", "SAFE", False),
341+
("POST", "SAFE", False),
342+
],
343+
)
344+
async def test_run_operation_enforces_allowed_base_url_path_case_before_authentication(method, route, is_allowed):
345+
runner = OpenApiRunner(
346+
{},
347+
server_url_validation_options=ServerUrlValidationOptions(
348+
allowed_base_urls=["https://api.example.com/api/safe"]
349+
),
350+
)
351+
operation = RestApiOperation(
352+
id="caseSensitiveOperation",
353+
method=method,
354+
servers=[{"url": "https://api.example.com"}],
355+
path="/api/{route}",
356+
params=[
357+
RestApiParameter(
358+
name="route",
359+
type="string",
360+
location=RestApiParameterLocation.PATH,
361+
is_required=True,
362+
)
363+
],
364+
)
365+
response = MagicMock()
366+
response.text = "response text"
367+
runner.http_client = AsyncMock()
368+
runner.http_client.headers = {}
369+
runner.http_client.request = AsyncMock(return_value=response)
370+
runner.auth_callback = AsyncMock(return_value={"Authorization": "******"})
371+
372+
if is_allowed:
373+
result = await runner.run_operation(operation, {"route": route})
374+
375+
assert result == "response text"
376+
runner.auth_callback.assert_awaited_once_with()
377+
runner.http_client.request.assert_awaited_once()
378+
request = runner.http_client.request.await_args.kwargs
379+
assert request["method"] == method
380+
assert request["url"] == "https://api.example.com/api/safe"
381+
assert request["headers"]["Authorization"] == "******"
382+
else:
383+
with pytest.raises(FunctionExecutionException, match="allowed base URLs"):
384+
await runner.run_operation(operation, {"route": route})
385+
386+
runner.auth_callback.assert_not_awaited()
387+
runner.http_client.request.assert_not_awaited()
388+
389+
331390
async def test_run_operation_blocks_disallowed_url_before_request():
332391
runner = OpenApiRunner({})
333392
operation = MagicMock()

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,21 @@ async def test_validate_server_url_rejects_when_allowed_base_urls_do_not_match()
132132
await validate_server_url("https://api.example.com/v2/orders", options)
133133

134134

135+
@pytest.mark.parametrize("path", ["/api/safe", "/api/safe/orders"])
136+
async def test_validate_server_url_allows_matching_allowed_base_url_path_case(path):
137+
options = ServerUrlValidationOptions(allowed_base_urls=["https://API.EXAMPLE.COM:443/api/safe"])
138+
139+
await validate_server_url(f"https://api.example.com{path}", options)
140+
141+
142+
@pytest.mark.parametrize("path", ["/api/SAFE", "/api/SAFE/orders", "/api/private", "/api/safeX"])
143+
async def test_validate_server_url_rejects_non_matching_allowed_base_url_path(path):
144+
options = ServerUrlValidationOptions(allowed_base_urls=["https://api.example.com/api/safe"])
145+
146+
with pytest.raises(FunctionExecutionException, match="allowed base URLs"):
147+
await validate_server_url(f"https://api.example.com{path}", options)
148+
149+
135150
@pytest.mark.parametrize("url", ["https://10.0.0.5/", "https://[fec0::1]/"])
136151
async def test_validate_server_url_allows_private_network_access_after_scheme_gate(url):
137152
options = ServerUrlValidationOptions(allow_private_network_access=True)

0 commit comments

Comments
 (0)