Python: surface mid-run oauth_consent_request items from ResponsesHostServer

python/packages/core/agent_framework/_oauth.py

@@ -0,0 +1,79 @@
+# Copyright (c) Microsoft. All rights reserved.
+
+from __future__ import annotations
+
+import logging
+import re
+from urllib.parse import urlparse
+
+logger = logging.getLogger("agent_framework")
+
+__all__ = ["validate_oauth_consent_link"]
+
+# Characters allowed in a registered host name, and in the bracketed IPv6 form that
+# ``urlparse`` reports with its brackets already stripped.
+_HOST_PATTERN = re.compile(r"^[A-Za-z0-9._~%-]+$")
+_IPV6_HOST_PATTERN = re.compile(r"^[0-9A-Fa-f:.%-]+$")
+
+
+def _is_valid_host(hostname: str) -> bool:
+    """Return whether *hostname* is syntactically usable by a standard URL client.
+
+    ``urlparse`` does not reject hosts containing illegal characters, so values such as
+    ``exa mple.com`` are reported as a hostname even though no client can resolve them.
+    """
+    pattern = _IPV6_HOST_PATTERN if ":" in hostname else _HOST_PATTERN
+    return bool(pattern.match(hostname))
+
+
+def validate_oauth_consent_link(consent_link: str | None, *, item_id: str | None = None) -> str | None:
+    """Return *consent_link* when it is an absolute HTTPS URL a client can open, else ``None``.
+
+    A consent link is rendered as a clickable prompt by the client, so anything that is not
+    an absolute ``https`` URL is dropped rather than surfaced. Validation is shared by every
+    package that parses or re-emits ``oauth_consent_request`` content so the accepted shape
+    cannot drift between the provider that parses a link and the host that renders it.
+
+    ``urlparse`` is permissive in three ways that matter here, all handled below:
+
+    * it raises ``ValueError`` for malformed authorities (``https://[broken``) and for invalid
+      ports, but the port is only validated when it is read;
+    * a non-empty ``netloc`` does not imply a host (``https://@`` has one but no host), and a
+      non-empty ``hostname`` does not imply a usable one (``https://exa mple.com`` reports one);
+    * it silently strips tab and newline, so control characters must be rejected up front.
+
+    Args:
+        consent_link: The candidate consent URL, which may be ``None`` or empty.
+
+    Keyword Args:
+        item_id: Optional identifier of the source item, included in warning logs.
+
+    Returns:
+        The link unchanged when it is usable, otherwise ``None``.
+    """
+    if not consent_link:
+        return None
+
+    log_id = item_id or "<unknown>"
+
+    if any(char.isspace() or ord(char) < 0x20 or ord(char) == 0x7F for char in consent_link):
+        logger.warning(
+            "Skipping oauth_consent_request with whitespace or control characters in consent_link (item id=%s)",
+            log_id,
+        )
+        return None
+    try:
+        parsed = urlparse(consent_link)
+        hostname = parsed.hostname
+        # Reading ``port`` is what validates it; ``https://host:bad`` raises here.
+        _ = parsed.port
+    except ValueError:
+        logger.warning("Skipping oauth_consent_request with malformed consent_link (item id=%s)", log_id)
+        return None
+    if parsed.scheme.lower() != "https" or not hostname:
+        logger.warning("Skipping oauth_consent_request with non-HTTPS consent_link (item id=%s)", log_id)
+        return None
+    if not _is_valid_host(hostname):
+        logger.warning("Skipping oauth_consent_request with an invalid consent_link host (item id=%s)", log_id)
+        return None
+    return consent_link

python/packages/core/tests/test_oauth.py

@@ -0,0 +1,56 @@
+# Copyright (c) Microsoft. All rights reserved.
+
+import pytest
+
+from agent_framework._oauth import validate_oauth_consent_link
+
+
+@pytest.mark.parametrize(
+    "link",
+    [
+        "https://login.example.com/consent",
+        "https://login.example.com:8443/consent?state=abc#frag",
+        "https://192.0.2.10/consent",
+        "https://[2001:db8::1]:8443/consent",
+        "HTTPS://login.example.com/consent",
+    ],
+)
+def test_usable_links_are_returned_unchanged(link: str) -> None:
+    assert validate_oauth_consent_link(link) == link
+
+
+@pytest.mark.parametrize(
+    ("link", "reason"),
+    [
+        (None, "missing"),
+        ("", "empty"),
+        ("   ", "whitespace only"),
+        ("http://login.example.com/consent", "non-HTTPS scheme"),
+        ("ftp://login.example.com/consent", "non-HTTPS scheme"),
+        ("/consent", "relative, so no scheme or host"),
+        ("https://", "no host"),
+        ("https://@", "netloc present but no host"),
+        ("https://[broken", "malformed authority, urlparse raises"),
+        ("https://login.example.com:bad/consent", "port only validated when read"),
+        ("https://login.example.com:99999/consent", "port out of range"),
+        ("https://exa mple.com/consent", "space in host, unresolvable"),
+        ("https://cons|ent.example.com/", "illegal character in host"),
+        ("https://login.example.com/consent\n", "trailing newline, silently stripped by urlparse"),
+        ("https://login.example.com/\tconsent", "embedded tab, silently stripped by urlparse"),
+    ],
+)
+def test_unusable_links_are_rejected(link: str | None, reason: str) -> None:
+    assert validate_oauth_consent_link(link) is None, reason
+
+
+def test_rejection_is_logged_with_the_item_id(caplog: pytest.LogCaptureFixture) -> None:
+    with caplog.at_level("WARNING"):
+        assert validate_oauth_consent_link("http://login.example.com", item_id="item-5") is None
+    assert "non-HTTPS" in caplog.text
+    assert "item-5" in caplog.text
+
+
+def test_rejection_without_an_item_id_still_logs(caplog: pytest.LogCaptureFixture) -> None:
+    with caplog.at_level("WARNING"):
+        assert validate_oauth_consent_link("http://login.example.com") is None
+    assert "non-HTTPS" in caplog.text

python/packages/foundry/agent_framework_foundry/_oauth_helpers.py

@@ -4,26 +4,21 @@
 
 import logging
 from typing import Any
-from urllib.parse import urlparse
 
 from agent_framework import ChatResponseUpdate, Content
+from agent_framework._oauth import validate_oauth_consent_link
 
 logger = logging.getLogger(__name__)
 
 
 def _validate_consent_link(consent_link: str, item_id: str) -> str:
-    """Validate a consent link is HTTPS with a valid netloc.
+    """Validate a consent link is HTTPS with a valid host and port.
 
-    Returns the link unchanged if valid, or an empty string if not.
+    Thin wrapper over the shared core validator that keeps this module's empty-string
+    contract. The rules live in ``agent_framework._oauth`` so the parser here and the
+    Foundry hosting layer that re-emits the link cannot drift apart.
     """
-    parsed = urlparse(consent_link)
-    if parsed.scheme.lower() != "https" or not parsed.netloc:
-        logger.warning(
-            "Skipping oauth_consent_request with non-HTTPS consent_link (item id=%s)",
-            item_id,
-        )
-        return ""
-    return consent_link
+    return validate_oauth_consent_link(consent_link, item_id=item_id) or ""
 
 
 def try_parse_oauth_consent_event(event: Any, model: str) -> ChatResponseUpdate | None:
@@ -33,6 +28,10 @@ def try_parse_oauth_consent_event(event: Any, model: str) -> ChatResponseUpdate
     ``response.output_item.added`` carrying an ``oauth_consent_request`` item
     or a top-level ``response.oauth_consent_requested`` event,
     or ``None`` so the caller can fall through to the base implementation.
+
+    The consent request is surfaced even when its link is missing or unusable, so that a
+    turn which cannot proceed is never reported as a silent success. Link validation is
+    applied for diagnostics here and enforced by the host that renders the link.
     """
     consent_link: str = ""
     raw_item: Any = None
@@ -51,22 +50,32 @@ def try_parse_oauth_consent_event(event: Any, model: str) -> ChatResponseUpdate
     item_id = getattr(raw_item, "id", "<unknown>")
 
     if consent_link:
-        consent_link = _validate_consent_link(consent_link, item_id)
-
-    contents: list[Content] = []
-    if consent_link:
-        contents.append(
-            Content.from_oauth_consent_request(
-                consent_link=consent_link,
-                raw_representation=raw_item,
-            )
-        )
+        # Validation here is diagnostic only. The provider has signalled that the turn
+        # cannot proceed without consent, so the request is always surfaced: dropping it
+        # would let a blocked turn finish as a silent success. The host re-validates and
+        # is the single authority on whether a link is renderable, failing the response
+        # when it is not.
+        _validate_consent_link(consent_link, item_id)
     else:
         logger.warning(
             "Received oauth_consent_request output without valid consent_link (item id=%s)",
             item_id,
         )
 
