Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions src/chat_sdk/adapters/teams/graph/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
41 changes: 41 additions & 0 deletions tests/test_teams_graph_primitive.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading