mirror of
https://github.com/microsoft/semantic-kernel.git
synced 2026-10-12 02:19:06 +00:00
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 😄
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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"]:
|
||||
|
||||
Reference in New Issue
Block a user