From 3dd16f53379436b9aed5f64b4387b1662d622c69 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 23 Jun 2026 12:11:52 -0700 Subject: [PATCH 1/3] fix(teams): close mixed-case-scheme bypass in graph SSRF/token-leak guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/chat_sdk/adapters/teams/graph/__init__.py | 20 +++++++++++++------ tests/test_teams_graph_primitive.py | 17 ++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/chat_sdk/adapters/teams/graph/__init__.py b/src/chat_sdk/adapters/teams/graph/__init__.py index 75d6085..b3b5d73 100644 --- a/src/chat_sdk/adapters/teams/graph/__init__.py +++ b/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,15 @@ 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 (fail closed); only a truly relative path is joined onto the base. + parsed = urlparse(path_or_url) + if parsed.scheme or parsed.netloc: 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 diff --git a/tests/test_teams_graph_primitive.py b/tests/test_teams_graph_primitive.py index 7965d50..35605cb 100644 --- a/tests/test_teams_graph_primitive.py +++ b/tests/test_teams_graph_primitive.py @@ -309,6 +309,23 @@ 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() + 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. From d247683530873831b01647d34eeb08517a0d179a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 23 Jun 2026 12:21:29 -0700 Subject: [PATCH 2/3] =?UTF-8?q?fix(teams):=20harden=20graph=20URL=20routin?= =?UTF-8?q?g=20per=20review=20=E2=80=94=20fail-closed=20parse,=20keep=20co?= =?UTF-8?q?lon=20paths?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/chat_sdk/adapters/teams/graph/__init__.py | 17 +++++++----- tests/test_teams_graph_primitive.py | 26 +++++++++++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/chat_sdk/adapters/teams/graph/__init__.py b/src/chat_sdk/adapters/teams/graph/__init__.py index b3b5d73..c850fa4 100644 --- a/src/chat_sdk/adapters/teams/graph/__init__.py +++ b/src/chat_sdk/adapters/teams/graph/__init__.py @@ -262,12 +262,17 @@ async def call_teams_graph_api(path_or_url: str, options: TeamsGraphOptions) -> # 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 (fail closed); only a truly relative path is joined onto the base. - parsed = urlparse(path_or_url) - if parsed.scheme or parsed.netloc: + # 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 diff --git a/tests/test_teams_graph_primitive.py b/tests/test_teams_graph_primitive.py index 35605cb..60b7336 100644 --- a/tests/test_teams_graph_primitive.py +++ b/tests/test_teams_graph_primitive.py @@ -326,6 +326,32 @@ async def test_rejects_mixed_case_scheme_and_scheme_relative_attacker_urls(self) 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. From 379cc9c2ed74ea3deed6d11dfee9ec4901d5ddc4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 23 Jun 2026 12:23:11 -0700 Subject: [PATCH 3/3] style: ruff format (collapse one-line call in graph test) Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_teams_graph_primitive.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_teams_graph_primitive.py b/tests/test_teams_graph_primitive.py index 60b7336..5ab0cab 100644 --- a/tests/test_teams_graph_primitive.py +++ b/tests/test_teams_graph_primitive.py @@ -343,9 +343,7 @@ async def test_colon_in_relative_graph_path_still_joins_onto_the_trusted_base(se # 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) - ) + 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" )