From d89b6e4775598c62921654fbbec7cbc3ac0ca3ce Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Fri, 2 Oct 2026 10:19:19 -0700 Subject: [PATCH 01/15] First commit --- .../hosting/core/app/attachment_downloader.py | 10 ++- .../core/app/m365_attachment_downloader.py | 10 ++- .../hosting/core/security/__init__.py | 7 ++ .../hosting/core/security/_dns_cache.py | 0 .../hosting/core/security/_utils.py | 21 ++++++ .../hosting/core/security/middleware.py | 23 +++++++ .../old.py} | 65 +++++++++++++++++-- .../core/security/outbound_host_validator.py | 28 ++++++++ 8 files changed, 155 insertions(+), 9 deletions(-) create mode 100644 libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py create mode 100644 libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_dns_cache.py create mode 100644 libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py create mode 100644 libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/middleware.py rename libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/{outbound_host_validator.py => security/old.py} (60%) create mode 100644 libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py index 7f6a71965..8d0638197 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py @@ -13,7 +13,10 @@ ) from microsoft_agents.hosting.core.turn_context import TurnContext -from microsoft_agents.hosting.core.outbound_host_validator import OutboundHostValidator +from microsoft_agents.hosting.core.outbound_host_validator import ( + OutboundHostValidator, + _validator_middleware, +) from .input_file import InputFileDownloader, InputFile from ._utils import _parse_content_type, _basic_url_check @@ -73,7 +76,10 @@ async def _download_file(self, attachment: Attachment) -> InputFile | None: return None async with self._client_factory() as client: - async with client.get(remote_file_url) as response: + async with client.get( + remote_file_url, + middlewares=[_validator_middleware(self._host_validator)] + ) as response: if not (200 <= response.status < 300): return None diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py index 736f4d577..988cdec14 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py @@ -9,7 +9,6 @@ from microsoft_agents.activity import ( Attachment, Channels, - ChannelId, ) from microsoft_agents.hosting.core.authorization import ( @@ -17,7 +16,10 @@ Connections, ) from microsoft_agents.hosting.core.turn_context import TurnContext -from microsoft_agents.hosting.core.outbound_host_validator import OutboundHostValidator +from microsoft_agents.hosting.core.outbound_host_validator import ( + OutboundHostValidator, + _validator_middleware, +) from .input_file import InputFileDownloader, InputFile from ._utils import _parse_content_type, _basic_url_check @@ -145,7 +147,9 @@ async def _download_file( async with self._client_factory() as client: async with client.get( - download_url, headers={"Authorization": f"Bearer {access_token}"} + download_url, + headers={"Authorization": f"Bearer {access_token}"}, + middlewares=[_validator_middleware(self._host_validator)] ) as response: if not (200 <= response.status < 300): return None diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py new file mode 100644 index 000000000..06e8e63cf --- /dev/null +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py @@ -0,0 +1,7 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +from .middleware import _validator_middleware +from .outbound_host_validator import _OutboundHostValidator + +__all__ = ["_validator_middleware", "_OutboundHostValidator"] \ No newline at end of file diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_dns_cache.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_dns_cache.py new file mode 100644 index 000000000..e69de29bb diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py new file mode 100644 index 000000000..e51822230 --- /dev/null +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py @@ -0,0 +1,21 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +from collections import OrderedDict +from yarl import URL + +def _try_create_url(url: str | URL) -> URL | None: + """Attempts to create a URL object from the given string or URL. + + :param url: The URL string or URL object to create. + :return: A URL object if successful, None otherwise. + """ + try: + return URL(url) if isinstance(url, str) else url + except (ValueError, TypeError): + return None + +class _DNSCache: + + def __init__(self, max_size: int, ttl: float) -> None: + self._cache = OrderedDict[tuple[str, int]] \ No newline at end of file diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/middleware.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/middleware.py new file mode 100644 index 000000000..5bffb8fb2 --- /dev/null +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/middleware.py @@ -0,0 +1,23 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +from typing import Callable, Awaitable + +from aiohttp import ClientRequest, ClientHandlerType, ClientResponse + +from .outbound_host_validator import _OutboundHostValidator + +def _validator_middleware(validator: _OutboundHostValidator) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: + """Creates a middleware that validates outbound URLs using the given validator. + + :param validator: The outbound host validator to use. + :return: A middleware function that validates outbound URLs. + """ + async def _middleware(req: ClientRequest, handler: ClientHandlerType) -> ClientResponse: + + if validator.enabled and not validator.is_allowed(req.url): + raise ValueError(f"URL '{req.url}' is not allowed by the outbound host validator.") + + return await handler(req) + + return _middleware \ No newline at end of file diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/outbound_host_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py similarity index 60% rename from libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/outbound_host_validator.py rename to libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py index ea58f10e3..1066f54e4 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/outbound_host_validator.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py @@ -1,7 +1,16 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +from typing import Callable, Awaitable, Protocol + +from aiohttp import ( + ClientSession, + ClientRequest, + ClientHandlerType, + ClientResponse, +) from yarl import URL +from urllib.parse import urlparse _DEFAULT_MICROSOFT_HOSTS = [ "botframework.com", # Bot Connector / channel services URLs @@ -55,6 +64,34 @@ def _normalize(host: str) -> str | None: return host if host else None +class OutboundHostValidatorProtocol(Protocol): + + @property + def enabled(self) -> bool: + ... + + def is_allowed(self, url: str | URL) -> bool: + ... + +class BasicOutboundHostValidator(OutboundHostValidatorProtocol): + def __init__(self, enabled: bool = False): + self._enabled = enabled + + @property + def enabled(self) -> bool: + return self._enabled + + def is_allowed(self, url: str | URL) -> bool: + """Checks whether the given URL is allowed by the basic validator. + + :param url: The URL to check. + :return: True if the URL is allowed, False otherwise. + """ + parsed = urlparse(str(url)) + return parsed.scheme == "https" or ( + parsed.scheme == "http" and parsed.hostname == "localhost" + ) + class OutboundHostValidator: """Validates that an outbound URL targets an allowed host before the SDK makes a @@ -70,6 +107,12 @@ def __init__( hosts: list[str] | None = None, include_default_microsoft_hosts: bool = True, ): + """Initializes the outbound host validator. + + :param enabled: Whether the validator is enabled. + :param hosts: A list of additional allowed host suffixes. + :param include_default_microsoft_hosts: Whether to include the default Microsoft hosts. + """ self._enabled = enabled suffixes: list[str] = [] if include_default_microsoft_hosts: @@ -98,10 +141,7 @@ def is_allowed(self, url: str | URL) -> bool: return True url_obj = _try_create_url(url) - if not url_obj: - return False - - if not url_obj.absolute: + if not url_obj or not url_obj.absolute or url_obj.scheme != "https": return False host = url_obj.host @@ -115,3 +155,20 @@ def is_allowed(self, url: str | URL) -> bool: return True return False + +def _validator_middleware(validator: OutboundHostValidatorProtocol | None = None) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: + """Creates a middleware that validates outbound URLs using the given validator. + + :param validator: The outbound host validator to use. + :return: A middleware function that validates outbound URLs. + """ + validator = validator or OutboundHostValidator(enabled=True) + + async def _middleware(req: ClientRequest, handler: ClientHandlerType) -> ClientResponse: + + if validator is not None and validator.enabled and not validator.is_allowed(req.url): + raise ValueError(f"URL '{req.url}' is not allowed by the outbound host validator.") + + return await handler(req) + + return _middleware \ No newline at end of file diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py new file mode 100644 index 000000000..3363ba9eb --- /dev/null +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py @@ -0,0 +1,28 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +from typing import Protocol, runtime_checkable +from yarl import URL + +@runtime_checkable +class _OutboundHostValidator(Protocol): + + @property + def enabled(self) -> bool: + ... + + def is_allowed(self, url: str | URL) -> bool: + ... + +class OutboundHostValidator(_OutboundHostValidator): + + def __init__(self, enabled: bool): + self._enabled = enabled + + @property + def enabled(self) -> bool: + return self._enabled + + def is_allowed(self, url: str | URL) -> bool: + # Implement your logic to check if the URL is allowed + return True \ No newline at end of file From 6e7b2c5636ff5027d70bfe3814ef5bbbda2ec020 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Fri, 2 Oct 2026 13:55:41 -0700 Subject: [PATCH 02/15] Splitting into connector and url validator --- .../hosting/core/security/_connector.py | 46 ++++++++++++++++ .../hosting/core/security/_dns_cache.py | 0 .../hosting/core/security/_utils.py | 42 +++++++++++++-- .../hosting/core/security/host_validator.py | 54 +++++++++++++++++++ .../hosting/core/security/middleware.py | 7 +-- 5 files changed, 142 insertions(+), 7 deletions(-) create mode 100644 libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py delete mode 100644 libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_dns_cache.py create mode 100644 libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/host_validator.py diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py new file mode 100644 index 000000000..356185935 --- /dev/null +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py @@ -0,0 +1,46 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +from collections.abc import Sequence +from typing import Protocol + +from aiohttp import TCPConnector +from aiohttp.abc import ResolveResult +from aiohttp.tracing import Trace + +class _ResolvedResultValidator(Protocol): + """Protocol for validating resolved results to prevent SSRF attacks.""" + + def is_valid(self, resolved_result: ResolveResult) -> bool: + """Check if the resolved result is valid to prevent SSRF attacks. + + :param resolved_result: The resolved result to validate. + :return: True if the resolved result is valid, False otherwise. + """ + ... + +class _SSRFError(ValueError): + """Exception raised when an SSRF attack is detected.""" + +class _SSRFConnector(TCPConnector): + + def __init__(self, resolved_result_validator: _ResolvedResultValidator, *args, **kwargs): + super().__init__(*args, **kwargs) + self._resolved_result_validator = resolved_result_validator + + async def _resolve_host(self, host: str, port: int, traces: Sequence[Trace] | None = None) -> list[ResolveResult]: + """Resolve the host and validate the resolved results to prevent SSRF attacks. + + :param host: The hostname to resolve. + :param port: The port number to resolve. + :param traces: Optional sequence of Trace objects for tracing the resolution process. + :return: A list of validated ResolveResult objects. + :raises _SSRFError: If any resolved result is invalid. + """ + + res = await super()._resolve_host(host, port, traces) + + for r in res: + if not self._resolved_result_validator.is_valid(r): + raise _SSRFError(f"Invalid resolved result: {r}") + return res \ No newline at end of file diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_dns_cache.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_dns_cache.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py index e51822230..2dca80437 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py @@ -1,7 +1,13 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -from collections import OrderedDict +from __future__ import annotations + +from aiohttp import TCPConnector +from aiohttp.abc import ResolveResult + +import ipaddress + from yarl import URL def _try_create_url(url: str | URL) -> URL | None: @@ -15,7 +21,35 @@ def _try_create_url(url: str | URL) -> URL | None: except (ValueError, TypeError): return None -class _DNSCache: +class _SSRFConnector(TCPConnector): + + def __init__(self, host_validator: HostValidator, *args, **kwargs): + self._host_validator = host_validator + super().__init__(*args, **kwargs) + + + async def _resolve_host(self, host: str, port: int, traces: Sequence[Trace] | None = None) -> list[ResolveResult]: + + res = await super()._resolve_host(host, port, traces) + + for r in res: + + + return res + +class OutboundHostValidator: + + def __init__(self, enabled): + + self._connector = _SSRFConnector() + + def is_allowed(self, url: str | URL) -> bool: + """Checks if the given URL is allowed based on the host validator. + + :param url: The URL string or URL object to check. + :return: True if the URL is allowed, False otherwise. + """ + return self._connector._host_validator.is_allowed(url) # type: ignore - def __init__(self, max_size: int, ttl: float) -> None: - self._cache = OrderedDict[tuple[str, int]] \ No newline at end of file +async def _resolve_host(connector: TCPConnector) -> list[ResolveResult]: + return await connector._resolve_host() # type: ignore \ No newline at end of file diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/host_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/host_validator.py new file mode 100644 index 000000000..e3e4a2902 --- /dev/null +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/host_validator.py @@ -0,0 +1,54 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +import ipaddress +import logging + +from typing import Protocol +from yarl import URL + +logger = logging.getLogger(__name__) + +class _URLValidator(Protocol): + + def is_allowed(self, url: str | URL) -> bool: + ... + +class OutboundHostValidator(_URLValidator): + + def __init__(self, enabled: bool = False): + self._enabled = enabled + + @property + def enabled(self) -> bool: + return self._enabled + + def is_allowed(self, url: str | URL) -> bool: + if not self._enabled: + return True + + url_str: str + if isinstance(url, str): + url_str = url + try: + url = URL(url) + except ValueError: + logger.warning("Invalid URL: %s", url) + return False + else: + url_str = str(url) + + if url.scheme not in ("http", "https"): + return False + + if url.user or url.password: + return False + + if url.absolute: + + + + + if url.userinfo: + + host = url.host if isinstance(url, URL) else URL(url).host \ No newline at end of file diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/middleware.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/middleware.py index 5bffb8fb2..fc4f8e0a1 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/middleware.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/middleware.py @@ -5,17 +5,18 @@ from aiohttp import ClientRequest, ClientHandlerType, ClientResponse -from .outbound_host_validator import _OutboundHostValidator +from .host_validator import _HostValidator -def _validator_middleware(validator: _OutboundHostValidator) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: +def _validator_middleware(validator: _HostValidator) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: """Creates a middleware that validates outbound URLs using the given validator. :param validator: The outbound host validator to use. :return: A middleware function that validates outbound URLs. """ async def _middleware(req: ClientRequest, handler: ClientHandlerType) -> ClientResponse: + """Middleware that validates outbound URLs using the given host validator.""" - if validator.enabled and not validator.is_allowed(req.url): + if not validator.is_allowed(req.url): raise ValueError(f"URL '{req.url}' is not allowed by the outbound host validator.") return await handler(req) From d9c1278960ba31daca1b74ae79285f5c5fdef827 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Fri, 2 Oct 2026 15:01:08 -0700 Subject: [PATCH 03/15] Implementation of new OutboundHostValidator --- .../hosting/core/security/__init__.py | 7 +- .../hosting/core/security/_connector.py | 10 +- .../{middleware.py => _middleware.py} | 4 +- .../hosting/core/security/_utils.py | 47 ++- .../hosting/core/security/_validator.py | 33 ++ .../hosting/core/security/host_validator.py | 54 --- .../hosting/core/security/old.py | 328 +++++++++--------- .../core/security/outbound_host_validator.py | 163 ++++++++- 8 files changed, 380 insertions(+), 266 deletions(-) rename libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/{middleware.py => _middleware.py} (82%) create mode 100644 libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py delete mode 100644 libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/host_validator.py diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py index 06e8e63cf..82b079b8e 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py @@ -1,7 +1,8 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -from .middleware import _validator_middleware -from .outbound_host_validator import _OutboundHostValidator +from .outbound_host_validator import OutboundHostValidator -__all__ = ["_validator_middleware", "_OutboundHostValidator"] \ No newline at end of file +__all__ = [ + "OutboundHostValidator" +] \ No newline at end of file diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py index 356185935..98d5890ae 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py @@ -8,13 +8,13 @@ from aiohttp.abc import ResolveResult from aiohttp.tracing import Trace -class _ResolvedResultValidator(Protocol): +class _ResolveResultValidator(Protocol): """Protocol for validating resolved results to prevent SSRF attacks.""" - def is_valid(self, resolved_result: ResolveResult) -> bool: + def is_allowed(self, resolved: ResolveResult) -> bool: """Check if the resolved result is valid to prevent SSRF attacks. - :param resolved_result: The resolved result to validate. + :param resolved: The resolved result to validate. :return: True if the resolved result is valid, False otherwise. """ ... @@ -24,7 +24,7 @@ class _SSRFError(ValueError): class _SSRFConnector(TCPConnector): - def __init__(self, resolved_result_validator: _ResolvedResultValidator, *args, **kwargs): + def __init__(self, resolved_result_validator: _ResolveResultValidator, *args, **kwargs): super().__init__(*args, **kwargs) self._resolved_result_validator = resolved_result_validator @@ -41,6 +41,6 @@ async def _resolve_host(self, host: str, port: int, traces: Sequence[Trace] | No res = await super()._resolve_host(host, port, traces) for r in res: - if not self._resolved_result_validator.is_valid(r): + if not self._resolved_result_validator.is_allowed(r): raise _SSRFError(f"Invalid resolved result: {r}") return res \ No newline at end of file diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/middleware.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py similarity index 82% rename from libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/middleware.py rename to libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py index fc4f8e0a1..c2deb4e6f 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/middleware.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py @@ -5,9 +5,9 @@ from aiohttp import ClientRequest, ClientHandlerType, ClientResponse -from .host_validator import _HostValidator +from ._validator import _URLValidator -def _validator_middleware(validator: _HostValidator) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: +def _validator_middleware(validator: _URLValidator) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: """Creates a middleware that validates outbound URLs using the given validator. :param validator: The outbound host validator to use. diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py index 2dca80437..d94e054a1 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py @@ -21,35 +21,30 @@ def _try_create_url(url: str | URL) -> URL | None: except (ValueError, TypeError): return None -class _SSRFConnector(TCPConnector): +def _normalize_host(host: str) -> str | None: + """Normalizes the given host string. - def __init__(self, host_validator: HostValidator, *args, **kwargs): - self._host_validator = host_validator - super().__init__(*args, **kwargs) - - - async def _resolve_host(self, host: str, port: int, traces: Sequence[Trace] | None = None) -> list[ResolveResult]: - - res = await super()._resolve_host(host, port, traces) - - for r in res: - - - return res - -class OutboundHostValidator: + :param host: The host string to normalize. + :return: The normalized host string. + """ + if not host: + return None - def __init__(self, enabled): + host = host.strip().casefold() - self._connector = _SSRFConnector() + if host.startswith("*."): + host = host[2:] - def is_allowed(self, url: str | URL) -> bool: - """Checks if the given URL is allowed based on the host validator. + url_obj = _try_create_url(host) + if url_obj and url_obj.host: + host = url_obj.host + else: + slash = host.find("/") + if slash >= 0: + host = host[:slash] - :param url: The URL string or URL object to check. - :return: True if the URL is allowed, False otherwise. - """ - return self._connector._host_validator.is_allowed(url) # type: ignore + colon = host.find(":") + if colon >= 0: + host = host[:colon] -async def _resolve_host(connector: TCPConnector) -> list[ResolveResult]: - return await connector._resolve_host() # type: ignore \ No newline at end of file + return host if host else None \ No newline at end of file diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py new file mode 100644 index 000000000..bb50171b0 --- /dev/null +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py @@ -0,0 +1,33 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +import logging + +from typing import Protocol +from yarl import URL + +from aiohttp import ClientSession + +logger = logging.getLogger(__name__) + +class _URLValidator(Protocol): + """Protocol for URL validators.""" + + def is_allowed(self, url: str | URL) -> bool: + """Checks if the given URL is allowed. + + :param url: The URL to check. + :return: True if the URL is allowed, False otherwise. + """ + ... + +class _OutboundHostValidator(_URLValidator): + """Protocol for outbound host validators.""" + + def client(self, session: ClientSession | None = None) -> ClientSession: + """Returns a client session for making outbound requests. + + :param session: An optional existing ClientSession to use. + :return: A ClientSession instance. + """ + ... \ No newline at end of file diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/host_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/host_validator.py deleted file mode 100644 index e3e4a2902..000000000 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/host_validator.py +++ /dev/null @@ -1,54 +0,0 @@ -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. - -import ipaddress -import logging - -from typing import Protocol -from yarl import URL - -logger = logging.getLogger(__name__) - -class _URLValidator(Protocol): - - def is_allowed(self, url: str | URL) -> bool: - ... - -class OutboundHostValidator(_URLValidator): - - def __init__(self, enabled: bool = False): - self._enabled = enabled - - @property - def enabled(self) -> bool: - return self._enabled - - def is_allowed(self, url: str | URL) -> bool: - if not self._enabled: - return True - - url_str: str - if isinstance(url, str): - url_str = url - try: - url = URL(url) - except ValueError: - logger.warning("Invalid URL: %s", url) - return False - else: - url_str = str(url) - - if url.scheme not in ("http", "https"): - return False - - if url.user or url.password: - return False - - if url.absolute: - - - - - if url.userinfo: - - host = url.host if isinstance(url, URL) else URL(url).host \ No newline at end of file diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py index 1066f54e4..7729d6df1 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py @@ -1,174 +1,174 @@ -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. +# # Copyright (c) Microsoft Corporation. All rights reserved. +# # Licensed under the MIT License. -from typing import Callable, Awaitable, Protocol - -from aiohttp import ( - ClientSession, - ClientRequest, - ClientHandlerType, - ClientResponse, -) -from yarl import URL -from urllib.parse import urlparse - -_DEFAULT_MICROSOFT_HOSTS = [ - "botframework.com", # Bot Connector / channel services URLs - "smba.trafficmanager.net", # Teams service URLs - "teams.microsoft.com", - "teams.microsoft.us", - "graph.microsoft.com", # Microsoft Graph - "sharepoint.com", # Sharepoint / OneDrive hosted attachments - "svc.ms", # Teams attachment CDN - "blob.core.windows.net", # Azure Blob Storage / Attachment Management Service -] - - -def _try_create_url(url: str | URL) -> URL | None: - """Attempts to create a URL object from the given string or URL. - - :param url: The URL string or URL object to create. - :return: A URL object if successful, None otherwise. - """ - try: - return URL(url) if isinstance(url, str) else url - except (ValueError, TypeError): - return None - - -def _normalize(host: str) -> str | None: - """Normalizes a host string to a suffix for comparison. - - :param host: The host string to normalize. - :return: The normalized host suffix, or None if the input is invalid. - """ - if not host: - return None - - host = host.strip().casefold() - - if host.startswith("*."): - host = host[2:] - - url_obj = _try_create_url(host) - if url_obj and url_obj.host: - host = url_obj.host - else: - slash = host.find("/") - if slash >= 0: - host = host[:slash] - - colon = host.find(":") - if colon >= 0: - host = host[:colon] - - return host if host else None - -class OutboundHostValidatorProtocol(Protocol): - - @property - def enabled(self) -> bool: - ... - - def is_allowed(self, url: str | URL) -> bool: - ... - -class BasicOutboundHostValidator(OutboundHostValidatorProtocol): - def __init__(self, enabled: bool = False): - self._enabled = enabled - - @property - def enabled(self) -> bool: - return self._enabled - - def is_allowed(self, url: str | URL) -> bool: - """Checks whether the given URL is allowed by the basic validator. - - :param url: The URL to check. - :return: True if the URL is allowed, False otherwise. - """ - parsed = urlparse(str(url)) - return parsed.scheme == "https" or ( - parsed.scheme == "http" and parsed.hostname == "localhost" - ) - - -class OutboundHostValidator: - """Validates that an outbound URL targets an allowed host before the SDK makes a - server-side, often token-bearing, request to it (e.g. Activity.service_url callbacks or attachment - downloads). This is the SDK's shared anti-SSRF ("allow_hosts") control.""" - - _enabled: bool - _suffixes: set[str] - - def __init__( - self, - enabled: bool = False, - hosts: list[str] | None = None, - include_default_microsoft_hosts: bool = True, - ): - """Initializes the outbound host validator. - - :param enabled: Whether the validator is enabled. - :param hosts: A list of additional allowed host suffixes. - :param include_default_microsoft_hosts: Whether to include the default Microsoft hosts. - """ - self._enabled = enabled - suffixes: list[str] = [] - if include_default_microsoft_hosts: - suffixes.extend(_DEFAULT_MICROSOFT_HOSTS) - - if hosts: - for host in hosts: - normalized = _normalize(host) - if normalized is not None: - suffixes.append(normalized) +# from typing import Callable, Awaitable, Protocol + +# from aiohttp import ( +# ClientSession, +# ClientRequest, +# ClientHandlerType, +# ClientResponse, +# ) +# from yarl import URL +# from urllib.parse import urlparse + +# _DEFAULT_MICROSOFT_HOSTS = [ +# "botframework.com", # Bot Connector / channel services URLs +# "smba.trafficmanager.net", # Teams service URLs +# "teams.microsoft.com", +# "teams.microsoft.us", +# "graph.microsoft.com", # Microsoft Graph +# "sharepoint.com", # Sharepoint / OneDrive hosted attachments +# "svc.ms", # Teams attachment CDN +# "blob.core.windows.net", # Azure Blob Storage / Attachment Management Service +# ] + + +# def _try_create_url(url: str | URL) -> URL | None: +# """Attempts to create a URL object from the given string or URL. + +# :param url: The URL string or URL object to create. +# :return: A URL object if successful, None otherwise. +# """ +# try: +# return URL(url) if isinstance(url, str) else url +# except (ValueError, TypeError): +# return None + + +# def _normalize(host: str) -> str | None: +# """Normalizes a host string to a suffix for comparison. + +# :param host: The host string to normalize. +# :return: The normalized host suffix, or None if the input is invalid. +# """ +# if not host: +# return None + +# host = host.strip().casefold() + +# if host.startswith("*."): +# host = host[2:] + +# url_obj = _try_create_url(host) +# if url_obj and url_obj.host: +# host = url_obj.host +# else: +# slash = host.find("/") +# if slash >= 0: +# host = host[:slash] + +# colon = host.find(":") +# if colon >= 0: +# host = host[:colon] + +# return host if host else None + +# class OutboundHostValidatorProtocol(Protocol): + +# @property +# def enabled(self) -> bool: +# ... + +# def is_allowed(self, url: str | URL) -> bool: +# ... + +# class BasicOutboundHostValidator(OutboundHostValidatorProtocol): +# def __init__(self, enabled: bool = False): +# self._enabled = enabled + +# @property +# def enabled(self) -> bool: +# return self._enabled + +# def is_allowed(self, url: str | URL) -> bool: +# """Checks whether the given URL is allowed by the basic validator. + +# :param url: The URL to check. +# :return: True if the URL is allowed, False otherwise. +# """ +# parsed = urlparse(str(url)) +# return parsed.scheme == "https" or ( +# parsed.scheme == "http" and parsed.hostname == "localhost" +# ) + + +# class OutboundHostValidator: +# """Validates that an outbound URL targets an allowed host before the SDK makes a +# server-side, often token-bearing, request to it (e.g. Activity.service_url callbacks or attachment +# downloads). This is the SDK's shared anti-SSRF ("allow_hosts") control.""" + +# _enabled: bool +# _suffixes: set[str] + +# def __init__( +# self, +# enabled: bool = False, +# hosts: list[str] | None = None, +# include_default_microsoft_hosts: bool = True, +# ): +# """Initializes the outbound host validator. + +# :param enabled: Whether the validator is enabled. +# :param hosts: A list of additional allowed host suffixes. +# :param include_default_microsoft_hosts: Whether to include the default Microsoft hosts. +# """ +# self._enabled = enabled +# suffixes: list[str] = [] +# if include_default_microsoft_hosts: +# suffixes.extend(_DEFAULT_MICROSOFT_HOSTS) + +# if hosts: +# for host in hosts: +# normalized = _normalize(host) +# if normalized is not None: +# suffixes.append(normalized) - self._suffixes = set(suffixes) - - @property - def enabled(self) -> bool: - """Gets whether the validator is enabled. If disabled, all outbound hosts are allowed.""" - return self._enabled - - def is_allowed(self, url: str | URL) -> bool: - """Checks whether the given URL is allowed by the validator. +# self._suffixes = set(suffixes) + +# @property +# def enabled(self) -> bool: +# """Gets whether the validator is enabled. If disabled, all outbound hosts are allowed.""" +# return self._enabled + +# def is_allowed(self, url: str | URL) -> bool: +# """Checks whether the given URL is allowed by the validator. - :param url: The URL to check. - :return: True if the URL is allowed, False otherwise. - """ - if not self._enabled: - return True - - url_obj = _try_create_url(url) - if not url_obj or not url_obj.absolute or url_obj.scheme != "https": - return False +# :param url: The URL to check. +# :return: True if the URL is allowed, False otherwise. +# """ +# if not self._enabled: +# return True + +# url_obj = _try_create_url(url) +# if not url_obj or not url_obj.absolute or url_obj.scheme != "https": +# return False - host = url_obj.host - if not host: - return False - - host = host.casefold() - - for suffix in self._suffixes: - if host == suffix or host.endswith("." + suffix): - return True - - return False +# host = url_obj.host +# if not host: +# return False + +# host = host.casefold() + +# for suffix in self._suffixes: +# if host == suffix or host.endswith("." + suffix): +# return True + +# return False -def _validator_middleware(validator: OutboundHostValidatorProtocol | None = None) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: - """Creates a middleware that validates outbound URLs using the given validator. +# def _validator_middleware(validator: OutboundHostValidatorProtocol | None = None) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: +# """Creates a middleware that validates outbound URLs using the given validator. - :param validator: The outbound host validator to use. - :return: A middleware function that validates outbound URLs. - """ - validator = validator or OutboundHostValidator(enabled=True) +# :param validator: The outbound host validator to use. +# :return: A middleware function that validates outbound URLs. +# """ +# validator = validator or OutboundHostValidator(enabled=True) - async def _middleware(req: ClientRequest, handler: ClientHandlerType) -> ClientResponse: +# async def _middleware(req: ClientRequest, handler: ClientHandlerType) -> ClientResponse: - if validator is not None and validator.enabled and not validator.is_allowed(req.url): - raise ValueError(f"URL '{req.url}' is not allowed by the outbound host validator.") +# if validator is not None and validator.enabled and not validator.is_allowed(req.url): +# raise ValueError(f"URL '{req.url}' is not allowed by the outbound host validator.") - return await handler(req) +# return await handler(req) - return _middleware \ No newline at end of file +# return _middleware \ No newline at end of file diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py index 3363ba9eb..f2b8af207 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py @@ -1,28 +1,167 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -from typing import Protocol, runtime_checkable +import ipaddress +import logging + +from functools import lru_cache + +from aiohttp import ClientSession +from aiohttp.abc import ResolveResult + from yarl import URL -@runtime_checkable -class _OutboundHostValidator(Protocol): +from ._connector import _ResolveResultValidator, _SSRFConnector +from ._validator import _OutboundHostValidator +from ._middleware import _validator_middleware +from ._utils import _normalize_host - @property - def enabled(self) -> bool: - ... +logger = logging.getLogger(__name__) - def is_allowed(self, url: str | URL) -> bool: - ... +_DEFAULT_MICROSOFT_HOSTS = [ + "botframework.com", # Bot Connector / channel services URLs + "smba.trafficmanager.net", # Teams service URLs + "teams.microsoft.com", + "teams.microsoft.us", + "graph.microsoft.com", # Microsoft Graph + "sharepoint.com", # Sharepoint / OneDrive hosted attachments + "svc.ms", # Teams attachment CDN + "blob.core.windows.net", # Azure Blob Storage / Attachment Management Service +] + +class _BasicResolveResultValidator(_ResolveResultValidator): + """Basic implementation of the ResolveResultValidator that allows all resolved results.""" + + def __init__(self, *, allow_private_network_addresses: bool = False): + self._allow_private_network_addresses = allow_private_network_addresses + + def is_allowed(self, resolved: ResolveResult) -> bool: + + hostname = resolved.get("hostname") + + if not hostname: + return False + + try: + address = ipaddress.ip_address(hostname) # Validate the resolved host IP address + if (address.is_private or address.is_loopback) and not self._allow_private_network_addresses: + logger.warning("Private or loopback network address not allowed") + return False + + except ValueError: + logger.warning("Invalid IP address") + return False + + return True class OutboundHostValidator(_OutboundHostValidator): + """Outbound host validator that checks if URLs are allowed based on configured hosts and network address rules.""" - def __init__(self, enabled: bool): - self._enabled = enabled + def __init__( + self, + hosts: list[str] | None = None, + *, + enabled: bool | None = None, + include_default_microsoft_hosts: bool = True, + allow_private_network_addresses: bool = False, + ): + """ + Initializes the outbound host validator. + + :param hosts: A list of allowed hosts. If None, only the default Microsoft hosts will be considered (if included). + :param enabled: Whether the validator is enabled. If None, it will be automatically enabled if hosts are provided. + :param include_default_microsoft_hosts: Whether to include the default Microsoft hosts in the allowed list. + :param allow_private_network_addresses: Whether to allow private network addresses. + """ + if hosts or enabled: + self._enabled = True + + self._hosts: set[str] = set(hosts or []) + self._allow_private_network_addresses = allow_private_network_addresses + + if include_default_microsoft_hosts: + for host in _DEFAULT_MICROSOFT_HOSTS: + normalized_host = _normalize_host(host) + if normalized_host: + self._hosts.add(normalized_host) + else: + logger.warning("Failed to normalize host: %s", host) @property def enabled(self) -> bool: + """Indicates whether the outbound host validator is enabled.""" return self._enabled + def client(self, session: ClientSession | None = None) -> ClientSession: + """Creates a new client session with the outbound host validator applied. + :param session: An optional existing client session to use. + :return: A new client session with the outbound host validator applied. + """ + + headers = session.headers if session else {} + middleware = [_validator_middleware(self)] + + return ClientSession( + headers=headers, + connector=_SSRFConnector(_BasicResolveResultValidator()), + middlewares=middleware, + ) + def is_allowed(self, url: str | URL) -> bool: - # Implement your logic to check if the URL is allowed - return True \ No newline at end of file + """ + Checks if the given URL is allowed based on the configured hosts and network address rules. + + :param url: The URL string or URL object to check. + :return: True if the URL is allowed, False otherwise. + """ + if not self._enabled: + return True + return self._is_allowed(url) + + @lru_cache + def _is_allowed(self, url: str | URL) -> bool: + """ + Determines if the given URL is allowed based on the configured hosts and network address rules. + + :param url: The URL string or URL object to check. + :return: True if the URL is allowed, False otherwise. + """ + # Is this a valid URL? + url_str: str + if isinstance(url, str): + url_str = url + try: + url = URL(url) + except ValueError: + logger.warning("Invalid URL: %s", url) + return False + else: + url_str = str(url) + + if url.host is None: + return False + + # Only allow HTTP and HTTPS schemes + if url.scheme not in ("http", "https"): + return False + + if url.user or url.password: + return False + + address = None + if url.host: + try: + address = ipaddress.ip_address(url.host) + if address.is_private and not self._allow_private_network_addresses: + return False + if address.is_loopback: + return False + + except ValueError: + address = None + + for host in self._hosts: + url_host = (host or "").casefold() + if url.host == host or url_host.endswith("." + host): + return True + return False \ No newline at end of file From 367b0815e59902cd765395b8a6db680e06aa1c26 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Fri, 2 Oct 2026 15:17:36 -0700 Subject: [PATCH 04/15] Updates --- .../core/app/m365_attachment_downloader.py | 3 + .../hosting/core/security/old.py | 328 +++++++++--------- 2 files changed, 167 insertions(+), 164 deletions(-) diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py index 988cdec14..194fd7843 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py @@ -129,6 +129,9 @@ async def _download_file( :return: An InputFile instance or None if the download fails. """ name = attachment.name + client: aiohttp.ClientSession = self._client_factory() + if self._host_validator is not None: + client = self._host_validator.client(client) download_url: str | None = None if isinstance(attachment.content, dict): diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py index 7729d6df1..1066f54e4 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py @@ -1,174 +1,174 @@ -# # Copyright (c) Microsoft Corporation. All rights reserved. -# # Licensed under the MIT License. +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. -# from typing import Callable, Awaitable, Protocol - -# from aiohttp import ( -# ClientSession, -# ClientRequest, -# ClientHandlerType, -# ClientResponse, -# ) -# from yarl import URL -# from urllib.parse import urlparse - -# _DEFAULT_MICROSOFT_HOSTS = [ -# "botframework.com", # Bot Connector / channel services URLs -# "smba.trafficmanager.net", # Teams service URLs -# "teams.microsoft.com", -# "teams.microsoft.us", -# "graph.microsoft.com", # Microsoft Graph -# "sharepoint.com", # Sharepoint / OneDrive hosted attachments -# "svc.ms", # Teams attachment CDN -# "blob.core.windows.net", # Azure Blob Storage / Attachment Management Service -# ] - - -# def _try_create_url(url: str | URL) -> URL | None: -# """Attempts to create a URL object from the given string or URL. - -# :param url: The URL string or URL object to create. -# :return: A URL object if successful, None otherwise. -# """ -# try: -# return URL(url) if isinstance(url, str) else url -# except (ValueError, TypeError): -# return None - - -# def _normalize(host: str) -> str | None: -# """Normalizes a host string to a suffix for comparison. - -# :param host: The host string to normalize. -# :return: The normalized host suffix, or None if the input is invalid. -# """ -# if not host: -# return None - -# host = host.strip().casefold() - -# if host.startswith("*."): -# host = host[2:] - -# url_obj = _try_create_url(host) -# if url_obj and url_obj.host: -# host = url_obj.host -# else: -# slash = host.find("/") -# if slash >= 0: -# host = host[:slash] - -# colon = host.find(":") -# if colon >= 0: -# host = host[:colon] - -# return host if host else None - -# class OutboundHostValidatorProtocol(Protocol): - -# @property -# def enabled(self) -> bool: -# ... - -# def is_allowed(self, url: str | URL) -> bool: -# ... - -# class BasicOutboundHostValidator(OutboundHostValidatorProtocol): -# def __init__(self, enabled: bool = False): -# self._enabled = enabled - -# @property -# def enabled(self) -> bool: -# return self._enabled - -# def is_allowed(self, url: str | URL) -> bool: -# """Checks whether the given URL is allowed by the basic validator. - -# :param url: The URL to check. -# :return: True if the URL is allowed, False otherwise. -# """ -# parsed = urlparse(str(url)) -# return parsed.scheme == "https" or ( -# parsed.scheme == "http" and parsed.hostname == "localhost" -# ) - - -# class OutboundHostValidator: -# """Validates that an outbound URL targets an allowed host before the SDK makes a -# server-side, often token-bearing, request to it (e.g. Activity.service_url callbacks or attachment -# downloads). This is the SDK's shared anti-SSRF ("allow_hosts") control.""" - -# _enabled: bool -# _suffixes: set[str] - -# def __init__( -# self, -# enabled: bool = False, -# hosts: list[str] | None = None, -# include_default_microsoft_hosts: bool = True, -# ): -# """Initializes the outbound host validator. - -# :param enabled: Whether the validator is enabled. -# :param hosts: A list of additional allowed host suffixes. -# :param include_default_microsoft_hosts: Whether to include the default Microsoft hosts. -# """ -# self._enabled = enabled -# suffixes: list[str] = [] -# if include_default_microsoft_hosts: -# suffixes.extend(_DEFAULT_MICROSOFT_HOSTS) - -# if hosts: -# for host in hosts: -# normalized = _normalize(host) -# if normalized is not None: -# suffixes.append(normalized) +from typing import Callable, Awaitable, Protocol + +from aiohttp import ( + ClientSession, + ClientRequest, + ClientHandlerType, + ClientResponse, +) +from yarl import URL +from urllib.parse import urlparse + +_DEFAULT_MICROSOFT_HOSTS = [ + "botframework.com", # Bot Connector / channel services URLs + "smba.trafficmanager.net", # Teams service URLs + "teams.microsoft.com", + "teams.microsoft.us", + "graph.microsoft.com", # Microsoft Graph + "sharepoint.com", # Sharepoint / OneDrive hosted attachments + "svc.ms", # Teams attachment CDN + "blob.core.windows.net", # Azure Blob Storage / Attachment Management Service +] + + +def _try_create_url(url: str | URL) -> URL | None: + """Attempts to create a URL object from the given string or URL. + + :param url: The URL string or URL object to create. + :return: A URL object if successful, None otherwise. + """ + try: + return URL(url) if isinstance(url, str) else url + except (ValueError, TypeError): + return None + + +def _normalize(host: str) -> str | None: + """Normalizes a host string to a suffix for comparison. + + :param host: The host string to normalize. + :return: The normalized host suffix, or None if the input is invalid. + """ + if not host: + return None + + host = host.strip().casefold() + + if host.startswith("*."): + host = host[2:] + + url_obj = _try_create_url(host) + if url_obj and url_obj.host: + host = url_obj.host + else: + slash = host.find("/") + if slash >= 0: + host = host[:slash] + + colon = host.find(":") + if colon >= 0: + host = host[:colon] + + return host if host else None + +class OutboundHostValidatorProtocol(Protocol): + + @property + def enabled(self) -> bool: + ... + + def is_allowed(self, url: str | URL) -> bool: + ... + +class BasicOutboundHostValidator(OutboundHostValidatorProtocol): + def __init__(self, enabled: bool = False): + self._enabled = enabled + + @property + def enabled(self) -> bool: + return self._enabled + + def is_allowed(self, url: str | URL) -> bool: + """Checks whether the given URL is allowed by the basic validator. + + :param url: The URL to check. + :return: True if the URL is allowed, False otherwise. + """ + parsed = urlparse(str(url)) + return parsed.scheme == "https" or ( + parsed.scheme == "http" and parsed.hostname == "localhost" + ) + + +class OutboundHostValidator: + """Validates that an outbound URL targets an allowed host before the SDK makes a + server-side, often token-bearing, request to it (e.g. Activity.service_url callbacks or attachment + downloads). This is the SDK's shared anti-SSRF ("allow_hosts") control.""" + + _enabled: bool + _suffixes: set[str] + + def __init__( + self, + enabled: bool = False, + hosts: list[str] | None = None, + include_default_microsoft_hosts: bool = True, + ): + """Initializes the outbound host validator. + + :param enabled: Whether the validator is enabled. + :param hosts: A list of additional allowed host suffixes. + :param include_default_microsoft_hosts: Whether to include the default Microsoft hosts. + """ + self._enabled = enabled + suffixes: list[str] = [] + if include_default_microsoft_hosts: + suffixes.extend(_DEFAULT_MICROSOFT_HOSTS) + + if hosts: + for host in hosts: + normalized = _normalize(host) + if normalized is not None: + suffixes.append(normalized) -# self._suffixes = set(suffixes) - -# @property -# def enabled(self) -> bool: -# """Gets whether the validator is enabled. If disabled, all outbound hosts are allowed.""" -# return self._enabled - -# def is_allowed(self, url: str | URL) -> bool: -# """Checks whether the given URL is allowed by the validator. + self._suffixes = set(suffixes) + + @property + def enabled(self) -> bool: + """Gets whether the validator is enabled. If disabled, all outbound hosts are allowed.""" + return self._enabled + + def is_allowed(self, url: str | URL) -> bool: + """Checks whether the given URL is allowed by the validator. -# :param url: The URL to check. -# :return: True if the URL is allowed, False otherwise. -# """ -# if not self._enabled: -# return True - -# url_obj = _try_create_url(url) -# if not url_obj or not url_obj.absolute or url_obj.scheme != "https": -# return False + :param url: The URL to check. + :return: True if the URL is allowed, False otherwise. + """ + if not self._enabled: + return True + + url_obj = _try_create_url(url) + if not url_obj or not url_obj.absolute or url_obj.scheme != "https": + return False -# host = url_obj.host -# if not host: -# return False - -# host = host.casefold() - -# for suffix in self._suffixes: -# if host == suffix or host.endswith("." + suffix): -# return True - -# return False + host = url_obj.host + if not host: + return False + + host = host.casefold() + + for suffix in self._suffixes: + if host == suffix or host.endswith("." + suffix): + return True + + return False -# def _validator_middleware(validator: OutboundHostValidatorProtocol | None = None) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: -# """Creates a middleware that validates outbound URLs using the given validator. +def _validator_middleware(validator: OutboundHostValidatorProtocol | None = None) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: + """Creates a middleware that validates outbound URLs using the given validator. -# :param validator: The outbound host validator to use. -# :return: A middleware function that validates outbound URLs. -# """ -# validator = validator or OutboundHostValidator(enabled=True) + :param validator: The outbound host validator to use. + :return: A middleware function that validates outbound URLs. + """ + validator = validator or OutboundHostValidator(enabled=True) -# async def _middleware(req: ClientRequest, handler: ClientHandlerType) -> ClientResponse: + async def _middleware(req: ClientRequest, handler: ClientHandlerType) -> ClientResponse: -# if validator is not None and validator.enabled and not validator.is_allowed(req.url): -# raise ValueError(f"URL '{req.url}' is not allowed by the outbound host validator.") + if validator is not None and validator.enabled and not validator.is_allowed(req.url): + raise ValueError(f"URL '{req.url}' is not allowed by the outbound host validator.") -# return await handler(req) + return await handler(req) -# return _middleware \ No newline at end of file + return _middleware \ No newline at end of file From 1403a299da93810fccd64f5866775a49d3718e71 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Fri, 2 Oct 2026 16:04:59 -0700 Subject: [PATCH 05/15] Addressing feedback --- .../microsoft_agents/hosting/core/__init__.py | 4 +- .../hosting/core/_http_adapter_base.py | 6 +- .../hosting/core/app/attachment_downloader.py | 47 ++++++---- .../core/app/m365_attachment_downloader.py | 47 +++++----- .../hosting/core/security/__init__.py | 8 +- .../hosting/core/security/_connector.py | 15 +++- .../hosting/core/security/_middleware.py | 22 +++-- .../hosting/core/security/_utils.py | 4 +- .../hosting/core/security/_validator.py | 4 +- .../hosting/core/security/old.py | 33 ++++--- .../core/security/outbound_host_validator.py | 86 ++++++++++++------- 11 files changed, 176 insertions(+), 100 deletions(-) diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/__init__.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/__init__.py index 813f800eb..e1338972e 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/__init__.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/__init__.py @@ -9,7 +9,9 @@ from .middleware_set import Middleware, MiddlewareSet from .rest_channel_service_client_factory import RestChannelServiceClientFactory from .turn_context import TurnContext -from .outbound_host_validator import OutboundHostValidator + +# Security +from .security import OutboundHostValidator # HTTP abstractions from .http import ( diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_http_adapter_base.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_http_adapter_base.py index ec8173431..b4dab00b8 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_http_adapter_base.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_http_adapter_base.py @@ -23,7 +23,7 @@ from .message_factory import MessageFactory from .rest_channel_service_client_factory import RestChannelServiceClientFactory from .turn_context import TurnContext -from .outbound_host_validator import OutboundHostValidator, _try_create_url +from .security import OutboundHostValidator, _try_create_url logger = logging.getLogger(__name__) @@ -80,7 +80,7 @@ async def on_turn_error(context: TurnContext, error: Exception): connection_manager, **(channel_service_client_factory_options or {}), ) - self._host_validator = host_validator or OutboundHostValidator() + self._host_validator = host_validator or OutboundHostValidator(enabled=False) super().__init__(factory) @@ -205,7 +205,7 @@ def _validate_service_url( or not activity_url or claim_url_host.casefold() != activity_url_host.casefold() ): - if self._host_validator and self._host_validator.enabled: + if self._host_validator.enabled: logger.warning( "Service URL host mismatch: %s vs %s", claim_url_host, diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py index 8d0638197..43f296ecf 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py @@ -2,6 +2,7 @@ # Licensed under the MIT License. import json +import logging from typing import Callable import aiohttp @@ -13,30 +14,32 @@ ) from microsoft_agents.hosting.core.turn_context import TurnContext -from microsoft_agents.hosting.core.outbound_host_validator import ( +from microsoft_agents.hosting.core.security import ( OutboundHostValidator, - _validator_middleware, + _SSRFError, ) from .input_file import InputFileDownloader, InputFile from ._utils import _parse_content_type, _basic_url_check +logger = logging.getLogger(__name__) + class AttachmentDownloader(InputFileDownloader): """Downloads attachments from a given turn context.""" def __init__( self, - client_factory: Callable[[], aiohttp.ClientSession] | None = None, host_validator: OutboundHostValidator | None = None, + *, + client_session_kwargs: dict | None = None ): """Constructor for AttachmentDownloader. - :param client_factory: A callable that returns an aiohttp.ClientSession instance. :param host_validator: An optional OutboundHostValidator instance. """ - self._client_factory = client_factory or aiohttp.ClientSession + self._client_session_kwargs = client_session_kwargs or {} self._host_validator = host_validator async def download_files(self, context: TurnContext) -> list[InputFile]: @@ -51,35 +54,35 @@ async def download_files(self, context: TurnContext) -> list[InputFile]: if not context.activity.attachments: return [] + client: aiohttp.ClientSession + if self._host_validator is not None: + client = self._host_validator.client(self._client_session_kwargs) + else: + client = aiohttp.ClientSession(**self._client_session_kwargs) + files: list[InputFile] = [] for attachment in context.activity.attachments: - file = await self._download_file(attachment) + file = await self._download_file(client, attachment) if file: files.append(file) + await client.close() return files - async def _download_file(self, attachment: Attachment) -> InputFile | None: + async def _download_file( + self, client: aiohttp.ClientSession, attachment: Attachment + ) -> InputFile | None: """Downloads a single file from the given attachment. + :param client: The aiohttp.ClientSession instance to use for downloading the file. :param attachment: The attachment to download. :return: An InputFile instance if the download is successful, None otherwise. """ if attachment.content_url and _basic_url_check(attachment.content_url): remote_file_url = attachment.content_url - if ( - self._host_validator - and self._host_validator.enabled - and not self._host_validator.is_allowed(remote_file_url) - ): - return None - - async with self._client_factory() as client: - async with client.get( - remote_file_url, - middlewares=[_validator_middleware(self._host_validator)] - ) as response: + try: + async with client.get(remote_file_url) as response: if not (200 <= response.status < 300): return None @@ -100,6 +103,12 @@ async def _download_file(self, attachment: Attachment) -> InputFile | None: content_url=attachment.content_url, filename=attachment.name, ) + except _SSRFError: + logger.warning( + "Outbound host validation failed for download URL: %s", + remote_file_url, + ) + return None else: content = bytes(json.dumps(attachment.content), "utf-8") return InputFile( diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py index 194fd7843..922742972 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py @@ -2,6 +2,7 @@ # Licensed under the MIT License. import json +import logging from typing import Callable, cast, Any import aiohttp @@ -16,14 +17,16 @@ Connections, ) from microsoft_agents.hosting.core.turn_context import TurnContext -from microsoft_agents.hosting.core.outbound_host_validator import ( +from microsoft_agents.hosting.core.security import ( OutboundHostValidator, - _validator_middleware, + _SSRFError, ) from .input_file import InputFileDownloader, InputFile from ._utils import _parse_content_type, _basic_url_check +logger = logging.getLogger(__name__) + class M365AttachmentDownloader(InputFileDownloader): """Downloads attachments from M365/Teams using the configured Token Provider (from Connections).""" @@ -31,9 +34,9 @@ class M365AttachmentDownloader(InputFileDownloader): def __init__( self, connections: Connections, - client_factory: Callable[[], aiohttp.ClientSession] | None = None, host_validator: OutboundHostValidator | None = None, *, + client_session_kwargs: dict[str, Any] | None = None, token_provider_name: str = "", use_anonymous: bool = False, scopes: list[str] | None = None, @@ -41,7 +44,6 @@ def __init__( """Constructor for M365AttachmentDownloader. :param connections: A Connections instance. - :param client_factory: A callable that returns an aiohttp.ClientSession instance. :param host_validator: An optional OutboundHostValidator instance. :param token_provider_name: The name of the token provider. :param use_anonymous: Whether to use anonymous access. @@ -50,8 +52,8 @@ def __init__( """ self._connections = connections - self._client_factory = client_factory or aiohttp.ClientSession self._host_validator = host_validator + self._client_session_kwargs = client_session_kwargs or {} self._token_provider_name = token_provider_name self._use_anonymous = use_anonymous @@ -111,27 +113,35 @@ async def download_files(self, context: TurnContext) -> list[InputFile]: outgoing_audience_claim, self._scopes ) + client: aiohttp.ClientSession + if self._host_validator is not None: + client = self._host_validator.client(self._client_session_kwargs) + else: + client = aiohttp.ClientSession(**self._client_session_kwargs) + files: list[InputFile] = [] for att in attachments: - file = await self._download_file(att, access_token) + file = await self._download_file(client, att, access_token) if file: files.append(file) + await client.close() return files async def _download_file( - self, attachment: Attachment, access_token: str + self, + client: aiohttp.ClientSession, + attachment: Attachment, + access_token: str, ) -> InputFile | None: """Download a single file from the given attachment. + :param client: The aiohttp.ClientSession instance to use for downloading the file. :param attachment: The Attachment instance. :param access_token: The access token for authentication. :return: An InputFile instance or None if the download fails. """ name = attachment.name - client: aiohttp.ClientSession = self._client_factory() - if self._host_validator is not None: - client = self._host_validator.client(client) download_url: str | None = None if isinstance(attachment.content, dict): @@ -140,19 +150,11 @@ async def _download_file( else: download_url = attachment.content_url - if download_url and _basic_url_check(download_url): - if ( - self._host_validator is not None - and self._host_validator.enabled - and not self._host_validator.is_allowed(download_url) - ): - return None - - async with self._client_factory() as client: + if download_url: + try: async with client.get( download_url, headers={"Authorization": f"Bearer {access_token}"}, - middlewares=[_validator_middleware(self._host_validator)] ) as response: if not (200 <= response.status < 300): return None @@ -172,6 +174,11 @@ async def _download_file( content_url=attachment.content_url, filename=name, ) + except _SSRFError: + logger.warning( + "Outbound host validation failed for download URL: %s", download_url + ) + return None else: content = bytes(json.dumps(attachment.content), "utf-8") return InputFile( diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py index 82b079b8e..68e2a6d79 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py @@ -1,8 +1,12 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +from ._connector import _SSRFError +from ._utils import _try_create_url from .outbound_host_validator import OutboundHostValidator __all__ = [ - "OutboundHostValidator" -] \ No newline at end of file + "OutboundHostValidator", + "_SSRFError", + "_try_create_url", +] diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py index 98d5890ae..d2368ad35 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py @@ -8,27 +8,34 @@ from aiohttp.abc import ResolveResult from aiohttp.tracing import Trace + class _ResolveResultValidator(Protocol): """Protocol for validating resolved results to prevent SSRF attacks.""" def is_allowed(self, resolved: ResolveResult) -> bool: """Check if the resolved result is valid to prevent SSRF attacks. - + :param resolved: The resolved result to validate. :return: True if the resolved result is valid, False otherwise. """ ... + class _SSRFError(ValueError): """Exception raised when an SSRF attack is detected.""" + class _SSRFConnector(TCPConnector): - def __init__(self, resolved_result_validator: _ResolveResultValidator, *args, **kwargs): + def __init__( + self, resolved_result_validator: _ResolveResultValidator, *args, **kwargs + ): super().__init__(*args, **kwargs) self._resolved_result_validator = resolved_result_validator - async def _resolve_host(self, host: str, port: int, traces: Sequence[Trace] | None = None) -> list[ResolveResult]: + async def _resolve_host( + self, host: str, port: int, traces: Sequence[Trace] | None = None + ) -> list[ResolveResult]: """Resolve the host and validate the resolved results to prevent SSRF attacks. :param host: The hostname to resolve. @@ -43,4 +50,4 @@ async def _resolve_host(self, host: str, port: int, traces: Sequence[Trace] | No for r in res: if not self._resolved_result_validator.is_allowed(r): raise _SSRFError(f"Invalid resolved result: {r}") - return res \ No newline at end of file + return res diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py index c2deb4e6f..2334d449a 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py @@ -5,20 +5,28 @@ from aiohttp import ClientRequest, ClientHandlerType, ClientResponse -from ._validator import _URLValidator +from ._validator import _URLValidator, _SSRFError -def _validator_middleware(validator: _URLValidator) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: + +def _validator_middleware( + validator: _URLValidator, +) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: """Creates a middleware that validates outbound URLs using the given validator. - + :param validator: The outbound host validator to use. :return: A middleware function that validates outbound URLs. - """ - async def _middleware(req: ClientRequest, handler: ClientHandlerType) -> ClientResponse: + """ + + async def _middleware( + req: ClientRequest, handler: ClientHandlerType + ) -> ClientResponse: """Middleware that validates outbound URLs using the given host validator.""" if not validator.is_allowed(req.url): - raise ValueError(f"URL '{req.url}' is not allowed by the outbound host validator.") + raise _SSRFError( + f"URL '{req.url}' is not allowed by the outbound host validator." + ) return await handler(req) - return _middleware \ No newline at end of file + return _middleware diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py index d94e054a1..73fe69abe 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py @@ -10,6 +10,7 @@ from yarl import URL + def _try_create_url(url: str | URL) -> URL | None: """Attempts to create a URL object from the given string or URL. @@ -21,6 +22,7 @@ def _try_create_url(url: str | URL) -> URL | None: except (ValueError, TypeError): return None + def _normalize_host(host: str) -> str | None: """Normalizes the given host string. @@ -47,4 +49,4 @@ def _normalize_host(host: str) -> str | None: if colon >= 0: host = host[:colon] - return host if host else None \ No newline at end of file + return host if host else None diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py index bb50171b0..ee099713b 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py @@ -10,6 +10,7 @@ logger = logging.getLogger(__name__) + class _URLValidator(Protocol): """Protocol for URL validators.""" @@ -21,6 +22,7 @@ def is_allowed(self, url: str | URL) -> bool: """ ... + class _OutboundHostValidator(_URLValidator): """Protocol for outbound host validators.""" @@ -30,4 +32,4 @@ def client(self, session: ClientSession | None = None) -> ClientSession: :param session: An optional existing ClientSession to use. :return: A ClientSession instance. """ - ... \ No newline at end of file + ... diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py index 1066f54e4..6cbde28ed 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py @@ -64,14 +64,14 @@ def _normalize(host: str) -> str | None: return host if host else None + class OutboundHostValidatorProtocol(Protocol): @property - def enabled(self) -> bool: - ... + def enabled(self) -> bool: ... + + def is_allowed(self, url: str | URL) -> bool: ... - def is_allowed(self, url: str | URL) -> bool: - ... class BasicOutboundHostValidator(OutboundHostValidatorProtocol): def __init__(self, enabled: bool = False): @@ -156,19 +156,30 @@ def is_allowed(self, url: str | URL) -> bool: return False -def _validator_middleware(validator: OutboundHostValidatorProtocol | None = None) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: + +def _validator_middleware( + validator: OutboundHostValidatorProtocol | None = None, +) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: """Creates a middleware that validates outbound URLs using the given validator. - + :param validator: The outbound host validator to use. :return: A middleware function that validates outbound URLs. """ validator = validator or OutboundHostValidator(enabled=True) - - async def _middleware(req: ClientRequest, handler: ClientHandlerType) -> ClientResponse: - if validator is not None and validator.enabled and not validator.is_allowed(req.url): - raise ValueError(f"URL '{req.url}' is not allowed by the outbound host validator.") + async def _middleware( + req: ClientRequest, handler: ClientHandlerType + ) -> ClientResponse: + + if ( + validator is not None + and validator.enabled + and not validator.is_allowed(req.url) + ): + raise ValueError( + f"URL '{req.url}' is not allowed by the outbound host validator." + ) return await handler(req) - return _middleware \ No newline at end of file + return _middleware diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py index f2b8af207..4480f64cd 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py @@ -11,12 +11,12 @@ from yarl import URL -from ._connector import _ResolveResultValidator, _SSRFConnector +from ._connector import _ResolveResultValidator, _SSRFConnector, _SSRFError from ._validator import _OutboundHostValidator from ._middleware import _validator_middleware from ._utils import _normalize_host -logger = logging.getLogger(__name__) +logger = logging.getLogger(__name__) _DEFAULT_MICROSOFT_HOSTS = [ "botframework.com", # Bot Connector / channel services URLs @@ -29,6 +29,7 @@ "blob.core.windows.net", # Azure Blob Storage / Attachment Management Service ] + class _BasicResolveResultValidator(_ResolveResultValidator): """Basic implementation of the ResolveResultValidator that allows all resolved results.""" @@ -37,14 +38,16 @@ def __init__(self, *, allow_private_network_addresses: bool = False): def is_allowed(self, resolved: ResolveResult) -> bool: - hostname = resolved.get("hostname") + host = resolved.get("host") - if not hostname: + if not host: return False try: - address = ipaddress.ip_address(hostname) # Validate the resolved host IP address - if (address.is_private or address.is_loopback) and not self._allow_private_network_addresses: + address = ipaddress.ip_address( + host + ) # Validate the resolved host IP address + if (not address.is_global) and not self._allow_private_network_addresses: logger.warning("Private or loopback network address not allowed") return False @@ -54,6 +57,7 @@ def is_allowed(self, resolved: ResolveResult) -> bool: return True + class OutboundHostValidator(_OutboundHostValidator): """Outbound host validator that checks if URLs are allowed based on configured hosts and network address rules.""" @@ -73,10 +77,17 @@ def __init__( :param include_default_microsoft_hosts: Whether to include the default Microsoft hosts in the allowed list. :param allow_private_network_addresses: Whether to allow private network addresses. """ - if hosts or enabled: - self._enabled = True + self._enabled = bool(hosts or enabled) + + self._hosts: set[str] = set() + + for host in hosts or []: + normalized_host = _normalize_host(host) + if normalized_host: + self._hosts.add(normalized_host) + else: + logger.warning("Failed to normalize host: %s", host) - self._hosts: set[str] = set(hosts or []) self._allow_private_network_addresses = allow_private_network_addresses if include_default_microsoft_hosts: @@ -87,24 +98,32 @@ def __init__( else: logger.warning("Failed to normalize host: %s", host) + self._resolved_validator = _BasicResolveResultValidator( + allow_private_network_addresses=self._allow_private_network_addresses + ) + @property def enabled(self) -> bool: """Indicates whether the outbound host validator is enabled.""" return self._enabled - def client(self, session: ClientSession | None = None) -> ClientSession: + def client(self, client_session_kwargs: dict | None = None) -> ClientSession: """Creates a new client session with the outbound host validator applied. - :param session: An optional existing client session to use. + :param client_session_kwargs: Optional keyword arguments to pass to the ClientSession constructor. :return: A new client session with the outbound host validator applied. """ - headers = session.headers if session else {} + if "connector" in (client_session_kwargs or {}): + raise ValueError("Specifying a custom connector is not allowed.") + if "middlewares" in (client_session_kwargs or {}): + raise ValueError("Specifying custom middlewares is not allowed.") + middleware = [_validator_middleware(self)] return ClientSession( - headers=headers, - connector=_SSRFConnector(_BasicResolveResultValidator()), + connector=_SSRFConnector(self._resolved_validator), middlewares=middleware, + **(client_session_kwargs or {}), ) def is_allowed(self, url: str | URL) -> bool: @@ -114,9 +133,12 @@ def is_allowed(self, url: str | URL) -> bool: :param url: The URL string or URL object to check. :return: True if the URL is allowed, False otherwise. """ - if not self._enabled: - return True - return self._is_allowed(url) + try: + if not self._enabled: + return True + return self._is_allowed(url) + except _SSRFError: + return False @lru_cache def _is_allowed(self, url: str | URL) -> bool: @@ -148,20 +170,22 @@ def _is_allowed(self, url: str | URL) -> bool: if url.user or url.password: return False + if not url.host: + return False + address = None - if url.host: - try: - address = ipaddress.ip_address(url.host) - if address.is_private and not self._allow_private_network_addresses: - return False - if address.is_loopback: - return False - - except ValueError: - address = None + try: + address = ipaddress.ip_address(url.host) + if not address.is_global and not self._allow_private_network_addresses: + return False + + except ValueError: + address = None + + url_host = (url.host or "").casefold() - for host in self._hosts: - url_host = (host or "").casefold() - if url.host == host or url_host.endswith("." + host): + for valid_host in self._hosts: + url_host = (url.host or "").casefold() + if url_host == valid_host or url_host.endswith("." + valid_host): return True - return False \ No newline at end of file + return False From feb7232b0b733f35ca28d8d978c91aaf25ad8257 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Mon, 5 Oct 2026 11:13:42 -0700 Subject: [PATCH 06/15] Fixing the tests --- .../hosting/core/app/attachment_downloader.py | 38 ++- .../core/app/m365_attachment_downloader.py | 29 +- .../hosting/core/security/__init__.py | 6 +- .../hosting/core/security/_connector.py | 5 + .../hosting/core/security/_middleware.py | 3 +- .../hosting/core/security/_validator.py | 6 +- .../hosting/core/security/old.py | 185 ---------- .../core/security/outbound_host_validator.py | 51 +-- .../app/test_attachment_downloader.py | 77 +++-- .../app/test_m365_attachment_downloader.py | 79 +++-- .../test_outbound_host_validator.py | 322 ++++++++++++++++-- 11 files changed, 471 insertions(+), 330 deletions(-) delete mode 100644 libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py index 43f296ecf..3739e6b45 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py @@ -3,7 +3,7 @@ import json import logging -from typing import Callable +from typing import Any, Callable import aiohttp @@ -20,7 +20,7 @@ ) from .input_file import InputFileDownloader, InputFile -from ._utils import _parse_content_type, _basic_url_check +from ._utils import _parse_content_type logger = logging.getLogger(__name__) @@ -32,13 +32,21 @@ def __init__( self, host_validator: OutboundHostValidator | None = None, *, - client_session_kwargs: dict | None = None + client_session_kwargs: dict | None = None, + client_factory: Any = None, ): """Constructor for AttachmentDownloader. :param host_validator: An optional OutboundHostValidator instance. + :param client_session_kwargs: Optional keyword arguments for the aiohttp.ClientSession. + :param client_factory: (deprecated) A custom client factory, if any. """ + if client_factory is not None: + logger.warning( + "The 'client_factory' parameter is deprecated and will be ignored." + ) + self._client_session_kwargs = client_session_kwargs or {} self._host_validator = host_validator @@ -54,19 +62,21 @@ async def download_files(self, context: TurnContext) -> list[InputFile]: if not context.activity.attachments: return [] - client: aiohttp.ClientSession + client_factory: Callable[[], aiohttp.ClientSession] if self._host_validator is not None: - client = self._host_validator.client(self._client_session_kwargs) + host_validator = self._host_validator # to capture in lambda + client_factory = lambda: host_validator.client(self._client_session_kwargs) else: - client = aiohttp.ClientSession(**self._client_session_kwargs) - - files: list[InputFile] = [] - for attachment in context.activity.attachments: - file = await self._download_file(client, attachment) - if file: - files.append(file) + client_factory = lambda: aiohttp.ClientSession( + **self._client_session_kwargs + ) - await client.close() + async with client_factory() as client: + files: list[InputFile] = [] + for attachment in context.activity.attachments: + file = await self._download_file(client, attachment) + if file: + files.append(file) return files async def _download_file( @@ -78,7 +88,7 @@ async def _download_file( :param attachment: The attachment to download. :return: An InputFile instance if the download is successful, None otherwise. """ - if attachment.content_url and _basic_url_check(attachment.content_url): + if attachment.content_url: remote_file_url = attachment.content_url try: diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py index 922742972..c9041fb2f 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py @@ -40,6 +40,7 @@ def __init__( token_provider_name: str = "", use_anonymous: bool = False, scopes: list[str] | None = None, + client_factory: Any = None, ): """Constructor for M365AttachmentDownloader. @@ -49,8 +50,14 @@ def __init__( :param use_anonymous: Whether to use anonymous access. :param scopes: A list of scopes for the access token. :param connections: A Connections instance. + :param client_factory: (deprecated) A custom client factory, if any. """ + if client_factory is not None: + logger.warning( + "The 'client_factory' parameter is deprecated and will be ignored." + ) + self._connections = connections self._host_validator = host_validator self._client_session_kwargs = client_session_kwargs or {} @@ -113,19 +120,21 @@ async def download_files(self, context: TurnContext) -> list[InputFile]: outgoing_audience_claim, self._scopes ) - client: aiohttp.ClientSession + client_factory: Callable[[], aiohttp.ClientSession] if self._host_validator is not None: - client = self._host_validator.client(self._client_session_kwargs) + host_validator = self._host_validator # to + client_factory = lambda: host_validator.client(self._client_session_kwargs) else: - client = aiohttp.ClientSession(**self._client_session_kwargs) - - files: list[InputFile] = [] - for att in attachments: - file = await self._download_file(client, att, access_token) - if file: - files.append(file) + client_factory = lambda: aiohttp.ClientSession( + **self._client_session_kwargs + ) - await client.close() + async with client_factory() as client: + files: list[InputFile] = [] + for att in attachments: + file = await self._download_file(client, att, access_token) + if file: + files.append(file) return files async def _download_file( diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py index 68e2a6d79..4c99637e8 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/__init__.py @@ -1,12 +1,14 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -from ._connector import _SSRFError -from ._utils import _try_create_url +from ._connector import _SSRFError, _SSRFConnector +from ._utils import _try_create_url, _normalize_host from .outbound_host_validator import OutboundHostValidator __all__ = [ "OutboundHostValidator", "_SSRFError", + "_SSRFConnector", "_try_create_url", + "_normalize_host", ] diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py index d2368ad35..17ef00675 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_connector.py @@ -26,10 +26,15 @@ class _SSRFError(ValueError): class _SSRFConnector(TCPConnector): + """TCPConnector subclass that validates resolved host results to prevent SSRF attacks.""" def __init__( self, resolved_result_validator: _ResolveResultValidator, *args, **kwargs ): + """Initialize the SSRFConnector with a resolved result validator. + + :param resolved_result_validator: The validator used to check resolved host results. + """ super().__init__(*args, **kwargs) self._resolved_result_validator = resolved_result_validator diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py index 2334d449a..3b4a88ae6 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py @@ -5,7 +5,8 @@ from aiohttp import ClientRequest, ClientHandlerType, ClientResponse -from ._validator import _URLValidator, _SSRFError +from ._connector import _SSRFError +from ._validator import _URLValidator def _validator_middleware( diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py index ee099713b..cebf2a69a 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_validator.py @@ -23,13 +23,13 @@ def is_allowed(self, url: str | URL) -> bool: ... -class _OutboundHostValidator(_URLValidator): +class _OutboundHostValidator(_URLValidator, Protocol): """Protocol for outbound host validators.""" - def client(self, session: ClientSession | None = None) -> ClientSession: + def client(self, client_session_kwargs: dict | None = None) -> ClientSession: """Returns a client session for making outbound requests. - :param session: An optional existing ClientSession to use. + :param client_session_kwargs: Optional keyword arguments to pass to the ClientSession constructor. :return: A ClientSession instance. """ ... diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py deleted file mode 100644 index 6cbde28ed..000000000 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/old.py +++ /dev/null @@ -1,185 +0,0 @@ -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. - -from typing import Callable, Awaitable, Protocol - -from aiohttp import ( - ClientSession, - ClientRequest, - ClientHandlerType, - ClientResponse, -) -from yarl import URL -from urllib.parse import urlparse - -_DEFAULT_MICROSOFT_HOSTS = [ - "botframework.com", # Bot Connector / channel services URLs - "smba.trafficmanager.net", # Teams service URLs - "teams.microsoft.com", - "teams.microsoft.us", - "graph.microsoft.com", # Microsoft Graph - "sharepoint.com", # Sharepoint / OneDrive hosted attachments - "svc.ms", # Teams attachment CDN - "blob.core.windows.net", # Azure Blob Storage / Attachment Management Service -] - - -def _try_create_url(url: str | URL) -> URL | None: - """Attempts to create a URL object from the given string or URL. - - :param url: The URL string or URL object to create. - :return: A URL object if successful, None otherwise. - """ - try: - return URL(url) if isinstance(url, str) else url - except (ValueError, TypeError): - return None - - -def _normalize(host: str) -> str | None: - """Normalizes a host string to a suffix for comparison. - - :param host: The host string to normalize. - :return: The normalized host suffix, or None if the input is invalid. - """ - if not host: - return None - - host = host.strip().casefold() - - if host.startswith("*."): - host = host[2:] - - url_obj = _try_create_url(host) - if url_obj and url_obj.host: - host = url_obj.host - else: - slash = host.find("/") - if slash >= 0: - host = host[:slash] - - colon = host.find(":") - if colon >= 0: - host = host[:colon] - - return host if host else None - - -class OutboundHostValidatorProtocol(Protocol): - - @property - def enabled(self) -> bool: ... - - def is_allowed(self, url: str | URL) -> bool: ... - - -class BasicOutboundHostValidator(OutboundHostValidatorProtocol): - def __init__(self, enabled: bool = False): - self._enabled = enabled - - @property - def enabled(self) -> bool: - return self._enabled - - def is_allowed(self, url: str | URL) -> bool: - """Checks whether the given URL is allowed by the basic validator. - - :param url: The URL to check. - :return: True if the URL is allowed, False otherwise. - """ - parsed = urlparse(str(url)) - return parsed.scheme == "https" or ( - parsed.scheme == "http" and parsed.hostname == "localhost" - ) - - -class OutboundHostValidator: - """Validates that an outbound URL targets an allowed host before the SDK makes a - server-side, often token-bearing, request to it (e.g. Activity.service_url callbacks or attachment - downloads). This is the SDK's shared anti-SSRF ("allow_hosts") control.""" - - _enabled: bool - _suffixes: set[str] - - def __init__( - self, - enabled: bool = False, - hosts: list[str] | None = None, - include_default_microsoft_hosts: bool = True, - ): - """Initializes the outbound host validator. - - :param enabled: Whether the validator is enabled. - :param hosts: A list of additional allowed host suffixes. - :param include_default_microsoft_hosts: Whether to include the default Microsoft hosts. - """ - self._enabled = enabled - suffixes: list[str] = [] - if include_default_microsoft_hosts: - suffixes.extend(_DEFAULT_MICROSOFT_HOSTS) - - if hosts: - for host in hosts: - normalized = _normalize(host) - if normalized is not None: - suffixes.append(normalized) - - self._suffixes = set(suffixes) - - @property - def enabled(self) -> bool: - """Gets whether the validator is enabled. If disabled, all outbound hosts are allowed.""" - return self._enabled - - def is_allowed(self, url: str | URL) -> bool: - """Checks whether the given URL is allowed by the validator. - - :param url: The URL to check. - :return: True if the URL is allowed, False otherwise. - """ - if not self._enabled: - return True - - url_obj = _try_create_url(url) - if not url_obj or not url_obj.absolute or url_obj.scheme != "https": - return False - - host = url_obj.host - if not host: - return False - - host = host.casefold() - - for suffix in self._suffixes: - if host == suffix or host.endswith("." + suffix): - return True - - return False - - -def _validator_middleware( - validator: OutboundHostValidatorProtocol | None = None, -) -> Callable[[ClientRequest, ClientHandlerType], Awaitable[ClientResponse]]: - """Creates a middleware that validates outbound URLs using the given validator. - - :param validator: The outbound host validator to use. - :return: A middleware function that validates outbound URLs. - """ - validator = validator or OutboundHostValidator(enabled=True) - - async def _middleware( - req: ClientRequest, handler: ClientHandlerType - ) -> ClientResponse: - - if ( - validator is not None - and validator.enabled - and not validator.is_allowed(req.url) - ): - raise ValueError( - f"URL '{req.url}' is not allowed by the outbound host validator." - ) - - return await handler(req) - - return _middleware diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py index 4480f64cd..65ceeba02 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py @@ -14,7 +14,7 @@ from ._connector import _ResolveResultValidator, _SSRFConnector, _SSRFError from ._validator import _OutboundHostValidator from ._middleware import _validator_middleware -from ._utils import _normalize_host +from ._utils import _normalize_host, _try_create_url logger = logging.getLogger(__name__) @@ -31,7 +31,7 @@ class _BasicResolveResultValidator(_ResolveResultValidator): - """Basic implementation of the ResolveResultValidator that allows all resolved results.""" + """Validates resolved network addresses used by the SSRF-safe connector.""" def __init__(self, *, allow_private_network_addresses: bool = False): self._allow_private_network_addresses = allow_private_network_addresses @@ -44,10 +44,11 @@ def is_allowed(self, resolved: ResolveResult) -> bool: return False try: - address = ipaddress.ip_address( - host - ) # Validate the resolved host IP address - if (not address.is_global) and not self._allow_private_network_addresses: + address = ipaddress.ip_address(host) + if not ( + address.is_global + or (address.is_private and self._allow_private_network_addresses) + ): logger.warning("Private or loopback network address not allowed") return False @@ -77,7 +78,7 @@ def __init__( :param include_default_microsoft_hosts: Whether to include the default Microsoft hosts in the allowed list. :param allow_private_network_addresses: Whether to allow private network addresses. """ - self._enabled = bool(hosts or enabled) + self._enabled = bool(hosts) if enabled is None else enabled self._hosts: set[str] = set() @@ -115,13 +116,19 @@ def client(self, client_session_kwargs: dict | None = None) -> ClientSession: if "connector" in (client_session_kwargs or {}): raise ValueError("Specifying a custom connector is not allowed.") + if "connector_owner" in (client_session_kwargs or {}): + raise ValueError("Specifying a connector_owner value is not allowed.") if "middlewares" in (client_session_kwargs or {}): raise ValueError("Specifying custom middlewares is not allowed.") middleware = [_validator_middleware(self)] + if not self._enabled: + return ClientSession(**(client_session_kwargs or {})) + return ClientSession( connector=_SSRFConnector(self._resolved_validator), + connector_owner=True, middlewares=middleware, **(client_session_kwargs or {}), ) @@ -141,24 +148,17 @@ def is_allowed(self, url: str | URL) -> bool: return False @lru_cache - def _is_allowed(self, url: str | URL) -> bool: + def _is_allowed(self, _url: str | URL) -> bool: """ Determines if the given URL is allowed based on the configured hosts and network address rules. - :param url: The URL string or URL object to check. + :param _url: The URL string or URL object to check. :return: True if the URL is allowed, False otherwise. """ - # Is this a valid URL? - url_str: str - if isinstance(url, str): - url_str = url - try: - url = URL(url) - except ValueError: - logger.warning("Invalid URL: %s", url) - return False - else: - url_str = str(url) + url = _try_create_url(_url) + if not url: + logger.warning("Invalid URL: %s", _url) + return False if url.host is None: return False @@ -173,19 +173,20 @@ def _is_allowed(self, url: str | URL) -> bool: if not url.host: return False - address = None try: address = ipaddress.ip_address(url.host) - if not address.is_global and not self._allow_private_network_addresses: + if not ( + address.is_global + or (address.is_private and self._allow_private_network_addresses) + ): return False except ValueError: - address = None + pass - url_host = (url.host or "").casefold() + url_host = url.host.casefold() for valid_host in self._hosts: - url_host = (url.host or "").casefold() if url_host == valid_host or url_host.endswith("." + valid_host): return True return False diff --git a/tests/hosting_core/app/test_attachment_downloader.py b/tests/hosting_core/app/test_attachment_downloader.py index 799b3478f..05e596919 100644 --- a/tests/hosting_core/app/test_attachment_downloader.py +++ b/tests/hosting_core/app/test_attachment_downloader.py @@ -14,7 +14,7 @@ ) from microsoft_agents.hosting.core import TurnContext from microsoft_agents.hosting.core.app.attachment_downloader import AttachmentDownloader -from microsoft_agents.hosting.core.outbound_host_validator import OutboundHostValidator +from microsoft_agents.hosting.core.security import OutboundHostValidator, _SSRFError def _make_context( @@ -53,11 +53,18 @@ async def __aexit__(self, *args) -> bool: class _FakeSession: - def __init__(self, response: _FakeResponse): + def __init__( + self, + response: _FakeResponse, + validator: OutboundHostValidator | None = None, + ): self._response = response + self._validator = validator self.requested_urls: list[str] = [] def get(self, url: str, **kwargs) -> _FakeResponse: + if self._validator and not self._validator.is_allowed(url): + raise _SSRFError(f"URL '{url}' is not allowed") self.requested_urls.append(url) return self._response @@ -68,6 +75,13 @@ async def __aexit__(self, *args) -> bool: return False +def _patch_client_session(monkeypatch, session: _FakeSession) -> None: + monkeypatch.setattr( + "microsoft_agents.hosting.core.app.attachment_downloader.aiohttp.ClientSession", + lambda **kwargs: session, + ) + + class TestAttachmentDownloaderChannelAndEmptyCases: @pytest.mark.asyncio async def test_returns_empty_list_for_teams_channel(self): @@ -112,12 +126,13 @@ async def test_downloads_inline_content_as_json_bytes(self): class TestAttachmentDownloaderRemoteContent: @pytest.mark.asyncio - async def test_downloads_remote_file_and_returns_input_file(self): + async def test_downloads_remote_file_and_returns_input_file(self, monkeypatch): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) session = _FakeSession(response) - downloader = AttachmentDownloader(client_factory=lambda: session) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() attachment = Attachment( content_type="text/plain", content_url="https://example.org/file.txt", @@ -135,12 +150,13 @@ async def test_downloads_remote_file_and_returns_input_file(self): assert session.requested_urls == ["https://example.org/file.txt"] @pytest.mark.asyncio - async def test_allows_http_localhost_urls(self): + async def test_allows_http_localhost_urls(self, monkeypatch): response = _FakeResponse( status=200, content=b"local-bytes", content_type="text/plain" ) session = _FakeSession(response) - downloader = AttachmentDownloader(client_factory=lambda: session) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() attachment = Attachment( content_type="text/plain", content_url="http://localhost:3000/file.txt" ) @@ -158,27 +174,32 @@ async def test_allows_http_localhost_urls(self): "http://localhost@evil.example/file.txt", ], ) - async def test_does_not_request_spoofed_localhost_urls(self, content_url): + async def test_forwards_urls_without_a_host_validator( + self, monkeypatch, content_url + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) session = _FakeSession(response) - downloader = AttachmentDownloader(client_factory=lambda: session) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() context = _make_context( attachments=[Attachment(content_type="text/plain", content_url=content_url)] ) - await downloader.download_files(context) + files = await downloader.download_files(context) - assert session.requested_urls == [] + assert len(files) == 1 + assert session.requested_urls == [content_url] @pytest.mark.asyncio - async def test_accepts_partial_content_response(self): + async def test_accepts_partial_content_response(self, monkeypatch): response = _FakeResponse( status=206, content=b"partial-bytes", content_type="text/plain" ) session = _FakeSession(response) - downloader = AttachmentDownloader(client_factory=lambda: session) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() context = _make_context( attachments=[ Attachment( @@ -194,12 +215,13 @@ async def test_accepts_partial_content_response(self): assert files[0].content == b"partial-bytes" @pytest.mark.asyncio - async def test_normalizes_image_content_type_to_png(self): + async def test_normalizes_image_content_type_to_png(self, monkeypatch): response = _FakeResponse( status=200, content=b"\x89PNG", content_type="image/jpeg" ) session = _FakeSession(response) - downloader = AttachmentDownloader(client_factory=lambda: session) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() attachment = Attachment( content_type="image/jpeg", content_url="https://example.org/pic.jpg" ) @@ -210,10 +232,11 @@ async def test_normalizes_image_content_type_to_png(self): assert files[0].content_type == "image/png" @pytest.mark.asyncio - async def test_returns_none_for_non_success_status(self): + async def test_returns_none_for_non_success_status(self, monkeypatch): response = _FakeResponse(status=404, content=b"", content_type="text/plain") session = _FakeSession(response) - downloader = AttachmentDownloader(client_factory=lambda: session) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() attachment = Attachment( content_type="text/plain", content_url="https://example.org/missing.txt" ) @@ -222,15 +245,16 @@ async def test_returns_none_for_non_success_status(self): assert await downloader.download_files(context) == [] @pytest.mark.asyncio - async def test_skips_disallowed_hosts_when_host_validator_enabled(self): + async def test_skips_disallowed_hosts_when_host_validator_enabled( + self, monkeypatch + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) - session = _FakeSession(response) host_validator = OutboundHostValidator(enabled=True, hosts=["contoso.com"]) - downloader = AttachmentDownloader( - client_factory=lambda: session, host_validator=host_validator - ) + session = _FakeSession(response, validator=host_validator) + monkeypatch.setattr(host_validator, "client", lambda kwargs: session) + downloader = AttachmentDownloader(host_validator=host_validator) attachment = Attachment( content_type="text/plain", content_url="https://evil.example.com/file.txt" ) @@ -240,15 +264,16 @@ async def test_skips_disallowed_hosts_when_host_validator_enabled(self): assert session.requested_urls == [] @pytest.mark.asyncio - async def test_allows_permitted_hosts_when_host_validator_enabled(self): + async def test_allows_permitted_hosts_when_host_validator_enabled( + self, monkeypatch + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) - session = _FakeSession(response) host_validator = OutboundHostValidator(enabled=True, hosts=["contoso.com"]) - downloader = AttachmentDownloader( - client_factory=lambda: session, host_validator=host_validator - ) + session = _FakeSession(response, validator=host_validator) + monkeypatch.setattr(host_validator, "client", lambda kwargs: session) + downloader = AttachmentDownloader(host_validator=host_validator) attachment = Attachment( content_type="text/plain", content_url="https://contoso.com/file.txt" ) diff --git a/tests/hosting_core/app/test_m365_attachment_downloader.py b/tests/hosting_core/app/test_m365_attachment_downloader.py index f7abc6d78..0c719b9d4 100644 --- a/tests/hosting_core/app/test_m365_attachment_downloader.py +++ b/tests/hosting_core/app/test_m365_attachment_downloader.py @@ -21,7 +21,7 @@ from microsoft_agents.hosting.core.app.m365_attachment_downloader import ( M365AttachmentDownloader, ) -from microsoft_agents.hosting.core.outbound_host_validator import OutboundHostValidator +from microsoft_agents.hosting.core.security import OutboundHostValidator, _SSRFError class _FakeTokenProvider(AccessTokenProviderBase): @@ -121,11 +121,18 @@ async def __aexit__(self, *args) -> bool: class _FakeSession: - def __init__(self, response: _FakeResponse): + def __init__( + self, + response: _FakeResponse, + validator: OutboundHostValidator | None = None, + ): self._response = response + self._validator = validator self.requested_urls: list[str] = [] def get(self, url: str, headers=None, **kwargs) -> _FakeResponse: + if self._validator and not self._validator.is_allowed(url): + raise _SSRFError(f"URL '{url}' is not allowed") self.requested_urls.append(url) self._response.request_headers = headers return self._response @@ -137,6 +144,13 @@ async def __aexit__(self, *args) -> bool: return False +def _patch_client_session(monkeypatch, session: _FakeSession) -> None: + monkeypatch.setattr( + "microsoft_agents.hosting.core.app.m365_attachment_downloader.aiohttp.ClientSession", + lambda **kwargs: session, + ) + + class TestM365AttachmentDownloaderChannelAndValidation: @pytest.mark.asyncio async def test_raises_when_context_has_no_identity(self): @@ -203,14 +217,14 @@ async def test_returns_empty_list_for_non_teams_channel(self): assert await downloader.download_files(context) == [] @pytest.mark.asyncio - async def test_allows_m365_copilot_channel(self): + async def test_allows_m365_copilot_channel(self, monkeypatch): response = _FakeResponse( status=200, content=b"bytes", content_type="text/plain" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, ) context = _make_context( channel_id="msteams:COPILOT", @@ -268,14 +282,17 @@ async def test_downloads_inline_content_as_json_bytes(self): class TestM365AttachmentDownloaderRemoteContent: @pytest.mark.asyncio - async def test_downloads_remote_file_using_download_url_from_content(self): + async def test_downloads_remote_file_using_download_url_from_content( + self, monkeypatch + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) token_provider = _FakeTokenProvider("p") downloader = M365AttachmentDownloader( - connections=_FakeConnections(token_provider), client_factory=lambda: session + connections=_FakeConnections(token_provider) ) attachment = Attachment( content_type="text/plain", @@ -293,14 +310,16 @@ async def test_downloads_remote_file_using_download_url_from_content(self): assert session.requested_urls == ["https://example.org/real-download"] @pytest.mark.asyncio - async def test_falls_back_to_content_url_when_download_url_missing(self): + async def test_falls_back_to_content_url_when_download_url_missing( + self, monkeypatch + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, ) attachment = Attachment( content_type="text/plain", @@ -322,14 +341,16 @@ async def test_falls_back_to_content_url_when_download_url_missing(self): "http://localhost@evil.example/file.txt", ], ) - async def test_does_not_request_spoofed_localhost_download_urls(self, download_url): + async def test_forwards_download_urls_without_a_host_validator( + self, monkeypatch, download_url + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, ) attachment = Attachment( content_type="text/plain", @@ -338,19 +359,20 @@ async def test_does_not_request_spoofed_localhost_download_urls(self, download_u ) context = _make_context(attachments=[attachment]) - await downloader.download_files(context) + files = await downloader.download_files(context) - assert session.requested_urls == [] + assert len(files) == 1 + assert session.requested_urls == [download_url] @pytest.mark.asyncio - async def test_accepts_partial_content_response(self): + async def test_accepts_partial_content_response(self, monkeypatch): response = _FakeResponse( status=206, content=b"partial-bytes", content_type="text/plain" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, ) context = _make_context( attachments=[ @@ -367,14 +389,14 @@ async def test_accepts_partial_content_response(self): assert files[0].content == b"partial-bytes" @pytest.mark.asyncio - async def test_normalizes_image_content_type_to_png(self): + async def test_normalizes_image_content_type_to_png(self, monkeypatch): response = _FakeResponse( status=200, content=b"\x89PNG", content_type="image/jpeg" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, ) attachment = Attachment( content_type="image/jpeg", content_url="https://example.org/pic.jpg" @@ -386,12 +408,12 @@ async def test_normalizes_image_content_type_to_png(self): assert files[0].content_type == "image/png" @pytest.mark.asyncio - async def test_returns_none_entry_for_failed_status(self): + async def test_returns_none_entry_for_failed_status(self, monkeypatch): response = _FakeResponse(status=404, content=b"", content_type="text/plain") session = _FakeSession(response) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, ) attachment = Attachment( content_type="text/plain", content_url="https://example.org/missing.txt" @@ -401,15 +423,17 @@ async def test_returns_none_entry_for_failed_status(self): assert await downloader.download_files(context) == [] @pytest.mark.asyncio - async def test_skips_disallowed_hosts_when_host_validator_enabled(self): + async def test_skips_disallowed_hosts_when_host_validator_enabled( + self, monkeypatch + ): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) - session = _FakeSession(response) host_validator = OutboundHostValidator(enabled=True, hosts=["contoso.com"]) + session = _FakeSession(response, validator=host_validator) + monkeypatch.setattr(host_validator, "client", lambda kwargs: session) downloader = M365AttachmentDownloader( connections=_FakeConnections(_FakeTokenProvider("p")), - client_factory=lambda: session, host_validator=host_validator, ) attachment = Attachment( @@ -423,15 +447,15 @@ async def test_skips_disallowed_hosts_when_host_validator_enabled(self): assert session.requested_urls == [] @pytest.mark.asyncio - async def test_uses_anonymous_mode_without_requesting_token(self): + async def test_uses_anonymous_mode_without_requesting_token(self, monkeypatch): response = _FakeResponse( status=200, content=b"file-bytes", content_type="text/plain" ) session = _FakeSession(response) + _patch_client_session(monkeypatch, session) token_provider = _FakeTokenProvider("p") downloader = M365AttachmentDownloader( connections=_FakeConnections(token_provider), - client_factory=lambda: session, use_anonymous=True, ) attachment = Attachment( @@ -445,13 +469,14 @@ async def test_uses_anonymous_mode_without_requesting_token(self): assert token_provider.requested is None @pytest.mark.asyncio - async def test_uses_named_token_provider_when_configured(self): + async def test_uses_named_token_provider_when_configured(self, monkeypatch): named_provider = _FakeTokenProvider("named") + session = _FakeSession( + _FakeResponse(status=200, content=b"bytes", content_type="text/plain") + ) + _patch_client_session(monkeypatch, session) downloader = M365AttachmentDownloader( connections=_FakeConnections(named_provider), - client_factory=lambda: _FakeSession( - _FakeResponse(status=200, content=b"bytes", content_type="text/plain") - ), token_provider_name="named-connection", ) attachment = Attachment( diff --git a/tests/hosting_core/test_outbound_host_validator.py b/tests/hosting_core/test_outbound_host_validator.py index a0fc66988..ec5bed611 100644 --- a/tests/hosting_core/test_outbound_host_validator.py +++ b/tests/hosting_core/test_outbound_host_validator.py @@ -1,14 +1,47 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +import socket +from types import SimpleNamespace +from unittest.mock import AsyncMock, Mock + import pytest +from aiohttp import TCPConnector +from aiohttp.abc import ResolveResult from yarl import URL -from microsoft_agents.hosting.core.outbound_host_validator import ( - OutboundHostValidator, - _normalize, +from microsoft_agents.hosting.core.security._connector import ( + _SSRFConnector, + _SSRFError, +) +from microsoft_agents.hosting.core.security._middleware import ( + _validator_middleware, +) +from microsoft_agents.hosting.core.security._utils import ( + _normalize_host, _try_create_url, ) +from microsoft_agents.hosting.core.security.outbound_host_validator import ( + OutboundHostValidator, + _BasicResolveResultValidator, +) + + +def _resolve_result( + host: str, + *, + hostname: str = "example.com", + family: int = socket.AF_INET, + port: int = 443, +) -> ResolveResult: + return { + "hostname": hostname, + "host": host, + "port": port, + "family": family, + "proto": 0, + "flags": 0, + } class TestTryCreateUrl: @@ -22,12 +55,19 @@ def test_returns_existing_url(self): assert _try_create_url(url) is url - @pytest.mark.parametrize("url", ["http://[invalid", "https://example.com:invalid"]) + @pytest.mark.parametrize( + "url", + [ + "http://[invalid", + "https://example.com:invalid", + None, + ], + ) def test_returns_none_for_invalid_url(self, url): assert _try_create_url(url) is None -class TestNormalize: +class TestNormalizeHost: @pytest.mark.parametrize( ("host", "expected"), [ @@ -40,11 +80,65 @@ class TestNormalize: ], ) def test_normalizes_host(self, host, expected): - assert _normalize(host) == expected + assert _normalize_host(host) == expected @pytest.mark.parametrize("host", ["", " "]) def test_returns_none_for_empty_normalized_host(self, host): - assert _normalize(host) is None + assert _normalize_host(host) is None + + +class TestBasicResolveResultValidator: + @pytest.mark.parametrize( + "host", + [ + "8.8.8.8", + "2606:4700:4700::1111", + ], + ) + def test_allows_global_addresses(self, host): + validator = _BasicResolveResultValidator() + + assert validator.is_allowed(_resolve_result(host)) is True + + @pytest.mark.parametrize( + "host", + [ + "10.0.0.1", + "127.0.0.1", + "169.254.169.254", + "::1", + "fe80::1", + ], + ) + def test_denies_non_global_addresses_by_default(self, host): + validator = _BasicResolveResultValidator() + + assert validator.is_allowed(_resolve_result(host)) is False + + @pytest.mark.parametrize("host", ["10.0.0.1", "fd00::1"]) + def test_allows_private_addresses_when_configured(self, host): + validator = _BasicResolveResultValidator(allow_private_network_addresses=True) + + assert validator.is_allowed(_resolve_result(host)) is True + + @pytest.mark.parametrize( + "resolved", + [ + _resolve_result("not-an-ip"), + { + "hostname": "example.com", + "host": "", + "port": 443, + "family": socket.AF_INET, + "proto": 0, + "flags": 0, + }, + ], + ) + def test_denies_invalid_resolved_results(self, resolved): + validator = _BasicResolveResultValidator() + + assert validator.is_allowed(resolved) is False class TestOutboundHostValidator: @@ -70,6 +164,26 @@ def test_default_options_disable_enforcement(self): assert validator.enabled is False assert validator.is_allowed("https://evil.example.com/relay") is True + def test_explicitly_disabled_with_hosts_remains_disabled(self): + validator = OutboundHostValidator( + hosts=["contoso.com"], + enabled=False, + include_default_microsoft_hosts=False, + ) + + assert validator.enabled is False + assert validator.is_allowed("https://evil.example.com/relay") is True + + def test_hosts_enable_enforcement_when_enabled_is_unspecified(self): + validator = OutboundHostValidator( + hosts=["contoso.com"], + include_default_microsoft_hosts=False, + ) + + assert validator.enabled is True + assert validator.is_allowed("https://contoso.com/api") is True + assert validator.is_allowed("https://evil.example.com/api") is False + @pytest.mark.parametrize( "url", [ @@ -84,7 +198,6 @@ def test_default_options_disable_enforcement(self): def test_enabled_allows_first_party_microsoft_hosts(self, url): validator = OutboundHostValidator(enabled=True) - assert validator.enabled is True assert validator.is_allowed(url) is True @pytest.mark.parametrize( @@ -96,9 +209,11 @@ def test_enabled_allows_first_party_microsoft_hosts(self, url): "http://localhost/admin", "https://localhost/admin", "https://evil.trafficmanager.net/relay", + "ftp://graph.microsoft.com/file", + None, ], ) - def test_enabled_denies_unknown_hosts(self, url): + def test_enabled_denies_unknown_or_invalid_urls(self, url): validator = OutboundHostValidator(enabled=True) assert validator.is_allowed(url) is False @@ -110,31 +225,31 @@ def test_enabled_denies_unknown_hosts(self, url): "https://contoso.com/some/path", "contoso.com:8443", "contoso.com/path", + "*.contoso.com", ], ) - def test_enabled_normalizes_configured_host(self, configured_host): - validator = OutboundHostValidator(enabled=True, hosts=[configured_host]) + def test_normalizes_configured_host(self, configured_host): + validator = OutboundHostValidator( + hosts=[configured_host], + include_default_microsoft_hosts=False, + ) assert validator.is_allowed("https://contoso.com/api") is True assert validator.is_allowed("https://files.contoso.com/api") is True - def test_enabled_allows_configured_host_exact_and_subdomain(self): - validator = OutboundHostValidator(enabled=True, hosts=["contoso.com"]) + def test_allows_configured_host_exact_and_subdomain(self): + validator = OutboundHostValidator( + hosts=["contoso.com"], + include_default_microsoft_hosts=False, + ) assert validator.is_allowed("https://contoso.com/api") is True assert validator.is_allowed("https://files.contoso.com/api") is True assert validator.is_allowed("https://notcontoso.com/api") is False assert validator.is_allowed("https://contoso.com.evil.com/api") is False - def test_enabled_accepts_wildcard_prefix_in_configured_host(self): - validator = OutboundHostValidator(enabled=True, hosts=["*.fabrikam.com"]) - - assert validator.is_allowed("https://api.fabrikam.com/x") is True - assert validator.is_allowed("https://fabrikam.com/x") is True - - def test_enabled_without_defaults_denies_microsoft_hosts(self): + def test_can_exclude_default_microsoft_hosts(self): validator = OutboundHostValidator( - enabled=True, hosts=["contoso.com"], include_default_microsoft_hosts=False, ) @@ -142,27 +257,13 @@ def test_enabled_without_defaults_denies_microsoft_hosts(self): assert validator.is_allowed("https://graph.microsoft.com/v1.0/me") is False assert validator.is_allowed("https://contoso.com/x") is True - def test_enabled_host_match_is_case_insensitive(self): + def test_host_match_is_case_insensitive(self): validator = OutboundHostValidator(enabled=True) assert validator.is_allowed("https://GRAPH.MICROSOFT.COM/v1.0/me") is True - @pytest.mark.parametrize( - "url", - [ - "not-a-uri", - "/relative/path", - None, - ], - ) - def test_enabled_denies_non_absolute_or_invalid_urls(self, url): - validator = OutboundHostValidator(enabled=True) - - assert validator.is_allowed(url) is False - def test_accepts_url_object(self): validator = OutboundHostValidator( - enabled=True, hosts=["example.com"], include_default_microsoft_hosts=False, ) @@ -171,9 +272,156 @@ def test_accepts_url_object(self): def test_rejects_userinfo_host_confusion(self): validator = OutboundHostValidator( - enabled=True, hosts=["example.com"], include_default_microsoft_hosts=False, ) assert validator.is_allowed("https://example.com@evil.example/path") is False + + def test_private_ip_literal_requires_both_host_and_private_network_opt_in(self): + validator = OutboundHostValidator( + hosts=["10.0.0.1"], + include_default_microsoft_hosts=False, + allow_private_network_addresses=True, + ) + + assert validator.is_allowed("https://10.0.0.1/file") is True + assert validator.is_allowed("https://10.0.0.2/file") is False + + @pytest.mark.asyncio + async def test_client_uses_ssrf_connector_when_enabled(self): + validator = OutboundHostValidator( + hosts=["example.com"], + include_default_microsoft_hosts=False, + ) + + client = validator.client({"headers": {"X-Test": "value"}}) + try: + assert isinstance(client.connector, _SSRFConnector) + assert client.headers["X-Test"] == "value" + assert len(client._middlewares) == 1 + finally: + await client.close() + + @pytest.mark.asyncio + async def test_client_uses_standard_connector_when_disabled(self): + validator = OutboundHostValidator(enabled=False) + + client = validator.client() + try: + assert isinstance(client.connector, TCPConnector) + assert not isinstance(client.connector, _SSRFConnector) + assert client._middlewares == () + finally: + await client.close() + + @pytest.mark.parametrize( + "option", + [ + {"connector": Mock()}, + {"connector_owner": False}, + {"middlewares": []}, + ], + ) + def test_client_rejects_security_sensitive_session_options(self, option): + validator = OutboundHostValidator(enabled=True) + + with pytest.raises(ValueError): + validator.client(option) + + +class TestValidatorMiddleware: + @pytest.mark.asyncio + async def test_calls_handler_when_url_is_allowed(self): + validator = Mock() + validator.is_allowed.return_value = True + request = SimpleNamespace(url=URL("https://example.com/file")) + response = object() + handler = AsyncMock(return_value=response) + + result = await _validator_middleware(validator)(request, handler) + + assert result is response + validator.is_allowed.assert_called_once_with(request.url) + handler.assert_awaited_once_with(request) + + @pytest.mark.asyncio + async def test_raises_ssrf_error_without_calling_handler_when_url_is_denied(self): + validator = Mock() + validator.is_allowed.return_value = False + request = SimpleNamespace(url=URL("https://evil.example/file")) + handler = AsyncMock() + + with pytest.raises(_SSRFError, match="evil.example"): + await _validator_middleware(validator)(request, handler) + + handler.assert_not_awaited() + + +class TestSSRFConnector: + @pytest.mark.asyncio + async def test_returns_all_results_when_validator_allows_them(self, monkeypatch): + resolved = [ + _resolve_result("8.8.8.8"), + _resolve_result( + "2606:4700:4700::1111", + family=socket.AF_INET6, + ), + ] + traces = [Mock()] + resolve_host = AsyncMock(return_value=resolved) + monkeypatch.setattr(TCPConnector, "_resolve_host", resolve_host) + validator = Mock() + validator.is_allowed.return_value = True + connector = _SSRFConnector(validator) + + try: + result = await connector._resolve_host("example.com", 443, traces) + finally: + await connector.close() + + assert result is resolved + resolve_host.assert_awaited_once_with("example.com", 443, traces) + assert validator.is_allowed.call_count == 2 + validator.is_allowed.assert_any_call(resolved[0]) + validator.is_allowed.assert_any_call(resolved[1]) + + @pytest.mark.asyncio + async def test_raises_when_any_resolved_result_is_denied(self, monkeypatch): + resolved = [ + _resolve_result("8.8.8.8"), + _resolve_result("127.0.0.1"), + ] + monkeypatch.setattr( + TCPConnector, + "_resolve_host", + AsyncMock(return_value=resolved), + ) + validator = Mock() + validator.is_allowed.side_effect = [True, False] + connector = _SSRFConnector(validator) + + try: + with pytest.raises(_SSRFError, match="127.0.0.1"): + await connector._resolve_host("example.com", 443) + finally: + await connector.close() + + assert validator.is_allowed.call_count == 2 + + @pytest.mark.asyncio + async def test_returns_empty_result_without_validation(self, monkeypatch): + monkeypatch.setattr( + TCPConnector, + "_resolve_host", + AsyncMock(return_value=[]), + ) + validator = Mock() + connector = _SSRFConnector(validator) + + try: + assert await connector._resolve_host("example.com", 443) == [] + finally: + await connector.close() + + validator.is_allowed.assert_not_called() From ddb33925e8972ab7bcac4b3f33ae9d9b6ca4144a Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Mon, 5 Oct 2026 11:26:35 -0700 Subject: [PATCH 07/15] Enhanced multicast handling and documentation --- .../core/security/outbound_host_validator.py | 27 ++++++++++++------- .../app/test_attachment_downloader.py | 10 ++++--- .../app/test_m365_attachment_downloader.py | 4 +-- .../test_outbound_host_validator.py | 1 + 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py index 65ceeba02..e054a727d 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py @@ -30,6 +30,19 @@ ] +def _allow_addr( + address: ipaddress.IPv4Address | ipaddress.IPv6Address, + allow_private_network_addresses: bool, +) -> bool: + if address.is_multicast: + return False + if address.is_global: + return True + if address.is_private and allow_private_network_addresses: + return True + return False + + class _BasicResolveResultValidator(_ResolveResultValidator): """Validates resolved network addresses used by the SSRF-safe connector.""" @@ -45,10 +58,7 @@ def is_allowed(self, resolved: ResolveResult) -> bool: try: address = ipaddress.ip_address(host) - if not ( - address.is_global - or (address.is_private and self._allow_private_network_addresses) - ): + if not _allow_addr(address, self._allow_private_network_addresses): logger.warning("Private or loopback network address not allowed") return False @@ -163,10 +173,10 @@ def _is_allowed(self, _url: str | URL) -> bool: if url.host is None: return False - # Only allow HTTP and HTTPS schemes - if url.scheme not in ("http", "https"): + if url.scheme != "https": return False + # disallow user info in the URL (scheme://user:password@localhost) if url.user or url.password: return False @@ -175,10 +185,7 @@ def _is_allowed(self, _url: str | URL) -> bool: try: address = ipaddress.ip_address(url.host) - if not ( - address.is_global - or (address.is_private and self._allow_private_network_addresses) - ): + if not _allow_addr(address, self._allow_private_network_addresses): return False except ValueError: diff --git a/tests/hosting_core/app/test_attachment_downloader.py b/tests/hosting_core/app/test_attachment_downloader.py index 05e596919..e7511f151 100644 --- a/tests/hosting_core/app/test_attachment_downloader.py +++ b/tests/hosting_core/app/test_attachment_downloader.py @@ -150,7 +150,9 @@ async def test_downloads_remote_file_and_returns_input_file(self, monkeypatch): assert session.requested_urls == ["https://example.org/file.txt"] @pytest.mark.asyncio - async def test_allows_http_localhost_urls(self, monkeypatch): + async def test_downloads_https_localhost_urls_without_a_validator( + self, monkeypatch + ): response = _FakeResponse( status=200, content=b"local-bytes", content_type="text/plain" ) @@ -158,7 +160,7 @@ async def test_allows_http_localhost_urls(self, monkeypatch): _patch_client_session(monkeypatch, session) downloader = AttachmentDownloader() attachment = Attachment( - content_type="text/plain", content_url="http://localhost:3000/file.txt" + content_type="text/plain", content_url="https://localhost:3000/file.txt" ) context = _make_context(attachments=[attachment]) @@ -170,8 +172,8 @@ async def test_allows_http_localhost_urls(self, monkeypatch): @pytest.mark.parametrize( "content_url", [ - "http://localhost.evil.example/file.txt", - "http://localhost@evil.example/file.txt", + "https://localhost.evil.example/file.txt", + "https://localhost@evil.example/file.txt", ], ) async def test_forwards_urls_without_a_host_validator( diff --git a/tests/hosting_core/app/test_m365_attachment_downloader.py b/tests/hosting_core/app/test_m365_attachment_downloader.py index 0c719b9d4..bec13cc20 100644 --- a/tests/hosting_core/app/test_m365_attachment_downloader.py +++ b/tests/hosting_core/app/test_m365_attachment_downloader.py @@ -337,8 +337,8 @@ async def test_falls_back_to_content_url_when_download_url_missing( @pytest.mark.parametrize( "download_url", [ - "http://localhost.evil.example/file.txt", - "http://localhost@evil.example/file.txt", + "https://localhost.evil.example/file.txt", + "https://localhost@evil.example/file.txt", ], ) async def test_forwards_download_urls_without_a_host_validator( diff --git a/tests/hosting_core/test_outbound_host_validator.py b/tests/hosting_core/test_outbound_host_validator.py index ec5bed611..44ff5f3af 100644 --- a/tests/hosting_core/test_outbound_host_validator.py +++ b/tests/hosting_core/test_outbound_host_validator.py @@ -207,6 +207,7 @@ def test_enabled_allows_first_party_microsoft_hosts(self, url): "https://169.254.169.254/latest/meta-data", "https://internal-test.local:8443/secret", "http://localhost/admin", + "http://graph.microsoft.com/v1.0/me", "https://localhost/admin", "https://evil.trafficmanager.net/relay", "ftp://graph.microsoft.com/file", From 2ee65c4b13ddd69c62c332d1602936ceb2d8d793 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Mon, 5 Oct 2026 11:34:37 -0700 Subject: [PATCH 08/15] Updating changelog --- changelog.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/changelog.md b/changelog.md index f7ca38055..232ec4c62 100644 --- a/changelog.md +++ b/changelog.md @@ -1,3 +1,18 @@ +# Microsoft 365 Agents SDK for Python - Release Notes v1.9.0 (Unreleased) + +**Release Date:** Unreleased +**Previous Version:** 1.8.0 (Released 2026-10-01) + +## Major Features & Enhancements + +- **Outbound Host Validation Hardening**: Enhanced `OutboundHostValidator` with HTTPS-only URL enforcement, DNS resolution validation for public and optionally private network addresses, and an SSRF-aware `aiohttp` connector that validates resolved IPv4 and IPv6 addresses before connecting. + +## Breaking Changes + +- **Attachment Downloader HTTP Configuration**: Replaced custom `client_factory` injection with `client_session_kwargs`; the deprecated `client_factory` parameter is ignored. This ensures attachment requests always use the validator-managed middleware and connector, preventing custom client sessions from bypassing hostname allowlists, redirect validation, or resolved-address checks while still allowing supported `aiohttp.ClientSession` options to be configured. + +--- + # Microsoft 365 Agents SDK for Python - Release Notes v1.8.0 **Release Date:** 2026-10-01 From 3af1f8386e29a9e0fdca9ce129bdb844717a2c34 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Mon, 5 Oct 2026 12:33:25 -0700 Subject: [PATCH 09/15] Addressing PR feedback --- .../microsoft-agents-hosting-aiohttp/setup.py | 2 +- .../hosting/core/app/attachment_downloader.py | 4 +- .../core/app/m365_attachment_downloader.py | 2 +- .../hosting/core/security/_utils.py | 5 -- .../core/security/outbound_host_validator.py | 43 +++++++++++--- .../microsoft-agents-hosting-core/setup.py | 2 +- .../microsoft-agents-hosting-msteams/setup.py | 2 +- .../microsoft-agents-hosting-slack/setup.py | 2 +- .../microsoft-agents-hosting-teams/setup.py | 2 +- .../app/test_attachment_downloader.py | 39 +++++++++++-- .../app/test_m365_attachment_downloader.py | 57 +++++++++++++++++++ .../test_outbound_host_validator.py | 36 ++++++++++-- 12 files changed, 166 insertions(+), 30 deletions(-) diff --git a/libraries/microsoft-agents-hosting-aiohttp/setup.py b/libraries/microsoft-agents-hosting-aiohttp/setup.py index f7a4e1bbb..88b58e29d 100644 --- a/libraries/microsoft-agents-hosting-aiohttp/setup.py +++ b/libraries/microsoft-agents-hosting-aiohttp/setup.py @@ -13,6 +13,6 @@ version=package_version, install_requires=[ f"microsoft-agents-hosting-core=={package_version}", - "aiohttp>=3.11.11", + "aiohttp>=3.12", ], ) diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py index 3739e6b45..a2ae9ea19 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py @@ -20,7 +20,7 @@ ) from .input_file import InputFileDownloader, InputFile -from ._utils import _parse_content_type +from ._utils import _parse_content_type, _basic_url_check logger = logging.getLogger(__name__) @@ -88,7 +88,7 @@ async def _download_file( :param attachment: The attachment to download. :return: An InputFile instance if the download is successful, None otherwise. """ - if attachment.content_url: + if attachment.content_url and _basic_url_check(attachment.content_url): remote_file_url = attachment.content_url try: diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py index c9041fb2f..0cf1add33 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py @@ -159,7 +159,7 @@ async def _download_file( else: download_url = attachment.content_url - if download_url: + if download_url and _basic_url_check(download_url): try: async with client.get( download_url, diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py index 73fe69abe..29b460f34 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py @@ -3,11 +3,6 @@ from __future__ import annotations -from aiohttp import TCPConnector -from aiohttp.abc import ResolveResult - -import ipaddress - from yarl import URL diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py index e054a727d..75201c13b 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py @@ -29,6 +29,30 @@ "blob.core.windows.net", # Azure Blob Storage / Attachment Management Service ] +_ALLOWED_CLIENT_SESSION_KWARGS = frozenset( + { + "auth", + "auto_decompress", + "base_url", + "cookie_jar", + "cookies", + "fallback_charset_resolver", + "headers", + "json_serialize", + "max_field_size", + "max_line_size", + "raise_for_status", + "read_bufsize", + "requote_redirect_url", + "response_class", + "skip_auto_headers", + "timeout", + "trace_configs", + "version", + "ws_response_class", + } +) + def _allow_addr( address: ipaddress.IPv4Address | ipaddress.IPv6Address, @@ -120,27 +144,28 @@ def enabled(self) -> bool: def client(self, client_session_kwargs: dict | None = None) -> ClientSession: """Creates a new client session with the outbound host validator applied. - :param client_session_kwargs: Optional keyword arguments to pass to the ClientSession constructor. + + :param client_session_kwargs: Optional allowlisted keyword arguments to pass + to the ClientSession constructor. :return: A new client session with the outbound host validator applied. """ - if "connector" in (client_session_kwargs or {}): - raise ValueError("Specifying a custom connector is not allowed.") - if "connector_owner" in (client_session_kwargs or {}): - raise ValueError("Specifying a connector_owner value is not allowed.") - if "middlewares" in (client_session_kwargs or {}): - raise ValueError("Specifying custom middlewares is not allowed.") + kwargs = client_session_kwargs or {} + unsupported_options = set(kwargs) - _ALLOWED_CLIENT_SESSION_KWARGS + if unsupported_options: + options = ", ".join(sorted(unsupported_options)) + raise ValueError(f"Unsupported ClientSession options: {options}.") middleware = [_validator_middleware(self)] if not self._enabled: - return ClientSession(**(client_session_kwargs or {})) + return ClientSession(**kwargs) return ClientSession( connector=_SSRFConnector(self._resolved_validator), connector_owner=True, middlewares=middleware, - **(client_session_kwargs or {}), + **kwargs, ) def is_allowed(self, url: str | URL) -> bool: diff --git a/libraries/microsoft-agents-hosting-core/setup.py b/libraries/microsoft-agents-hosting-core/setup.py index 56079d3ee..fa8450d45 100644 --- a/libraries/microsoft-agents-hosting-core/setup.py +++ b/libraries/microsoft-agents-hosting-core/setup.py @@ -18,7 +18,7 @@ "python-dotenv>=1.1.1", "opentelemetry-api>=1.27.0", "opentelemetry-sdk>=1.27.0", - "aiohttp>=3.11.11", + "aiohttp>=3.12", "yarl>=1.17.0,<2.0", "azure-core", ], diff --git a/libraries/microsoft-agents-hosting-msteams/setup.py b/libraries/microsoft-agents-hosting-msteams/setup.py index c3cb4c4fa..668348f94 100644 --- a/libraries/microsoft-agents-hosting-msteams/setup.py +++ b/libraries/microsoft-agents-hosting-msteams/setup.py @@ -13,7 +13,7 @@ version=package_version, install_requires=[ f"microsoft-agents-hosting-core=={package_version}", - "aiohttp>=3.11.11", + "aiohttp>=3.12", "microsoft-teams-api==2.1.0", "msgraph-sdk>=1.58.0", ], diff --git a/libraries/microsoft-agents-hosting-slack/setup.py b/libraries/microsoft-agents-hosting-slack/setup.py index f7a4e1bbb..88b58e29d 100644 --- a/libraries/microsoft-agents-hosting-slack/setup.py +++ b/libraries/microsoft-agents-hosting-slack/setup.py @@ -13,6 +13,6 @@ version=package_version, install_requires=[ f"microsoft-agents-hosting-core=={package_version}", - "aiohttp>=3.11.11", + "aiohttp>=3.12", ], ) diff --git a/libraries/microsoft-agents-hosting-teams/setup.py b/libraries/microsoft-agents-hosting-teams/setup.py index 62611d341..ce37c4139 100644 --- a/libraries/microsoft-agents-hosting-teams/setup.py +++ b/libraries/microsoft-agents-hosting-teams/setup.py @@ -13,7 +13,7 @@ version=package_version, install_requires=[ f"microsoft-agents-hosting-core=={package_version}", - "aiohttp>=3.11.11", + "aiohttp>=3.12", "microsoft-teams-api>=2.0.0,<3", ], ) diff --git a/tests/hosting_core/app/test_attachment_downloader.py b/tests/hosting_core/app/test_attachment_downloader.py index e7511f151..d9ccdcc8a 100644 --- a/tests/hosting_core/app/test_attachment_downloader.py +++ b/tests/hosting_core/app/test_attachment_downloader.py @@ -150,9 +150,7 @@ async def test_downloads_remote_file_and_returns_input_file(self, monkeypatch): assert session.requested_urls == ["https://example.org/file.txt"] @pytest.mark.asyncio - async def test_downloads_https_localhost_urls_without_a_validator( - self, monkeypatch - ): + async def test_allows_http_localhost_urls_without_a_validator(self, monkeypatch): response = _FakeResponse( status=200, content=b"local-bytes", content_type="text/plain" ) @@ -160,13 +158,46 @@ async def test_downloads_https_localhost_urls_without_a_validator( _patch_client_session(monkeypatch, session) downloader = AttachmentDownloader() attachment = Attachment( - content_type="text/plain", content_url="https://localhost:3000/file.txt" + content_type="text/plain", content_url="http://localhost:3000/file.txt" + ) + context = _make_context(attachments=[attachment]) + + files = await downloader.download_files(context) + + assert len(files) == 1 + assert session.requested_urls == ["http://localhost:3000/file.txt"] + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "content_url", + [ + "http://example.org/file.txt", + "ftp://example.org/file.txt", + "/relative/file.txt", + ], + ) + async def test_does_not_request_urls_that_fail_basic_validation( + self, monkeypatch, content_url + ): + response = _FakeResponse( + status=200, content=b"remote-bytes", content_type="text/plain" + ) + session = _FakeSession(response) + _patch_client_session(monkeypatch, session) + downloader = AttachmentDownloader() + attachment = Attachment( + content_type="application/vnd.custom", + content={"source": "inline"}, + content_url=content_url, + name="data.json", ) context = _make_context(attachments=[attachment]) files = await downloader.download_files(context) assert len(files) == 1 + assert files[0].content == bytes(json.dumps({"source": "inline"}), "utf-8") + assert session.requested_urls == [] @pytest.mark.asyncio @pytest.mark.parametrize( diff --git a/tests/hosting_core/app/test_m365_attachment_downloader.py b/tests/hosting_core/app/test_m365_attachment_downloader.py index bec13cc20..32c1d43d9 100644 --- a/tests/hosting_core/app/test_m365_attachment_downloader.py +++ b/tests/hosting_core/app/test_m365_attachment_downloader.py @@ -333,6 +333,63 @@ async def test_falls_back_to_content_url_when_download_url_missing( assert len(files) == 1 assert session.requested_urls == ["https://example.org/file.txt"] + @pytest.mark.asyncio + async def test_allows_http_localhost_download_urls(self, monkeypatch): + response = _FakeResponse( + status=200, content=b"local-bytes", content_type="text/plain" + ) + session = _FakeSession(response) + _patch_client_session(monkeypatch, session) + downloader = M365AttachmentDownloader( + connections=_FakeConnections(_FakeTokenProvider("p")), + ) + attachment = Attachment( + content_type="text/plain", + content_url="https://example.org/file.txt", + content={"downloadUrl": "http://localhost:3000/file.txt"}, + ) + context = _make_context(attachments=[attachment]) + + files = await downloader.download_files(context) + + assert len(files) == 1 + assert session.requested_urls == ["http://localhost:3000/file.txt"] + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "download_url", + [ + "http://example.org/file.txt", + "ftp://example.org/file.txt", + "/relative/file.txt", + ], + ) + async def test_does_not_request_urls_that_fail_basic_validation( + self, monkeypatch, download_url + ): + response = _FakeResponse( + status=200, content=b"remote-bytes", content_type="text/plain" + ) + session = _FakeSession(response) + _patch_client_session(monkeypatch, session) + downloader = M365AttachmentDownloader( + connections=_FakeConnections(_FakeTokenProvider("p")), + ) + content = {"downloadUrl": download_url, "source": "inline"} + attachment = Attachment( + content_type="application/vnd.custom", + content_url="https://example.org/file.txt", + content=content, + name="data.json", + ) + context = _make_context(attachments=[attachment]) + + files = await downloader.download_files(context) + + assert len(files) == 1 + assert files[0].content == bytes(json.dumps(content), "utf-8") + assert session.requested_urls == [] + @pytest.mark.asyncio @pytest.mark.parametrize( "download_url", diff --git a/tests/hosting_core/test_outbound_host_validator.py b/tests/hosting_core/test_outbound_host_validator.py index 44ff5f3af..3881151aa 100644 --- a/tests/hosting_core/test_outbound_host_validator.py +++ b/tests/hosting_core/test_outbound_host_validator.py @@ -6,7 +6,7 @@ from unittest.mock import AsyncMock, Mock import pytest -from aiohttp import TCPConnector +from aiohttp import ClientTimeout, TCPConnector from aiohttp.abc import ResolveResult from yarl import URL @@ -296,10 +296,19 @@ async def test_client_uses_ssrf_connector_when_enabled(self): include_default_microsoft_hosts=False, ) - client = validator.client({"headers": {"X-Test": "value"}}) + timeout = ClientTimeout(total=30) + client = validator.client( + { + "headers": {"X-Test": "value"}, + "raise_for_status": True, + "timeout": timeout, + } + ) try: assert isinstance(client.connector, _SSRFConnector) assert client.headers["X-Test"] == "value" + assert client.raise_for_status is True + assert client.timeout is timeout assert len(client._middlewares) == 1 finally: await client.close() @@ -322,14 +331,33 @@ async def test_client_uses_standard_connector_when_disabled(self): {"connector": Mock()}, {"connector_owner": False}, {"middlewares": []}, + {"proxy": "http://proxy.example"}, + {"proxy_auth": Mock()}, + {"trust_env": True}, + {"request_class": Mock()}, + {"loop": Mock()}, + {"unknown_option": True}, ], ) - def test_client_rejects_security_sensitive_session_options(self, option): + def test_client_rejects_non_allowlisted_session_options(self, option): validator = OutboundHostValidator(enabled=True) - with pytest.raises(ValueError): + option_name = next(iter(option)) + with pytest.raises( + ValueError, + match=f"Unsupported ClientSession options: {option_name}", + ): validator.client(option) + def test_client_reports_all_non_allowlisted_session_options(self): + validator = OutboundHostValidator(enabled=True) + + with pytest.raises( + ValueError, + match="Unsupported ClientSession options: proxy, trust_env", + ): + validator.client({"trust_env": True, "proxy": "http://proxy.example"}) + class TestValidatorMiddleware: @pytest.mark.asyncio From cf1ad211f4dc02fb3c69bd8a77d3c5b354e917f4 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Mon, 5 Oct 2026 13:38:55 -0700 Subject: [PATCH 10/15] ADdressing PR feedback --- .../hosting/core/app/m365_attachment_downloader.py | 9 ++++++--- .../hosting/core/security/_middleware.py | 4 +--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py index 0cf1add33..8ed2e9621 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py @@ -3,6 +3,8 @@ import json import logging +import warnings + from typing import Callable, cast, Any import aiohttp @@ -49,13 +51,14 @@ def __init__( :param token_provider_name: The name of the token provider. :param use_anonymous: Whether to use anonymous access. :param scopes: A list of scopes for the access token. - :param connections: A Connections instance. :param client_factory: (deprecated) A custom client factory, if any. """ if client_factory is not None: - logger.warning( - "The 'client_factory' parameter is deprecated and will be ignored." + warnings.warn( + "The 'client_factory' parameter is deprecated and will be ignored.", + DeprecationWarning, + stacklevel=2, ) self._connections = connections diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py index 3b4a88ae6..1c14c8912 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_middleware.py @@ -24,9 +24,7 @@ async def _middleware( """Middleware that validates outbound URLs using the given host validator.""" if not validator.is_allowed(req.url): - raise _SSRFError( - f"URL '{req.url}' is not allowed by the outbound host validator." - ) + raise _SSRFError(f"URL is not allowed by the outbound host validator.") return await handler(req) From f86502121604877bf83292b94f47f003487c5eb4 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Mon, 5 Oct 2026 15:38:13 -0700 Subject: [PATCH 11/15] Fixing test --- tests/hosting_core/test_outbound_host_validator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/hosting_core/test_outbound_host_validator.py b/tests/hosting_core/test_outbound_host_validator.py index 3881151aa..a992e3059 100644 --- a/tests/hosting_core/test_outbound_host_validator.py +++ b/tests/hosting_core/test_outbound_host_validator.py @@ -381,7 +381,7 @@ async def test_raises_ssrf_error_without_calling_handler_when_url_is_denied(self request = SimpleNamespace(url=URL("https://evil.example/file")) handler = AsyncMock() - with pytest.raises(_SSRFError, match="evil.example"): + with pytest.raises(_SSRFError, match="URL is not allowed by the outbound host validator"): await _validator_middleware(validator)(request, handler) handler.assert_not_awaited() From f9300695bd649bee9474e7ccc706ba4a6d1ff74f Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Tue, 6 Oct 2026 11:40:09 -0700 Subject: [PATCH 12/15] Handling misuse of ipaddress is_private property and improving ip address normalization for allow list --- .../hosting/core/security/_utils.py | 29 +++++++++++++++++++ .../core/security/outbound_host_validator.py | 20 ++++++++++++- .../test_outbound_host_validator.py | 28 +++++++++++++++++- 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py index 29b460f34..271a22f7a 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/_utils.py @@ -3,6 +3,8 @@ from __future__ import annotations +import ipaddress + from yarl import URL @@ -18,6 +20,29 @@ def _try_create_url(url: str | URL) -> URL | None: return None +def _normalize_ip_address(address: str) -> str | None: + """Check for potential ip address and normalize + + :param address: The IP address string to normalize. + :return: The normalized IP address string if valid, None otherwise. + """ + + right_bracket = -1 + if address.startswith("["): + for i, c in enumerate(address[::-1]): + if c == "]": + right_bracket = len(address) - i - 1 + break + + if right_bracket >= 0: + address = address[1:right_bracket] + + try: + return str(ipaddress.ip_address(address)) + except ValueError: + return None + + def _normalize_host(host: str) -> str | None: """Normalizes the given host string. @@ -29,6 +54,10 @@ def _normalize_host(host: str) -> str | None: host = host.strip().casefold() + ip_address = _normalize_ip_address(host) + if ip_address is not None: + return ip_address + if host.startswith("*."): host = host[2:] diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py index 75201c13b..2187cbac7 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/security/outbound_host_validator.py @@ -58,8 +58,21 @@ def _allow_addr( address: ipaddress.IPv4Address | ipaddress.IPv6Address, allow_private_network_addresses: bool, ) -> bool: - if address.is_multicast: + """Allows or denies an IP address based on network address rules. + + :param address: The IP address to check. + :param allow_private_network_addresses: Whether to allow private network addresses. + :return: True if the address is allowed, False otherwise. + """ + if ( + address.is_multicast + or address.is_loopback + or address.is_link_local + or address.is_unspecified + or address.is_reserved + ): return False + if address.is_global: return True if address.is_private and allow_private_network_addresses: @@ -74,6 +87,11 @@ def __init__(self, *, allow_private_network_addresses: bool = False): self._allow_private_network_addresses = allow_private_network_addresses def is_allowed(self, resolved: ResolveResult) -> bool: + """Checks if the resolved network address is allowed. + + :param resolved: The resolved network address result. + :return: True if the address is allowed, False otherwise. + """ host = resolved.get("host") diff --git a/tests/hosting_core/test_outbound_host_validator.py b/tests/hosting_core/test_outbound_host_validator.py index a992e3059..1577c55f7 100644 --- a/tests/hosting_core/test_outbound_host_validator.py +++ b/tests/hosting_core/test_outbound_host_validator.py @@ -77,6 +77,14 @@ class TestNormalizeHost: ("example.com:443", "example.com"), ("example.com/path", "example.com"), ("https://Example.COM:443/path", "example.com"), + ("2606:4700:4700::1111", "2606:4700:4700::1111"), + (" 2606:4700:4700::1111 ", "2606:4700:4700::1111"), + ("[2606:4700:4700::1111]", "2606:4700:4700::1111"), + ("[2606:4700:4700::1111]:443", "2606:4700:4700::1111"), + ( + "https://[2606:4700:4700::1111]:443/path", + "2606:4700:4700::1111", + ), ], ) def test_normalizes_host(self, host, expected): @@ -121,6 +129,22 @@ def test_allows_private_addresses_when_configured(self, host): assert validator.is_allowed(_resolve_result(host)) is True + @pytest.mark.parametrize( + "host", + [ + "127.0.0.1", + "::1", + "169.254.169.254", + "fe80::1", + "0.0.0.0", + "240.0.0.1", + ], + ) + def test_denies_special_addresses_when_private_addresses_are_configured(self, host): + validator = _BasicResolveResultValidator(allow_private_network_addresses=True) + + assert validator.is_allowed(_resolve_result(host)) is False + @pytest.mark.parametrize( "resolved", [ @@ -381,7 +405,9 @@ async def test_raises_ssrf_error_without_calling_handler_when_url_is_denied(self request = SimpleNamespace(url=URL("https://evil.example/file")) handler = AsyncMock() - with pytest.raises(_SSRFError, match="URL is not allowed by the outbound host validator"): + with pytest.raises( + _SSRFError, match="URL is not allowed by the outbound host validator" + ): await _validator_middleware(validator)(request, handler) handler.assert_not_awaited() From c771be6f798a9874ae44a7f2ce9ce6e045933c69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Brand=C3=A3o?= Date: Tue, 6 Oct 2026 11:46:50 -0700 Subject: [PATCH 13/15] Improve warning message for outbound host validation failure Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../microsoft_agents/hosting/core/app/attachment_downloader.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py index a2ae9ea19..fa4498c54 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py @@ -115,8 +115,7 @@ async def _download_file( ) except _SSRFError: logger.warning( - "Outbound host validation failed for download URL: %s", - remote_file_url, + "Outbound host validation failed for an attachment download URL." ) return None else: From 1068e08071cf59bed8b2afe9853fec925024b1ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20Brand=C3=A3o?= Date: Tue, 6 Oct 2026 11:47:13 -0700 Subject: [PATCH 14/15] Improve warning message for download URL validation Update warning message for outbound host validation failure. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../hosting/core/app/m365_attachment_downloader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py index 8ed2e9621..d3ec47d4d 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py @@ -188,7 +188,7 @@ async def _download_file( ) except _SSRFError: logger.warning( - "Outbound host validation failed for download URL: %s", download_url + "Outbound host validation failed for an attachment download URL." ) return None else: From c4993330a9cc728b1a86571f515e772825ae2485 Mon Sep 17 00:00:00 2001 From: Rodrigo Brandao Date: Tue, 6 Oct 2026 11:50:39 -0700 Subject: [PATCH 15/15] Updating stale docstring --- .../hosting/core/app/m365_attachment_downloader.py | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py index d3ec47d4d..04bd9c2a9 100644 --- a/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py +++ b/libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py @@ -48,6 +48,7 @@ def __init__( :param connections: A Connections instance. :param host_validator: An optional OutboundHostValidator instance. + :param client_session_kwargs: Optional keyword arguments to pass to the aiohttp ClientSession constructor. :param token_provider_name: The name of the token provider. :param use_anonymous: Whether to use anonymous access. :param scopes: A list of scopes for the access token.