diff --git a/python/semantic_kernel/core_plugins/http_plugin.py b/python/semantic_kernel/core_plugins/http_plugin.py index c461deccbf..21759fbdf8 100644 --- a/python/semantic_kernel/core_plugins/http_plugin.py +++ b/python/semantic_kernel/core_plugins/http_plugin.py @@ -1,6 +1,7 @@ # Copyright (c) Microsoft. All rights reserved. import json +import logging from typing import Annotated, Any, ClassVar from urllib.parse import urlparse @@ -10,6 +11,8 @@ from semantic_kernel.exceptions import FunctionExecutionException from semantic_kernel.functions.kernel_function_decorator import kernel_function from semantic_kernel.kernel_pydantic import KernelBaseModel +logger = logging.getLogger(__name__) + class HttpPlugin(KernelBaseModel): """A plugin that provides HTTP functionality. @@ -29,7 +32,8 @@ class HttpPlugin(KernelBaseModel): Security: - By default, all requests are blocked unless ``allowed_domains`` is provided - or ``allow_all_domains`` is set to True. + or ``allow_all_domains`` is set to True. A warning is logged when neither + option is configured. - When ``allowed_domains`` is set and ``allow_all_domains`` is False, HTTP redirects are disabled to prevent redirect-based domain bypass (SSRF). - When ``allow_all_domains`` is True, redirects are allowed regardless of @@ -57,6 +61,16 @@ class HttpPlugin(KernelBaseModel): _DEFAULT_SCHEME_PORTS: ClassVar[dict[str, int]] = {"http": 80, "https": 443} _DEFAULT_ALLOWED_PORTS: ClassVar[frozenset[int]] = frozenset({80, 443}) + def model_post_init(self, __context: Any) -> None: + """Warn when the default configuration blocks all requests.""" + super().model_post_init(__context) + if self.allowed_domains is None and not self.allow_all_domains: + logger.warning( + "HttpPlugin was created without `allowed_domains` and with `allow_all_domains=False`; " + "all HTTP requests will be blocked. Set `allowed_domains` or `allow_all_domains=True` " + "to enable requests." + ) + @property def _allow_redirects(self) -> bool: """Whether HTTP redirects should be followed. diff --git a/python/tests/unit/core_plugins/test_http_plugin.py b/python/tests/unit/core_plugins/test_http_plugin.py index c0c8211b1d..a153d1755a 100644 --- a/python/tests/unit/core_plugins/test_http_plugin.py +++ b/python/tests/unit/core_plugins/test_http_plugin.py @@ -1,5 +1,6 @@ # Copyright (c) Microsoft. All rights reserved. +import logging from unittest.mock import patch import pytest @@ -253,16 +254,35 @@ async def test_allowed_domains_exact_subdomain_match(): # Security regression tests -async def test_default_constructor_denies_all(): - """Test that default HttpPlugin() denies all requests (issue 115285).""" - plugin = HttpPlugin() +def test_default_constructor_denies_all_and_logs_warning(caplog): + """Test that default HttpPlugin() denies all requests and logs a warning.""" + with caplog.at_level(logging.WARNING, logger="semantic_kernel.core_plugins.http_plugin"): + plugin = HttpPlugin() + assert plugin._is_uri_allowed("https://example.com/path") is False assert plugin._is_uri_allowed("https://any-domain.com/path") is False + assert "without `allowed_domains`" in caplog.text + assert "all HTTP requests will be blocked" in caplog.text + + +@pytest.mark.parametrize( + "plugin_kwargs", + [ + {"allowed_domains": {"example.com"}}, + {"allow_all_domains": True}, + ], +) +def test_explicit_domain_configuration_does_not_log_default_warning(caplog, plugin_kwargs): + """Test that explicit domain configurations do not log the default warning.""" + with caplog.at_level(logging.WARNING, logger="semantic_kernel.core_plugins.http_plugin"): + HttpPlugin(**plugin_kwargs) + + assert "without `allowed_domains`" not in caplog.text @pytest.mark.parametrize("method", ["get", "post", "put", "delete"]) async def test_default_constructor_blocks_requests(method): - """Test that default HttpPlugin() blocks all HTTP methods (issue 115285).""" + """Test that default HttpPlugin() blocks all HTTP methods.""" plugin = HttpPlugin() with pytest.raises(FunctionExecutionException, match="Sending requests to the provided location is not allowed"): if method in ["post", "put"]: