fix(teams): close mixed-case-scheme bypass in graph SSRF/token-leak guard (#178)

* fix(teams): close mixed-case-scheme bypass in graph SSRF/token-leak guard

call_teams_graph_api routed absolute-vs-relative on `path_or_url.startswith("http")`,
which is case-sensitive. A value like `HTTPS://evil.example/x` (or `HtTpS://`, or the
scheme-relative `//evil.example/x`) fails that prefix test, so it fell into the
"relative path" branch where `urljoin` still resolved it to the absolute attacker host
— and the Graph-scoped bearer token was attached WITHOUT ever consulting
is_trusted_graph_url. The routing test diverged from the actual allowlist.

Route on the parsed form instead: anything urlparse reads as having a scheme or a
netloc is treated as absolute and forced through is_trusted_graph_url (fail closed);
only a truly relative path is joined onto the trusted base. is_trusted_graph_url itself
was already correct (urlparse + scheme=='https' + exact-host) — it just wasn't reached.
The relative-join path (incl. sovereign-cloud `options.graph_url` overrides) is unchanged.

Found by a retroactive local-Codex review of the merged 4.30/4.31 train + manual
verification (Codex rated it HIGH; verified real but gated on an attacker-influenced
@odata.nextLink, so MEDIUM — it still defeats the guard the PR advertises).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(teams): harden graph URL routing per review — fail-closed parse, keep colon paths

Address PR review feedback on the SSRF routing fix:
- Gemini (robustness): wrap urlparse in try/except and fail closed (treat an
  unparseable URL, e.g. a bad IPv6 literal, as absolute so the allowlist rejects
  it) instead of letting urlparse raise out of the function.
- Codex (P2, colon paths): keep routing on `scheme or netloc`. A bare `root:/…`
  segment never joined via urljoin anyway (urljoin sees scheme `root` and returns
  it unchanged), so rejecting it is the safe behavior — routing it to the relative
  branch would attach the Graph token to a malformed `root:` URL. Realistic Graph
  colon paths put the colon AFTER a slash (`me/drive/root:/x:/content`), parse with
  no scheme, and still join onto the trusted base — added a test proving it.

Tests: +malformed-URL-fails-closed, +colon-after-slash-relative-path-joins (19 total).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* style: ruff format (collapse one-line call in graph test)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: patrick-chinchill

src/chat_sdk/adapters/teams/graph/__init__.py

@@ -245,11 +245,11 @@ async def resolve_graph_access_token(options: TeamsGraphOptions) -> str:
 async def call_teams_graph_api(path_or_url: str, options: TeamsGraphOptions) -> Any:
     """Call a Microsoft Graph endpoint with a Graph-scoped bearer token.
 
-    Port of upstream ``callTeamsGraphApi``. An absolute ``path_or_url`` (one
-    starting with ``http``) is used as-is; otherwise it is joined onto the base
-    Graph URL (``options.graph_url`` or ``https://graph.microsoft.com/v1.0/``)
-    after stripping leading slashes. Raises :class:`TeamsApiError` for non-2xx
-    responses.
+    Port of upstream ``callTeamsGraphApi``. An absolute ``path_or_url`` (one that
+    :func:`urllib.parse.urlparse` reads as having a scheme or a host) is used
+    as-is; otherwise it is joined onto the base Graph URL (``options.graph_url``
+    or ``https://graph.microsoft.com/v1.0/``) after stripping leading slashes.
+    Raises :class:`TeamsApiError` for non-2xx responses.
 
     Python-specific hardening: an absolute ``path_or_url`` is validated through
     :func:`is_trusted_graph_url` **before** the token is resolved or attached,
@@ -259,7 +259,20 @@ async def call_teams_graph_api(path_or_url: str, options: TeamsGraphOptions) ->
     trusted base URL, so they need no host check. Upstream performs no such
     check.
     """
-    if path_or_url.startswith("http"):
+    # Route on the PARSED form, not a case-sensitive ``startswith("http")`` prefix.
+    # ``HTTPS://evil`` (mixed-case scheme) and ``//evil`` (scheme-relative) both slip
+    # past a lowercase-prefix test, yet ``urljoin`` still resolves them to an absolute
+    # attacker URL — so the routing diverged from ``is_trusted_graph_url`` and the Graph
+    # token could attach to an untrusted host. Anything ``urlparse`` reads as having a
+    # scheme or a netloc is treated as absolute and forced through the host allowlist;
+    # a malformed URL (``urlparse`` raises, e.g. a bad IPv6 literal) fails closed the
+    # same way. Only a truly relative path is joined onto the trusted base.
+    try:
+        parsed = urlparse(path_or_url)
+        is_absolute = bool(parsed.scheme or parsed.netloc)
+    except (ValueError, TypeError):
+        is_absolute = True  # unparseable -> fail closed; the allowlist below rejects it
+    if is_absolute:
         if not is_trusted_graph_url(path_or_url):
             raise ValueError(f"Refusing to call Microsoft Graph API on untrusted host: {path_or_url}")
         url = path_or_url

tests/test_teams_graph_primitive.py

@@ -309,6 +309,47 @@ async def test_call_with_an_absolute_attacker_url_is_rejected(self) -> None:
             await paginate_teams_graph("http://graph.microsoft.com/v1.0/next", _opts("p", fetch=request))
         request.assert_not_awaited()
 
+    @pytest.mark.asyncio
+    async def test_rejects_mixed_case_scheme_and_scheme_relative_attacker_urls(self) -> None:
+        # Regression: a case-sensitive ``startswith("http")`` routing test let a
+        # mixed-case scheme (``HTTPS://``) or a scheme-relative (``//host``) URL
+        # skip the absolute branch; ``urljoin`` still resolved it to the attacker
+        # host and attached the Graph token. Routing now keys off the parsed
+        # scheme/netloc, so every absolute form is forced through the allowlist.
+        for hostile in (
+            "HTTPS://evil.example/x",
+            "HtTpS://evil.example/x",
+            "//evil.example/x",
+        ):
+            request = _fetch(_json_response(_TOKEN_BODY))
+            with pytest.raises(ValueError, match="untrusted host"):
+                await paginate_teams_graph(hostile, _opts("p", fetch=request))
+            request.assert_not_awaited()
+
+    @pytest.mark.asyncio
+    async def test_malformed_url_fails_closed_without_fetching_a_token(self) -> None:
+        # urlparse raises on some inputs (e.g. a bad IPv6 literal). The router must
+        # fail closed — treat it as absolute and reject via the allowlist — never join
+        # it or fetch a token for it.
+        request = _fetch(_json_response(_TOKEN_BODY))
+        with pytest.raises(ValueError, match="untrusted host"):
+            await paginate_teams_graph("https://[oops", _opts("p", fetch=request))
+        request.assert_not_awaited()
+
+    @pytest.mark.asyncio
+    async def test_colon_in_relative_graph_path_still_joins_onto_the_trusted_base(self) -> None:
+        # A relative Graph segment whose colon falls after a slash (e.g. a OneDrive
+        # ``…/root:/path:/content`` address) parses with no scheme/netloc, so the
+        # parse-based routing still joins it onto the trusted base — it is not
+        # over-rejected as an absolute URL.
+        request = _fetch(_json_response(_TOKEN_BODY), _json_response({"id": "x"}))
+        result = await paginate_teams_graph("me/drive/root:/Reports/jan.csv:/content", _opts("p", fetch=request))
+        assert _url(request.await_args_list[1]) == (
+            "https://graph.microsoft.com/v1.0/me/drive/root:/Reports/jan.csv:/content"
+        )
+        assert _headers(request.await_args_list[1])["authorization"] == "Bearer graph-token"
+        assert result == {"id": "x"}
+
     def test_is_trusted_graph_url_allowlist(self) -> None:
         assert is_trusted_graph_url("https://graph.microsoft.com/v1.0/next") is True
         # Wrong scheme, lookalike suffix, foreign host, and parse junk all fail.