Skip to content

Commit d523438

Browse files
Python: Clarify HttpPlugin defaults (#14495)
### Motivation and Context Preserve established behavior while making the default configuration more visible to users. ### Description - Preserve established default behavior. - Add a targeted configuration warning. - Keep explicit validation behavior unchanged and add focused tests. ### Contribution Checklist - [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 - [x] All unit tests pass, and I have added new tests where possible - [x] I didn't break anyone 😄
1 parent c096a5e commit d523438

2 files changed

Lines changed: 39 additions & 5 deletions

File tree

‎python/semantic_kernel/core_plugins/http_plugin.py‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Copyright (c) Microsoft. All rights reserved.
22

33
import json
4+
import logging
45
from typing import Annotated, Any, ClassVar
56
from urllib.parse import urlparse
67

@@ -10,6 +11,8 @@
1011
from semantic_kernel.functions.kernel_function_decorator import kernel_function
1112
from semantic_kernel.kernel_pydantic import KernelBaseModel
1213

14+
logger = logging.getLogger(__name__)
15+
1316

1417
class HttpPlugin(KernelBaseModel):
1518
"""A plugin that provides HTTP functionality.
@@ -29,7 +32,8 @@ class HttpPlugin(KernelBaseModel):
2932
3033
Security:
3134
- By default, all requests are blocked unless ``allowed_domains`` is provided
32-
or ``allow_all_domains`` is set to True.
35+
or ``allow_all_domains`` is set to True. A warning is logged when neither
36+
option is configured.
3337
- When ``allowed_domains`` is set and ``allow_all_domains`` is False, HTTP
3438
redirects are disabled to prevent redirect-based domain bypass (SSRF).
3539
- When ``allow_all_domains`` is True, redirects are allowed regardless of
@@ -57,6 +61,16 @@ class HttpPlugin(KernelBaseModel):
5761
_DEFAULT_SCHEME_PORTS: ClassVar[dict[str, int]] = {"http": 80, "https": 443}
5862
_DEFAULT_ALLOWED_PORTS: ClassVar[frozenset[int]] = frozenset({80, 443})
5963

64+
def model_post_init(self, __context: Any) -> None:
65+
"""Warn when the default configuration blocks all requests."""
66+
super().model_post_init(__context)
67+
if self.allowed_domains is None and not self.allow_all_domains:
68+
logger.warning(
69+
"HttpPlugin was created without `allowed_domains` and with `allow_all_domains=False`; "
70+
"all HTTP requests will be blocked. Set `allowed_domains` or `allow_all_domains=True` "
71+
"to enable requests."
72+
)
73+
6074
@property
6175
def _allow_redirects(self) -> bool:
6276
"""Whether HTTP redirects should be followed.

‎python/tests/unit/core_plugins/test_http_plugin.py‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# Copyright (c) Microsoft. All rights reserved.
22

3+
import logging
34
from unittest.mock import patch
45

56
import pytest
@@ -253,16 +254,35 @@ async def test_allowed_domains_exact_subdomain_match():
253254
# Security regression tests
254255

255256

256-
async def test_default_constructor_denies_all():
257-
"""Test that default HttpPlugin() denies all requests (issue 115285)."""
258-
plugin = HttpPlugin()
257+
def test_default_constructor_denies_all_and_logs_warning(caplog):
258+
"""Test that default HttpPlugin() denies all requests and logs a warning."""
259+
with caplog.at_level(logging.WARNING, logger="semantic_kernel.core_plugins.http_plugin"):
260+
plugin = HttpPlugin()
261+
259262
assert plugin._is_uri_allowed("https://example.com/path") is False
260263
assert plugin._is_uri_allowed("https://any-domain.com/path") is False
264+
assert "without `allowed_domains`" in caplog.text
265+
assert "all HTTP requests will be blocked" in caplog.text
266+
267+
268+
@pytest.mark.parametrize(
269+
"plugin_kwargs",
270+
[
271+
{"allowed_domains": {"example.com"}},
272+
{"allow_all_domains": True},
273+
],
274+
)
275+
def test_explicit_domain_configuration_does_not_log_default_warning(caplog, plugin_kwargs):
276+
"""Test that explicit domain configurations do not log the default warning."""
277+
with caplog.at_level(logging.WARNING, logger="semantic_kernel.core_plugins.http_plugin"):
278+
HttpPlugin(**plugin_kwargs)
279+
280+
assert "without `allowed_domains`" not in caplog.text
261281

262282

263283
@pytest.mark.parametrize("method", ["get", "post", "put", "delete"])
264284
async def test_default_constructor_blocks_requests(method):
265-
"""Test that default HttpPlugin() blocks all HTTP methods (issue 115285)."""
285+
"""Test that default HttpPlugin() blocks all HTTP methods."""
266286
plugin = HttpPlugin()
267287
with pytest.raises(FunctionExecutionException, match="Sending requests to the provided location is not allowed"):
268288
if method in ["post", "put"]:

0 commit comments

Comments
 (0)