+    # ``server_label`` identifies the MCP server that needs consent and is required by
+    # downstream Responses output items. It is copied into ``additional_properties``
+    # because ``raw_representation`` is provider specific and does not survive a
+    # session round trip.
+    server_label = getattr(raw_item, "server_label", None)
+    additional_properties = {"server_label": server_label} if isinstance(server_label, str) and server_label else None
+    contents: list[Content] = [
+        Content.from_oauth_consent_request(
+            consent_link=consent_link,
+            additional_properties=additional_properties,
+            raw_representation=raw_item,
+        )
+    ]
+
     return ChatResponseUpdate(
         contents=contents,
         role="assistant",

python/packages/foundry/tests/foundry/test_foundry_agent.py

@@ -1655,8 +1655,8 @@ def test_parse_chunk_surfaces_oauth_consent_request() -> None:
     assert update.raw_representation is mock_event
 
 
-def test_parse_chunk_skips_non_https_oauth_consent() -> None:
-    """An oauth_consent_request with a non-HTTPS link is rejected."""
+def test_parse_chunk_surfaces_non_https_oauth_consent() -> None:
+    """A non-HTTPS link is still surfaced so the host can fail the response."""
 
     mock_project = MagicMock()
     mock_project.get_openai_client.return_value = MagicMock()
@@ -1678,11 +1678,12 @@ def test_parse_chunk_skips_non_https_oauth_consent() -> None:
     update = client._parse_chunk_from_openai(mock_event, {}, {})
 
     consent_contents = [c for c in update.contents if c.type == "oauth_consent_request"]
-    assert len(consent_contents) == 0
+    assert len(consent_contents) == 1
+    assert consent_contents[0].consent_link == "http://insecure.example.com/login"
 
 
 def test_parse_chunk_handles_missing_consent_link() -> None:
-    """An oauth_consent_request without a consent_link produces no content."""
+    """A missing consent_link still surfaces the request, with an empty link."""
 
     mock_project = MagicMock()
     mock_project.get_openai_client.return_value = MagicMock()
@@ -1704,11 +1705,12 @@ def test_parse_chunk_handles_missing_consent_link() -> None:
     update = client._parse_chunk_from_openai(mock_event, {}, {})
 
     consent_contents = [c for c in update.contents if c.type == "oauth_consent_request"]
-    assert len(consent_contents) == 0
+    assert len(consent_contents) == 1
+    assert consent_contents[0].consent_link == ""
 
 
 def test_parse_chunk_handles_empty_string_consent_link() -> None:
-    """An oauth_consent_request with empty-string consent_link produces no content."""
+    """An empty-string consent_link still surfaces the request."""
 
     mock_project = MagicMock()
     mock_project.get_openai_client.return_value = MagicMock()
@@ -1730,7 +1732,8 @@ def test_parse_chunk_handles_empty_string_consent_link() -> None:
     update = client._parse_chunk_from_openai(mock_event, {}, {})
 
     consent_contents = [c for c in update.contents if c.type == "oauth_consent_request"]
-    assert len(consent_contents) == 0
+    assert len(consent_contents) == 1
+    assert consent_contents[0].consent_link == ""
 
 
 def test_parse_chunk_delegates_non_oauth_events_to_super() -> None:

python/packages/foundry/tests/foundry/test_foundry_chat_client.py

@@ -1453,8 +1453,8 @@ def test_parse_chunk_surfaces_oauth_consent_request() -> None:
     assert update.model == "test-model"
 
 
-def test_parse_chunk_skips_non_https_oauth_consent() -> None:
-    """An oauth_consent_request with a non-HTTPS link is rejected."""
+def test_parse_chunk_surfaces_non_https_oauth_consent() -> None:
+    """A non-HTTPS link is still surfaced so the host can fail the response."""
 
     mock_project = MagicMock()
     mock_openai = _make_mock_openai_client()
@@ -1477,11 +1477,12 @@ def test_parse_chunk_skips_non_https_oauth_consent() -> None:
     update = client._parse_chunk_from_openai(mock_event, {}, {})
 
     consent_contents = [c for c in update.contents if c.type == "oauth_consent_request"]
-    assert len(consent_contents) == 0
+    assert len(consent_contents) == 1
+    assert consent_contents[0].consent_link == "http://insecure.example.com/login"
 
 
 def test_parse_chunk_handles_missing_consent_link() -> None:
-    """An oauth_consent_request without a consent_link produces no content."""
+    """A missing consent_link still surfaces the request, with an empty link."""
 
     mock_project = MagicMock()
     mock_openai = _make_mock_openai_client()
@@ -1504,11 +1505,12 @@ def test_parse_chunk_handles_missing_consent_link() -> None:
     update = client._parse_chunk_from_openai(mock_event, {}, {})
 
     consent_contents = [c for c in update.contents if c.type == "oauth_consent_request"]
-    assert len(consent_contents) == 0
+    assert len(consent_contents) == 1
+    assert consent_contents[0].consent_link == ""
 
 
 def test_parse_chunk_handles_empty_string_consent_link() -> None:
-    """An oauth_consent_request with empty-string consent_link produces no content."""
+    """An empty-string consent_link still surfaces the request."""
 
     mock_project = MagicMock()
     mock_openai = _make_mock_openai_client()
@@ -1531,7 +1533,8 @@ def test_parse_chunk_handles_empty_string_consent_link() -> None:
     update = client._parse_chunk_from_openai(mock_event, {}, {})
 
     consent_contents = [c for c in update.contents if c.type == "oauth_consent_request"]
-    assert len(consent_contents) == 0
+    assert len(consent_contents) == 1
+    assert consent_contents[0].consent_link == ""
 
 
 def test_parse_chunk_delegates_non_oauth_events_to_super() -> None: