Skip to content

Commit 32abf99

Browse files
Python: Harden HttpPlugin request validation (#13969)
Improve input validation and request handling in the Python HttpPlugin. ### Changes - Add explicit opt-in for unrestricted domain access - Tighten URL validation logic - Improve redirect handling when domain restrictions are configured - Add regression tests --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a7cea9a commit 32abf99

2 files changed

Lines changed: 251 additions & 30 deletions

File tree

python/semantic_kernel/core_plugins/http_plugin.py

Lines changed: 63 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,49 +15,85 @@ class HttpPlugin(KernelBaseModel):
1515
"""A plugin that provides HTTP functionality.
1616
1717
Usage:
18-
kernel.add_plugin(HttpPlugin(), "http")
19-
20-
# With allowed domains for security:
18+
# With allowed domains (recommended):
2119
kernel.add_plugin(HttpPlugin(allowed_domains=["example.com", "api.example.com"]), "http")
2220
21+
# Explicitly allow all domains (opt-in, less secure):
22+
kernel.add_plugin(HttpPlugin(allow_all_domains=True), "http")
23+
2324
Examples:
2425
{{http.getAsync $url}}
2526
{{http.postAsync $url}}
2627
{{http.putAsync $url}}
2728
{{http.deleteAsync $url}}
29+
30+
Security:
31+
- By default, all requests are blocked unless ``allowed_domains`` is provided
32+
or ``allow_all_domains`` is set to True.
33+
- When ``allowed_domains`` is set and ``allow_all_domains`` is False, HTTP
34+
redirects are disabled to prevent redirect-based domain bypass (SSRF).
35+
- When ``allow_all_domains`` is True, redirects are allowed regardless of
36+
whether ``allowed_domains`` is also set.
37+
- Only ``http`` and ``https`` URL schemes are permitted.
2838
"""
2939

3040
allowed_domains: set[str] | None = None
31-
"""List of allowed domains to send requests to. If None, all domains are allowed."""
41+
"""Set of allowed domains to send requests to."""
42+
43+
allow_all_domains: bool = False
44+
"""When True, requests to any domain are allowed. Must be explicitly set."""
45+
46+
_ALLOWED_SCHEMES: frozenset[str] = frozenset({"http", "https"})
47+
48+
@property
49+
def _allow_redirects(self) -> bool:
50+
"""Whether HTTP redirects should be followed.
51+
52+
Redirects are only allowed when ``allow_all_domains`` is True.
53+
When domain restrictions are configured, redirects are disabled
54+
to prevent redirect-based SSRF bypass.
55+
"""
56+
return self.allow_all_domains
3257

3358
def _is_uri_allowed(self, url: str) -> bool:
34-
"""Check if the URL's host is in the allowed domains list.
59+
"""Check if the URL's host and scheme are permitted.
3560
3661
Args:
3762
url: The URL to check.
3863
3964
Returns:
4065
True if the URL is allowed, False otherwise.
4166
"""
42-
if self.allowed_domains is None:
43-
return True
44-
4567
parsed = urlparse(url)
68+
69+
# Validate scheme
70+
if parsed.scheme.lower() not in self._ALLOWED_SCHEMES:
71+
return False
72+
4673
host = parsed.hostname
47-
if host is None:
74+
if not host:
4875
return False
4976

50-
# Case-insensitive comparison
51-
return host.lower() in {domain.lower() for domain in self.allowed_domains}
77+
# If allow_all_domains is set, skip domain check
78+
if self.allow_all_domains:
79+
return True
80+
81+
# If allowed_domains is set, check against it
82+
if self.allowed_domains is not None:
83+
return host.lower() in {domain.lower() for domain in self.allowed_domains}
84+
85+
# Default: deny all
86+
return False
5287

5388
def _validate_url(self, url: str) -> None:
54-
"""Validate the URL, checking if it's not empty and is in the allowed domains.
89+
"""Validate the URL, checking scheme, emptiness, and allowed domains.
5590
5691
Args:
5792
url: The URL to validate.
5893
5994
Raises:
60-
FunctionExecutionException: If the URL is empty or not in the allowed domains.
95+
FunctionExecutionException: If the URL is empty, uses a disallowed scheme,
96+
or targets a domain that is not allowed.
6197
"""
6298
if not url:
6399
raise FunctionExecutionException("url cannot be `None` or empty")
@@ -77,7 +113,10 @@ async def get(self, url: Annotated[str, "The URL to send the request to."]) -> s
77113
"""
78114
self._validate_url(url)
79115

80-
async with aiohttp.ClientSession() as session, session.get(url, raise_for_status=True) as response:
116+
async with (
117+
aiohttp.ClientSession() as session,
118+
session.get(url, raise_for_status=True, allow_redirects=self._allow_redirects) as response,
119+
):
81120
return await response.text()
82121

83122
@kernel_function(description="Makes a POST request to a uri", name="postAsync")
@@ -100,7 +139,9 @@ async def post(
100139
data = json.dumps(body) if body is not None else None
101140
async with (
102141
aiohttp.ClientSession() as session,
103-
session.post(url, headers=headers, data=data, raise_for_status=True) as response,
142+
session.post(
143+
url, headers=headers, data=data, raise_for_status=True, allow_redirects=self._allow_redirects
144+
) as response,
104145
):
105146
return await response.text()
106147

@@ -125,7 +166,9 @@ async def put(
125166
data = json.dumps(body) if body is not None else None
126167
async with (
127168
aiohttp.ClientSession() as session,
128-
session.put(url, headers=headers, data=data, raise_for_status=True) as response,
169+
session.put(
170+
url, headers=headers, data=data, raise_for_status=True, allow_redirects=self._allow_redirects
171+
) as response,
129172
):
130173
return await response.text()
131174

@@ -141,5 +184,8 @@ async def delete(self, url: Annotated[str, "The URI to send the request to."]) -
141184
"""
142185
self._validate_url(url)
143186

144-
async with aiohttp.ClientSession() as session, session.delete(url, raise_for_status=True) as response:
187+
async with (
188+
aiohttp.ClientSession() as session,
189+
session.delete(url, raise_for_status=True, allow_redirects=self._allow_redirects) as response,
190+
):
145191
return await response.text()

0 commit comments

Comments
 (0)