Skip to content

Commit 3dd16f5

Browse files
committed
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>
1 parent 68a77dc commit 3dd16f5

2 files changed

Lines changed: 31 additions & 6 deletions

File tree

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -245,11 +245,11 @@ async def resolve_graph_access_token(options: TeamsGraphOptions) -> str:
245245
async def call_teams_graph_api(path_or_url: str, options: TeamsGraphOptions) -> Any:
246246
"""Call a Microsoft Graph endpoint with a Graph-scoped bearer token.
247247
248-
Port of upstream ``callTeamsGraphApi``. An absolute ``path_or_url`` (one
249-
starting with ``http``) is used as-is; otherwise it is joined onto the base
250-
Graph URL (``options.graph_url`` or ``https://graph.microsoft.com/v1.0/``)
251-
after stripping leading slashes. Raises :class:`TeamsApiError` for non-2xx
252-
responses.
248+
Port of upstream ``callTeamsGraphApi``. An absolute ``path_or_url`` (one that
249+
:func:`urllib.parse.urlparse` reads as having a scheme or a host) is used
250+
as-is; otherwise it is joined onto the base Graph URL (``options.graph_url``
251+
or ``https://graph.microsoft.com/v1.0/``) after stripping leading slashes.
252+
Raises :class:`TeamsApiError` for non-2xx responses.
253253
254254
Python-specific hardening: an absolute ``path_or_url`` is validated through
255255
:func:`is_trusted_graph_url` **before** the token is resolved or attached,
@@ -259,7 +259,15 @@ async def call_teams_graph_api(path_or_url: str, options: TeamsGraphOptions) ->
259259
trusted base URL, so they need no host check. Upstream performs no such
260260
check.
261261
"""
262-
if path_or_url.startswith("http"):
262+
# Route on the PARSED form, not a case-sensitive ``startswith("http")`` prefix.
263+
# ``HTTPS://evil`` (mixed-case scheme) and ``//evil`` (scheme-relative) both slip
264+
# past a lowercase-prefix test, yet ``urljoin`` still resolves them to an absolute
265+
# attacker URL — so the routing diverged from ``is_trusted_graph_url`` and the
266+
# Graph token could attach to an untrusted host. Anything ``urlparse`` reads as
267+
# having a scheme or a netloc is treated as absolute and forced through the host
268+
# allowlist (fail closed); only a truly relative path is joined onto the base.
269+
parsed = urlparse(path_or_url)
270+
if parsed.scheme or parsed.netloc:
263271
if not is_trusted_graph_url(path_or_url):
264272
raise ValueError(f"Refusing to call Microsoft Graph API on untrusted host: {path_or_url}")
265273
url = path_or_url

tests/test_teams_graph_primitive.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,23 @@ async def test_call_with_an_absolute_attacker_url_is_rejected(self) -> None:
309309
await paginate_teams_graph("http://graph.microsoft.com/v1.0/next", _opts("p", fetch=request))
310310
request.assert_not_awaited()
311311

312+
@pytest.mark.asyncio
313+
async def test_rejects_mixed_case_scheme_and_scheme_relative_attacker_urls(self) -> None:
314+
# Regression: a case-sensitive ``startswith("http")`` routing test let a
315+
# mixed-case scheme (``HTTPS://``) or a scheme-relative (``//host``) URL
316+
# skip the absolute branch; ``urljoin`` still resolved it to the attacker
317+
# host and attached the Graph token. Routing now keys off the parsed
318+
# scheme/netloc, so every absolute form is forced through the allowlist.
319+
for hostile in (
320+
"HTTPS://evil.example/x",
321+
"HtTpS://evil.example/x",
322+
"//evil.example/x",
323+
):
324+
request = _fetch(_json_response(_TOKEN_BODY))
325+
with pytest.raises(ValueError, match="untrusted host"):
326+
await paginate_teams_graph(hostile, _opts("p", fetch=request))
327+
request.assert_not_awaited()
328+
312329
def test_is_trusted_graph_url_allowlist(self) -> None:
313330
assert is_trusted_graph_url("https://graph.microsoft.com/v1.0/next") is True
314331
# Wrong scheme, lookalike suffix, foreign host, and parse junk all fail.

0 commit comments

Comments
 (0)