test(teams): harden format boundary scan + tighten safe_link_href to upstream parity

Boundary test: replace the substring scan over raw import lines with an
AST-based one that reconstructs the fully-qualified module path each import
statement reaches. The old scan missed the idiomatic split form
`from chat_sdk.adapters.teams import adapter` (the dotted token
`chat_sdk.adapters.teams.adapter` never appears on one side of ` import `),
the most likely circular-import / SDK-pull-in regression. Both
`import <forbidden>` and `from <forbidden> import ...` forms are now caught
for the adapter module, microsoft_teams, and httpx/aiohttp.

safe_link_href: upstream uses `new URL(href).protocol`, which throws (=>
rejected => plain label) for malformed bare-scheme hrefs like `http:` or
`https://`. `urlparse(...).scheme` is lenient and passed those (emitting a
live link). Require a non-empty host for the http/https branch so bare-scheme
http(s) hrefs are rejected to parity; mailto: (no netloc) stays allowed.
Adds regression tests for both.

Author: patrick-chinchill

src/chat_sdk/adapters/teams/format.py

@@ -21,7 +21,11 @@
 through an exact ``{http, https, mailto}`` protocol allowlist using
 :func:`urllib.parse.urlparse` (port of the upstream ``URL().protocol`` check),
 rejecting ``javascript:``, ``data:``, relative, and other unsafe hrefs so they
-render as plain text rather than active links (SSRF / injection guard).
+render as plain text rather than active links (SSRF / injection guard). Because
+``urlparse`` is more lenient than the upstream ``new URL(...)`` (which throws on
+a bare-scheme href like ``http:`` or ``https://``), the ``http``/``https``
+branch additionally requires a non-empty host so those malformed hrefs are
+rejected to parity.
 """
 
 from __future__ import annotations
@@ -179,8 +183,21 @@ def safe_link_href(href: str) -> bool:
     Port of the upstream ``safeLinkHref`` protocol check using
     :func:`urllib.parse.urlparse`. Rejects ``javascript:``, ``data:``,
     relative, and any other scheme (SSRF / injection guard).
+
+    Upstream parses the href with ``new URL(href)``, which *throws* for a
+    malformed bare-scheme href like ``http:`` or ``https://`` (no authority),
+    so such hrefs are rejected and render as plain label text.
+    :func:`urllib.parse.urlparse` is lenient and would yield a matching scheme
+    with an empty ``netloc`` for those, so the ``http``/``https`` branch
+    additionally requires a non-empty host to match upstream. ``mailto:`` has
+    no ``netloc`` by design and stays allowed.
     """
     try:
-        return urlparse(href).scheme in _SAFE_LINK_PROTOCOLS
+        parsed = urlparse(href)
     except ValueError:
         return False
+    if parsed.scheme not in _SAFE_LINK_PROTOCOLS:
+        return False
+    if parsed.scheme in ("http", "https"):
+        return bool(parsed.netloc)
+    return True

tests/test_teams_format_primitives.py

@@ -14,6 +14,7 @@
 
 from __future__ import annotations
 
+import ast
 import importlib.util
 from pathlib import Path
 
@@ -102,6 +103,27 @@ def test_disallowed_protocols_fail_the_ssrf_gate(self):
         assert safe_link_href("/internal") is False
         assert safe_link_href("") is False
 
+    def test_bare_scheme_http_hrefs_are_rejected_to_upstream_parity(self):
+        # Upstream parses with `new URL(href)`, which throws (-> rejected) for a
+        # bare-scheme http(s) href with no host. `urlparse` is lenient and would
+        # yield a matching scheme with an empty netloc, so the gate requires a
+        # non-empty host for http/https to stay at parity.
+        assert safe_link_href("http:") is False
+        assert safe_link_href("https://") is False
+        assert safe_link_href("https:") is False
+        # ...but a host-bearing http(s) href and host-less mailto still pass.
+        assert safe_link_href("https://example.com") is True
+        assert safe_link_href("http://example.com") is True
+        assert safe_link_href("mailto:x@y.com") is True
+
+    def test_bare_scheme_links_render_as_plain_label(self):
+        # End-to-end: a bare-scheme href must render as plain label, while a
+        # host-bearing https link and a mailto link still emit live anchors.
+        assert markdown_to_teams_html("[l](http:)") == "l"
+        assert markdown_to_teams_html("[l](https://)") == "l"
+        assert markdown_to_teams_html("[m](mailto:x@y.com)") == '<a href="mailto:x@y.com">m</a>'
+        assert markdown_to_teams_html("[e](https://example.com)") == '<a href="https://example.com">e</a>'
+
     def test_unescape_does_not_collapse_double_escaped_ampersand(self):
         # `&amp;lt;` must round-trip to `&lt;`, not `<` (reverse-order unescape).
         assert unescape_teams_text("&amp;lt;") == "&lt;"
@@ -113,6 +135,35 @@ def _format_module_source() -> str:
     return Path(spec.origin).read_text(encoding="utf8")
 
 
+def _imported_module_paths(source: str) -> list[tuple[str, list[str]]]:
+    """Yield ``(line, modules)`` for each import statement in ``source``.
+
+    ``modules`` is every fully-qualified module path the statement reaches:
+
+    * ``import a.b`` -> ``["a.b"]``
+    * ``from a.b import c, d`` -> ``["a.b", "a.b.c", "a.b.d"]`` (so the split
+      ``from <pkg> import <module>`` form is normalized to ``<pkg>.<module>``)
+
+    AST parsing makes the scan robust to formatting and reconstructs the dotted
+    path that a plain substring check over the raw line would miss.
+    """
+    tree = ast.parse(source)
+    lines = source.splitlines()
+    results: list[tuple[str, list[str]]] = []
+    for node in ast.walk(tree):
+        if isinstance(node, ast.Import):
+            modules = [alias.name for alias in node.names]
+        elif isinstance(node, ast.ImportFrom):
+            base = node.module or ""
+            modules = [base] if base else []
+            modules += [f"{base}.{alias.name}" if base else alias.name for alias in node.names]
+        else:
+            continue
+        line = lines[node.lineno - 1].strip() if node.lineno - 1 < len(lines) else ""
+        results.append((line, modules))
+    return results
+
+
 class TestFormatImportBoundary:
     """Port of upstream ``format/boundary.test.ts``.
 
@@ -127,11 +178,10 @@ class TestFormatImportBoundary:
     def test_source_does_not_import_the_sdk_runtime_or_adapter(self):
         # Inspect only the actual import statements, so the docstring (which
         # *mentions* these modules to describe the boundary) is not flagged.
-        import_lines = [
-            line.strip()
-            for line in _format_module_source().splitlines()
-            if line.strip().startswith(("import ", "from "))
-        ]
+        # Both `import <forbidden>` AND `from <forbidden> import ...` forms must
+        # be caught — including the split `from chat_sdk.adapters.teams import
+        # adapter` form, which a plain substring scan misses (the dotted module
+        # path `chat_sdk.adapters.teams.adapter` never appears on one side).
         forbidden = (
             "microsoft_teams",
             "httpx",
@@ -141,7 +191,12 @@ def test_source_does_not_import_the_sdk_runtime_or_adapter(self):
             "chat_sdk.adapters.teams.cards",
             "chat_sdk.chat",
         )
-        present = [f"{token} :: {line}" for line in import_lines for token in forbidden if token in line]
+        present = [
+            f"{token} :: {line}"
+            for line, modules in _imported_module_paths(_format_module_source())
+            for token in forbidden
+            if any(module == token or module.startswith(f"{token}.") for module in modules)
+        ]
         assert not present, f"format primitive imports forbidden modules: {present}"
 
     def test_emoji_reuse_does_not_duplicate_unicode(self):